[PATCH] D12136: [X86] Emit more efficient >= comparisons against 0

Mitch Bodart via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 19 10:28:46 PDT 2015


mbodart added inline comments.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:24078
@@ +24077,3 @@
+static SDValue foldXorTruncShiftIntoCmp(SDNode *N, SelectionDAG &DAG) {
+  // This is only worth doing if the output type is i8.
+  if (N->getValueType(0) != MVT::i8)
----------------
majnemer wrote:
> mbodart wrote:
> > I would think we also want to suppress this if the xor has more than one user.
> It might be a little early for me (no coffee, etc.), why is it undesirable to use a `test`/`setns` sequence if there is more than one user?
More than one user likley means the xor is still live.

So the transformation would just add more instructions, without removing any.

I'm a bit new to LLVM, so maybe there's some larger context I'm not seeing.
If so, please shed some light.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:24113
@@ +24112,3 @@
+  SDValue Cond = DAG.getSetCC(DL, MVT::i8, ShiftOp,
+                              DAG.getConstant(-1, DL, ShiftOpTy), ISD::SETGT);
+  return Cond;
----------------
majnemer wrote:
> mbodart wrote:
> > This is fine.  But just curious, is there a reason you chose SETGT -1 over SETGE 0?
> We seem to normalize it to that: http://llvm.org/docs/doxygen/html/X86ISelLowering_8cpp_source.html#l03732
> 
> Using SETGE 0 works equally well but results in a `test`/`setge` sequence.  The sequence is equivalent but also requires remembering that `test` sets OF to 0.
OK, thanks for the explanation.


http://reviews.llvm.org/D12136





More information about the llvm-commits mailing list