[llvm-commits] [llvm] r63495 - in /llvm/trunk: lib/Target/X86/X86CallingConv.td lib/Target/X86/X86ISelLowering.cpp lib/Target/X86/X86Subtarget.cpp test/CodeGen/X86/2009-01-25-NoSSE.ll test/CodeGen/X86/nosse-varargs.ll

[Dan Gohman]

On Feb 2, 2009, at 1:57 PM, Török Edwin wrote:

> On 2009-02-02 23:43, Dan Gohman wrote:
>> On Feb 2, 2009, at 12:55 PM, Török Edwin wrote:
>>
>>> +    if (Is64Bit) {
>>> +      // Make sure SSE2  is enabled, it is available on all X86-64
>>> CPUs.
>>> +      X86SSELevel = SSE2;
>>> +    }
>>>
>>
>> It looks like this should also check if (X86SSELevel < SSE2) so that
>> it doesn't override when AutoDetectSubtargetFeatures detects an SSE
>> level greater than SSE2. With that fixed, this looks good.
>>
>
> Fixed.

Thanks!

>
>
>>
>>>  }
>>>
>>> -  // If requesting codegen for X86-64, make sure that 64-bit and  
>>> SSE2
>>> features
>>> -  // are enabled.  These are available on all x86-64 CPUs.
>>> +  // If requesting codegen for X86-64, make sure that 64-bit  
>>> features
>>> +  // are enabled.
>>>  if (Is64Bit) {
>>>    HasX86_64 = true;
>>>  }
>>>
>>
>>
>> Do you think it would make sense to move this up into the else clause
>> above too? I'm wondering if an assert(!Is64Bit || HasX86_64) after  
>> the
>> else would be a sane sanity check.
>>
>
> I tried that, but the assertion failed with llc -mcpu=yonah in one of
> the codegen tests, so I didn't move the if for now (added the assert
> though).
> I've committed r63552.


Thanks. Was the failure you saw test/CodeGen/X86/extractelement-load.ll?
That's actually a bug in the test, and I just fixed it. I'll finish up
moving the above code.

Dan