Skip to content

Conversation

@topperc
Copy link
Collaborator

@topperc topperc commented May 29, 2025

If opaque constants are involved we can have an AND with 2 constant
operands that hasn't been simplified. If this is the case, we need
to modify at least one of the constants if it is out of range.

Fixes #142004

topperc added 2 commits May 29, 2025 16:18
…t operands.

If opaque constants are involved we can have an AND with 2 constant
operands that hasn't been simplified. If this is the case, we need
to modify at least one of the constants if it is out of range.

Fixes llvm#142004
@topperc topperc requested review from RKSimon and dtcxzyw May 29, 2025 23:19
@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label May 29, 2025
@llvmbot
Copy link
Member

llvmbot commented May 29, 2025

@llvm/pr-subscribers-llvm-selectiondag

Author: Craig Topper (topperc)

Changes

If opaque constants are involved we can have an AND with 2 constant
operands that hasn't been simplified. If this is the case, we need
to modify at least one of the constants if it is out of range.

Fixes #142004


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+9-2)
  • (added) llvm/test/CodeGen/RISCV/pr142004.ll (+33)
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index e05f85ea3bd8e..fe5310aff6c73 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -6812,7 +6812,8 @@ bool DAGCombiner::SearchForAndLoads(SDNode *N,
 
     // Some constants may need fixing up later if they are too large.
     if (auto *C = dyn_cast<ConstantSDNode>(Op)) {
-      if ((N->getOpcode() == ISD::OR || N->getOpcode() == ISD::XOR) &&
+      if ((N->getOpcode() == ISD::OR || N->getOpcode() == ISD::XOR ||
+           N->getOpcode() == ISD::AND) &&
           (Mask->getAPIntValue() & C->getAPIntValue()) != C->getAPIntValue())
         NodesWithConsts.insert(N);
       continue;
@@ -6927,7 +6928,13 @@ bool DAGCombiner::BackwardsPropagateMask(SDNode *N) {
       SDValue Op0 = LogicN->getOperand(0);
       SDValue Op1 = LogicN->getOperand(1);
 
-      if (isa<ConstantSDNode>(Op0))
+      // We only need to fix AND if both inputs are constants. And we only need
+      // to fix one of the constants.
+      if (LogicN->getOpcode() == ISD::AND &&
+          (!isa<ConstantSDNode>(Op0) || !isa<ConstantSDNode>(Op1)))
+        continue;
+
+      if (isa<ConstantSDNode>(Op0) && LogicN->getOpcode() != ISD::AND)
         Op0 =
             DAG.getNode(ISD::AND, SDLoc(Op0), Op0.getValueType(), Op0, MaskOp);
 
diff --git a/llvm/test/CodeGen/RISCV/pr142004.ll b/llvm/test/CodeGen/RISCV/pr142004.ll
new file mode 100644
index 0000000000000..709644e49e704
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/pr142004.ll
@@ -0,0 +1,33 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc < %s -mtriple=riscv64 | FileCheck %s
+
+@f = global i64 0, align 8
+@d = global i64 0, align 8
+@e = global i32 0, align 8
+
+define i32 @foo(i32 %x) {
+; CHECK-LABEL: foo:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    lui a1, %hi(f)
+; CHECK-NEXT:    lui a2, %hi(d)
+; CHECK-NEXT:    lbu a1, %lo(f)(a1)
+; CHECK-NEXT:    lhu a2, %lo(d)(a2)
+; CHECK-NEXT:    slli a0, a0, 48
+; CHECK-NEXT:    srli a3, a0, 48
+; CHECK-NEXT:    xori a0, a1, 255
+; CHECK-NEXT:    or a0, a0, a2
+; CHECK-NEXT:    lui a1, %hi(e)
+; CHECK-NEXT:    sw a3, %lo(e)(a1)
+; CHECK-NEXT:    ret
+entry:
+  %1 = load i64, ptr @f, align 8
+  %conv1 = and i64 %1, 255
+  %conv2 = xor i64 %conv1, 255
+  %2 = load i64, ptr @d, align 8
+  %or = or i64 %conv2, %2
+  %conv3 = trunc i64 %or to i32
+  %conv4 = and i32 %conv3, 65535
+  %and = and i32 %x, 65535
+  store i32 %and, ptr @e
+  ret i32 %conv4
+}

if (auto *C = dyn_cast<ConstantSDNode>(Op)) {
if ((N->getOpcode() == ISD::OR || N->getOpcode() == ISD::XOR) &&
if ((N->getOpcode() == ISD::OR || N->getOpcode() == ISD::XOR ||
N->getOpcode() == ISD::AND) &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ISD::isBitwiseLogicOp(N->getOpcode())

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG w/ comment fixed.

if (auto *C = dyn_cast<ConstantSDNode>(Op)) {
if ((N->getOpcode() == ISD::OR || N->getOpcode() == ISD::XOR) &&
if ((N->getOpcode() == ISD::OR || N->getOpcode() == ISD::XOR ||
N->getOpcode() == ISD::AND) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@topperc topperc merged commit b4b3be7 into llvm:main May 30, 2025
11 checks passed
@topperc topperc deleted the pr/142004 branch May 30, 2025 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RISC-V] Miscompile on -O[1-3]

4 participants