[PATCH] D48768: [X86] When have BMI2, prefer shifts to clear low/high bits, rather than variable mask.

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

This looks like the right default for x86 based on size of the code and opportunities for other combines (and as noted, it's what we would have produced anyway until very recently), so LGTM.



================
Comment at: include/llvm/CodeGen/TargetLowering.h:515
+  /// Shifts:  x >> y << y
+  /// Different targets may have different preferences.
+  /// Returns true if the shift variant is preferred.
----------------
I don't think this line ("Different targets...") adds anything, so I'd remove it.


================
Comment at: include/llvm/CodeGen/TargetLowering.h:516
+  /// Different targets may have different preferences.
+  /// Returns true if the shift variant is preferred.
+  virtual bool preferShiftsToClearExtremeBits(SDValue X) const {
----------------
Clearer to say: "Return true if the variant with 2 shifts is preferred."


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4195-4204
+    switch (OuterShift = M->getOpcode()) {
+    case ISD::SHL:
+      InnerShift = ISD::SRL;
+      break;
+    case ISD::SRL:
+      InnerShift = ISD::SHL;
+      break;
----------------
It's a matter of taste, but I find it more readable/less code to use if+else or ?: for 2-case logic.


Repository:
  rL LLVM

https://reviews.llvm.org/D48768