[PATCH] D67087: [X86] Override BuildSDIVPow2 for X86.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

Thinking about this sequence in relation to some other folds: if the add was pulled through the select operands, we'd have a select-of-constants where the false op is a zero, and that would get reduced to a 'shift+and' and defeat the goal of this patch. So if cmov is the better option for this case, it's possible that we are doing the wrong transform in some other cases.

Another consideration: if we want to avoid having the 'select' getting squashed by some other transform, then using the x86-specific opcode should ensure that (that might be why AArch chose to go with target-specific opcodes?). Having the tests here (and as usual, we should have the baseline versions committed 1st) should signal if someone changes the behavior, but it might be worth leaving a TODO comment.



================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:20110
+  // Make sure the division is a power of 2 or the negation of a power of 2.
+  if (!(Divisor.isPowerOf2() || (-Divisor).isPowerOf2()))
+    return SDValue();
----------------
Does the caller always check this anyway? If so, could just make it an assert.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67087/new/

https://reviews.llvm.org/D67087