[PATCH] D17289: [X86] Fix False Data Dependency in popcnt

[Asaf Badouh via llvm-commits]

AsafBadouh added inline comments.

================
Comment at: ../llvm/lib/Target/X86/X86PopcntOpt.cpp:14
@@ +13,3 @@
+// instruction on its destination register.
+// The WA is to insert xor before the popcnt so it will remove the dependency.
+//
----------------
majnemer wrote:
> WA?
workaround, will change it.

================
Comment at: ../llvm/lib/Target/X86/X86PopcntOpt.cpp:68
@@ +67,3 @@
+  const X86Subtarget &ST = MF.getSubtarget<X86Subtarget>();
+  if (!ST.hasAVX() || !ST.hasPOPCNT())
+    return false;
----------------
mkuper wrote:
> majnemer wrote:
> > We don't do this dependency breaking if we `hasPOPCNT` but not `hasAVX` ?  I'd think we'd do this if `hasPOPCNT` is true regardless of what `hasAVX` is because we might want to run our program on machines older and newer than Sandy Bridge.
> This isn't like the other false dependency fixes we have, in the sense that this not an arch issue in the instruction definition, but rather a micro-arch bug. It doesn't exist in anything older than <arch-A>, and is supposed to be fixed in <arch-B> and above.
> 
> I think <arch-B> is Skylake, although I'm not entirely sure. Don't know what <arch-A> is. In any case, the point is - we do want a condition that's more complicated than hasPOPCNT(), I just don't know what it is.
As Michael explained, Sandy-bridge and later arch have that dependency.
hasAVX flag return true for Sandy-bridge and later.

Michael, what do you mean in "more complicated"?



Repository:
  rL LLVM

http://reviews.llvm.org/D17289