Bug 125275 – FTL should use cvttsd2si directly for double-to-int32 conversions

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 218490 [details]
the patch

[Nadav Rotem]

Filip, in sensibleDoubleToInt you are creating an undef vector and insert a value into it.  If you don't initialize the vector the RA can pick whatever vector register it wants, potentially causing a partial-register dependency.  I would zero the register before inserting a value into it.

[Filip Pizlo]

(In reply to comment #2)
> Filip, in sensibleDoubleToInt you are creating an undef vector and insert a value into it.  If you don't initialize the vector the RA can pick whatever vector register it wants, potentially causing a partial-register dependency.

For my own edification, can you explain what the downside to my current approach is?  Under what conditions would this lead to a partial-register dependency, and why is that bad?

I was leaving the second element of the vector as undef bits because I thought that this would be the best way to tell LLVM that it's OK to use a register that had garbage in those bits.  But I take it that initializing it to zero would anyway allow LLVM to do this because LLVM can see that the initialization of those bits to zero is dead anyway?

> I would zero the register before inserting a value into it.

Thanks for the feedback, I will do it!  To make sure you mean:

%1 = insertelement undef, %doubleValue, i32 0
%2 = insertelement %1, 0.0, i32 1
%3 = call @llvm.x86.sse2.cvttsd2si(%2)

Right?

[Filip Pizlo]

(In reply to comment #3)
> (In reply to comment #2)
> > Filip, in sensibleDoubleToInt you are creating an undef vector and insert a value into it.  If you don't initialize the vector the RA can pick whatever vector register it wants, potentially causing a partial-register dependency.
> 
> For my own edification, can you explain what the downside to my current approach is?  Under what conditions would this lead to a partial-register dependency, and why is that bad?
> 
> I was leaving the second element of the vector as undef bits because I thought that this would be the best way to tell LLVM that it's OK to use a register that had garbage in those bits.  But I take it that initializing it to zero would anyway allow LLVM to do this because LLVM can see that the initialization of those bits to zero is dead anyway?
> 
> > I would zero the register before inserting a value into it.
> 
> Thanks for the feedback, I will do it!  To make sure you mean:
> 
> %1 = insertelement undef, %doubleValue, i32 0
> %2 = insertelement %1, 0.0, i32 1
> %3 = call @llvm.x86.sse2.cvttsd2si(%2)
> 
> Right?

OK, I made this change and it seems to work.  I will land with this change.  In particular, here's what the relevant method looks like now:

    LValue sensibleDoubleToInt(LValue value)
    {
        RELEASE_ASSERT(isX86());
        return call(
            x86SSE2CvtTSD2SIIntrinsic(),
            insertElement(
                insertElement(getUndef(vectorType(doubleType, 2)), value, int32Zero),
                doubleZero, int32One));
    }

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/160205

[Nadav Rotem]

I(In reply to comment #3)
> (In reply to comment #2)
> > Filip, in sensibleDoubleToInt you are creating an undef vector and insert a value into it.  If you don't initialize the vector the RA can pick whatever vector register it wants, potentially causing a partial-register dependency.
> 
> For my own edification, can you explain what the downside to my current approach is?  Under what conditions would this lead to a partial-register dependency, and why is that bad?
> 
> I was leaving the second element of the vector as undef bits because I thought that this would be the best way to tell LLVM that it's OK to use a register that had garbage in those bits.  But I take it that initializing it to zero would anyway allow LLVM to do this because LLVM can see that the initialization of those bits to zero is dead anyway?
> 
> > I would zero the register before inserting a value into it.
> 
> Thanks for the feedback, I will do it!  To make sure you mean:
> 
> %1 = insertelement undef, %doubleValue, i32 0
> %2 = insertelement %1, 0.0, i32 1
> %3 = call @llvm.x86.sse2.cvttsd2si(%2)
> 
> Right?

The IR looks right. The assembly looks okay ?

[Filip Pizlo]

(In reply to comment #6)
> I(In reply to comment #3)
> > (In reply to comment #2)
> > > Filip, in sensibleDoubleToInt you are creating an undef vector and insert a value into it.  If you don't initialize the vector the RA can pick whatever vector register it wants, potentially causing a partial-register dependency.
> > 
> > For my own edification, can you explain what the downside to my current approach is?  Under what conditions would this lead to a partial-register dependency, and why is that bad?
> > 
> > I was leaving the second element of the vector as undef bits because I thought that this would be the best way to tell LLVM that it's OK to use a register that had garbage in those bits.  But I take it that initializing it to zero would anyway allow LLVM to do this because LLVM can see that the initialization of those bits to zero is dead anyway?
> > 
> > > I would zero the register before inserting a value into it.
> > 
> > Thanks for the feedback, I will do it!  To make sure you mean:
> > 
> > %1 = insertelement undef, %doubleValue, i32 0
> > %2 = insertelement %1, 0.0, i32 1
> > %3 = call @llvm.x86.sse2.cvttsd2si(%2)
> > 
> > Right?
> 
> The IR looks right. The assembly looks okay ?

Yup!  And it looked identical to what it was before.

Can you explain what the partial-register dependency problem is?

[Nadav Rotem]

> Can you explain what the partial-register dependency problem is?

Oops, I confused CVTTSD2SI with CVTTSI2SD. This code looks good. The the problem with the 2SD instruction is that it modifies a part of the XMM register. So the out-of-order engine can't execute the instruction whenever it wants because we need to wait for the upper part of the register to be calculated before we can execute the 2SD instruction.  By zeroing the upper part of the register we break this false dependency. Again, this does not apply to this instruction because we only read from the XMM register, not write into it.