1132894 - MUST_REUSE_INPUT should reuse the same physical register, but with a different type.

Status: REOPENED

(Core :: JavaScript Engine: JIT, defect)

Description

[Nicolas B. Pierron [:nbp]]

SIMD has an issue which is that if we want to type SIMD registers correctly, then we have to distinguish Int32x4 and Float32x4 content of the same register.

Adding a type to these registers is needed for spilling them efficiently on some x86 / x64 CPU.  Instructions such as MSimdBinaryComp can have a MIRType_Float32x4 operands with a MIRType_Int32x4 output.  At the moment we are using lowerForFPU, but MUST_REUSE_INPUT is at the moment unable to change the associated type of the register.

We should fix the register allocator to consider the type of the LDefinition, even if it is marked as being MUST_REUSE_INPUT.

Comment 1

[Jakob Stoklund Olesen [:jolesen]]

Is this still an issue after bug 1226027?

Comment 2

[Nicolas B. Pierron [:nbp]]

Bug 1226027 replace the LIR type by a unique one based on the size, which means that we no longer have the fine typing which caused this issue, so this should no longer be a problem.

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

There are actually 3 places that refer to this bug in the code. In particular, MSimdReinterpret ought to be simplified now. Reopening, so as we investigate on the topic and decide whether to remove the reference to this bug, or patch the code.

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: wip

That's a work in progress on this topic, but I am very unsure about what the fix should look like. Even though we only have a single type for representing the SIMD FloatRegister now, some of these checks still seem needed.

Nicolas, would you be willing to help here, please?

Comment 5

[Nicolas B. Pierron [:nbp]]

(In reply to Benjamin Bouvier [:bbouvier] from comment #4)
> Created attachment 8719450 [details] [diff] [review]
> wip
> 
> That's a work in progress on this topic, but I am very unsure about what the
> fix should look like. Even though we only have a single type for
> representing the SIMD FloatRegister now, some of these checks still seem
> needed.
> 
> Nicolas, would you be willing to help here, please?

The type, as referred in the comment, relates to the type which is stored as part of the Mask of the FloatRegisterSet.  This mask distinguish which register is used, and MUST_REUSE_INPUT is not aware of this aliasing relations, except that they cannot be allocated at the same time.

Before Bug 1226027, you could not reinterpret an Int32x4 register into a Float32x4 register, even if this is the same representation, and if both are stored inside FloatRegisters.

It should be same to start re-using the redefine function on x86 now.