[llvm-commits] [llvm] r146163 - in /llvm/trunk/lib/Target/X86: X86InstrFormats.td X86InstrInfo.td X86InstrSSE.td X86Subtarget.h

[Evan Cheng]

Duh. I completely missed it. Fixed it.

Evan

On Dec 8, 2011, at 5:12 PM, Bruno Cardoso Lopes wrote:

> Hi Evan,
> 
> On Thu, Dec 8, 2011 at 5:00 PM, Evan Cheng <evan.cheng at apple.com> wrote:
>> Author: evancheng
>> Date: Thu Dec  8 13:00:42 2011
>> New Revision: 146163
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=146163&view=rev
>> Log:
>> Many of the SSE patterns should not be selected when AVX is available. This led to the following code in X86Subtarget.cpp
>> 
>>  if (HasAVX)
>>    X86SSELevel = NoMMXSSE;
>> 
>> This is so patterns that are predicated on hasSSE3, etc. would not be selected when avx is available. Instead, the AVX variant is selected.
>> However, this breaks instructions which do not have AVX variants.
>> 
>> The right way to fix this is for the SSE but not-AVX patterns to predicate on something like hasSSE3() && !hasAVX().
>> Then we can take out the hack in X86Subtarget.cpp. Patterns which do not have AVX variants do not need to change.
>> 
>> However, we need to audit all the patterns before we make the change. This patch is workaround that fixes one specific case,
>> the prefetch instructions. rdar://10538297
>> 
>> Modified:
>>    llvm/trunk/lib/Target/X86/X86InstrFormats.td
>>    llvm/trunk/lib/Target/X86/X86InstrInfo.td
>>    llvm/trunk/lib/Target/X86/X86InstrSSE.td
>>    llvm/trunk/lib/Target/X86/X86Subtarget.h
>> 
>> Modified: llvm/trunk/lib/Target/X86/X86InstrFormats.td
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrFormats.td?rev=146163&r1=146162&r2=146163&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/X86InstrFormats.td (original)
>> +++ llvm/trunk/lib/Target/X86/X86InstrFormats.td Thu Dec  8 13:00:42 2011
>> @@ -334,6 +334,10 @@
>>            list<dag> pattern>
>>       : I<o, F, outs, ins, !strconcat("v", asm), pattern, SSEPackedSingle>, TB,
>>         Requires<[HasAVX]>;
>> +class VoPSI<bits<8> o, Format F, dag outs, dag ins, string asm,
>> +            list<dag> pattern>
>> +      : I<o, F, outs, ins, asm, pattern, SSEPackedSingle>, TB,
>> +        Requires<[HasSSE1orAVX]>;
>> 
>>  // SSE2 Instruction Templates:
>>  //
>> 
>> Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.td
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.td?rev=146163&r1=146162&r2=146163&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/X86InstrInfo.td (original)
>> +++ llvm/trunk/lib/Target/X86/X86InstrInfo.td Thu Dec  8 13:00:42 2011
>> @@ -476,6 +476,8 @@
>>  def HasXMM       : Predicate<"Subtarget->hasXMM()">;
>>  def HasXMMInt    : Predicate<"Subtarget->hasXMMInt()">;
>> 
>> +def HasSSE1orAVX : Predicate<"Subtarget->hasSSE1orAVX()">;
>> +
>>  def HasPOPCNT    : Predicate<"Subtarget->hasPOPCNT()">;
>>  def HasAES       : Predicate<"Subtarget->hasAES()">;
>>  def HasCLMUL     : Predicate<"Subtarget->hasCLMUL()">;
>> 
>> Modified: llvm/trunk/lib/Target/X86/X86InstrSSE.td
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrSSE.td?rev=146163&r1=146162&r2=146163&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/X86InstrSSE.td (original)
>> +++ llvm/trunk/lib/Target/X86/X86InstrSSE.td Thu Dec  8 13:00:42 2011
>> @@ -3183,13 +3183,13 @@
>>  //===----------------------------------------------------------------------===//
>> 
>>  // Prefetch intrinsic.
>> -def PREFETCHT0   : PSI<0x18, MRM1m, (outs), (ins i8mem:$src),
>> +def PREFETCHT0   : VoPSI<0x18, MRM1m, (outs), (ins i8mem:$src),
>>     "prefetcht0\t$src", [(prefetch addr:$src, imm, (i32 3), (i32 1))]>;
>> -def PREFETCHT1   : PSI<0x18, MRM2m, (outs), (ins i8mem:$src),
>> +def PREFETCHT1   : VoPSI<0x18, MRM2m, (outs), (ins i8mem:$src),
>>     "prefetcht1\t$src", [(prefetch addr:$src, imm, (i32 2), (i32 1))]>;
>> -def PREFETCHT2   : PSI<0x18, MRM3m, (outs), (ins i8mem:$src),
>> +def PREFETCHT2   : VoPSI<0x18, MRM3m, (outs), (ins i8mem:$src),
>>     "prefetcht2\t$src", [(prefetch addr:$src, imm, (i32 1), (i32 1))]>;
>> -def PREFETCHNTA  : PSI<0x18, MRM0m, (outs), (ins i8mem:$src),
>> +def PREFETCHNTA  : VoPSI<0x18, MRM0m, (outs), (ins i8mem:$src),
>>     "prefetchnta\t$src", [(prefetch addr:$src, imm, (i32 0), (i32 1))]>;
>> 
>>  // Flush cache
>> 
>> Modified: llvm/trunk/lib/Target/X86/X86Subtarget.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86Subtarget.h?rev=146163&r1=146162&r2=146163&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/X86Subtarget.h (original)
>> +++ llvm/trunk/lib/Target/X86/X86Subtarget.h Thu Dec  8 13:00:42 2011
>> @@ -193,6 +193,7 @@
>>   bool hasAVX2() const { return HasAVX2; }
>>   bool hasXMM() const { return hasSSE1() || hasAVX(); }
>>   bool hasXMMInt() const { return hasSSE2() || hasAVX(); }
>> +  bool hasSSE1orAVX() const { return hasSSE1() || hasAVX(); }
>>   bool hasSSE3orAVX() const { return hasSSE3() || hasAVX(); }
>>   bool hasSSSE3orAVX() const { return hasSSSE3() || hasAVX(); }
>>   bool hasSSE41orAVX() const { return hasSSE41() || hasAVX(); }
> 
> Why don't use hasXMM() instead of creating a hasSSE1orAVX()? That's
> how we usually check for this predicate all around the code. Perhaps
> it deserves some better naming though...
> 
> -- 
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc