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

[Bruno Cardoso Lopes]

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