Skip to content

Conversation

@dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Jun 8, 2024

The pr description in #94008 mismatches with the code.

  • When VT is smaller than ShiftVT, it is safe to use trunc.
  • When VT is larger than ShiftVT, it is safe to use zext iff
    is_zero_poison is true (i.e., opcode == ISD::CTTZ_ZERO_UNDEF). See
    also the counterexample src_shl_cttz2 -> tgt_shl_cttz2 in the alive2
    proofs.

Closes #94824.

@dtcxzyw dtcxzyw requested review from RKSimon, arsenm and topperc June 8, 2024 10:52
@llvmbot llvmbot added backend:X86 llvm:SelectionDAG SelectionDAGISel as well labels Jun 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 8, 2024

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-x86

Author: Yingwei Zheng (dtcxzyw)

Changes

The pr description in #94008 mismatches with the code.
> + When VT is smaller than ShiftVT, it is safe to use trunc.
> + When VT is larger than ShiftVT, it is safe to use zext iff
is_zero_poison is true (i.e., opcode == ISD::CTTZ_ZERO_UNDEF). See
also the counterexample src_shl_cttz2 -> tgt_shl_cttz2 in the alive2
proofs.

Closes #94824.


Full diff: https://github.com/llvm/llvm-project/pull/94850.diff

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+1-1)
  • (added) llvm/test/CodeGen/X86/pr94824.ll (+19)
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 9a5359015439e..1130a1ae20445 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -10117,7 +10117,7 @@ SDValue DAGCombiner::visitSHL(SDNode *N) {
   // fold (shl X, cttz(Y)) -> (mul (Y & -Y), X) if cttz is unsupported on the
   // target.
   if (((N1.getOpcode() == ISD::CTTZ &&
-        VT.getScalarSizeInBits() >= ShiftVT.getScalarSizeInBits()) ||
+        VT.getScalarSizeInBits() <= ShiftVT.getScalarSizeInBits()) ||
        N1.getOpcode() == ISD::CTTZ_ZERO_UNDEF) &&
       N1.hasOneUse() && !TLI.isOperationLegalOrCustom(ISD::CTTZ, ShiftVT) &&
       TLI.isOperationLegalOrCustom(ISD::MUL, VT)) {
diff --git a/llvm/test/CodeGen/X86/pr94824.ll b/llvm/test/CodeGen/X86/pr94824.ll
new file mode 100644
index 0000000000000..7744d00acf3d4
--- /dev/null
+++ b/llvm/test/CodeGen/X86/pr94824.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc < %s -mtriple=x86_64-linux-gnu | FileCheck %s
+
+define i16 @pr94824(i8 %x1) {
+; CHECK-LABEL: pr94824:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    orl $256, %edi # imm = 0x100
+; CHECK-NEXT:    rep bsfl %edi, %ecx
+; CHECK-NEXT:    movl $1, %eax
+; CHECK-NEXT:    # kill: def $cl killed $cl killed $ecx
+; CHECK-NEXT:    shll %cl, %eax
+; CHECK-NEXT:    # kill: def $ax killed $ax killed $eax
+; CHECK-NEXT:    retq
+entry:
+  %cttz = call i8 @llvm.cttz.i8(i8 %x1, i1 false)
+  %ext = zext i8 %cttz to i16
+  %shl = shl i16 1, %ext
+  ret i16 %shl
+}

@dtcxzyw dtcxzyw merged commit d9507a3 into llvm:main Jun 8, 2024
@dtcxzyw dtcxzyw deleted the fix-pr94824 branch June 8, 2024 13:40
@HerrCai0907 HerrCai0907 mentioned this pull request Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:X86 llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Miscompile of 1 << ZEXT(CTTZ(X))

3 participants