[PATCH] D48765: [X86] The TEST instruction is eliminated when BSF/TZCNT is used

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

We need tests cases that use tzcnt too.



================
Comment at: lib/Target/X86/X86ISelLowering.cpp:33435
+       FalseOp.getOpcode() == ISD::ADD)) {
+    auto *Cnst = CC == X86::COND_E ? dyn_cast<ConstantSDNode>(TrueOp)
+                                   : dyn_cast<ConstantSDNode>(FalseOp);
----------------
You already checked that it was definitely a constant above. So you shouldn't need a dyn_cast here.

Or you should just check the CC in the first if, then select your Add and your possible constant. And check that it is an Add and a constant.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:33437
+                                   : dyn_cast<ConstantSDNode>(FalseOp);
+    SDValue Add = CC == X86::COND_E ? FalseOp : TrueOp;
+    auto *AddOp1 = dyn_cast<ConstantSDNode>(Add.getOperand(1));
----------------
Probably should make sure the add only has one user. Otherwise you're increasing code size.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:33443
+        (AddOp2.getOpcode() == ISD::CTTZ_ZERO_UNDEF ||
+         AddOp2.getOpcode() == ISD::CTTZ)) {
+      APInt Diff = Cnst->getAPIntValue() - AddOp1->getAPIntValue();
----------------
The test cases only cover the CTTZ_ZERO_UNDEF version right?


Repository:
  rL LLVM

https://reviews.llvm.org/D48765