[llvm-commits] [llvm] r91672 - in /llvm/trunk: lib/Target/X86/X86.td lib/Target/X86/X86InstrInfo.cpp lib/Target/X86/X86InstrInfo.td lib/Target/X86/X86InstrSSE.td lib/Target/X86/X86Subtarget.cpp lib/Target/X86/X86Subtarget.h test/CodeGen/X86/break-sse-dep.ll

[Evan Cheng]

On Dec 21, 2009, at 10:47 AM, Chris Lattner wrote:

> 
> On Dec 21, 2009, at 10:45 AM, Evan Cheng wrote:
> 
>>>> 
>>>> +def FeatureBreakSSEDep : SubtargetFeature<"break-sse-dep", "BreakSSEDep","true",
>>>> +                       "Should break SSE partial update dep with load / xorps">;
>>> 
>>> Do we really need a feature for this?  Is this actually good for any SSE chip?
>> 
>> It's not clear. It's known to be an issue since CoreDuo. It's possible it will change for future u-arch so I'd leave it as a subtarget feature for now.
> 
> Unless there is a reason to have this, I'd prefer to not have it clutter up the td files.  I can't imagine a reasonable (non-scalarizing) implementation of SSE that wouldn't have this issue.

Really? It's a big surprise to Dan and I (and the engineer who noticed this) that unfolding the load actually breaks the register dependency. It's not documented anywhere in the public Intel manual.

> 
> For example, you didn't add this flag to any of the AMD chips.

That's intentional. I don't have a AMD machine to try it on and I did not want to introduce a regression. 

I don't really care that much whether it's a subtarget feature. If no one pipes up soon, I'll remove it.

Evan

> 
> -Chris