Skip to content

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Sep 22, 2025

This code was already creating HandleSDNodes to handle the case where a node gets replaced with an equivalent node. However, the code before the handles are created also performs RAUW operations, which can end up CSEing and deleting nodes.

Fix this issue by moving the handle creation earlier.

Fixes #160040.

This code was already creating HandleSDNodes to handle the case
where a node gets replaced with an equivalent node. However, the
code before the handles are created also performs RAUW operations,
which can end up CSEing and deleting nodes.

Fix this issue by moving the handle creation earlier.
@llvmbot
Copy link
Member

llvmbot commented Sep 22, 2025

@llvm/pr-subscribers-backend-powerpc

Author: Nikita Popov (nikic)

Changes

This code was already creating HandleSDNodes to handle the case where a node gets replaced with an equivalent node. However, the code before the handles are created also performs RAUW operations, which can end up CSEing and deleting nodes.

Fix this issue by moving the handle creation earlier.


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

2 Files Affected:

  • (modified) llvm/lib/Target/PowerPC/PPCISelLowering.cpp (+6-4)
  • (added) llvm/test/CodeGen/PowerPC/pr160040.ll (+24)
diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
index fa104e4f69d7f..47e107d11f71b 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -15407,6 +15407,12 @@ SDValue PPCTargetLowering::DAGCombineExtBoolTrunc(SDNode *N,
     }
   }
 
+  // Convert PromOps to handles before doing any RAUW operations, as these
+  // may CSE with existing nodes, deleting the originals.
+  std::list<HandleSDNode> PromOpHandles;
+  for (auto &PromOp : PromOps)
+    PromOpHandles.emplace_back(PromOp);
+
   // Replace all inputs, either with the truncation operand, or a
   // truncation or extension to the final output type.
   for (unsigned i = 0, ie = Inputs.size(); i != ie; ++i) {
@@ -15430,10 +15436,6 @@ SDValue PPCTargetLowering::DAGCombineExtBoolTrunc(SDNode *N,
         DAG.getAnyExtOrTrunc(InSrc, dl, N->getValueType(0)));
   }
 
-  std::list<HandleSDNode> PromOpHandles;
-  for (auto &PromOp : PromOps)
-    PromOpHandles.emplace_back(PromOp);
-
   // Replace all operations (these are all the same, but have a different
   // (promoted) return type). DAG.getNode will validate that the types of
   // a binary operator match, so go through the list in reverse so that
diff --git a/llvm/test/CodeGen/PowerPC/pr160040.ll b/llvm/test/CodeGen/PowerPC/pr160040.ll
new file mode 100644
index 0000000000000..865239b37112c
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/pr160040.ll
@@ -0,0 +1,24 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: llc -mtriple=powerpc64le-unknown-linux-gnu < %s | FileCheck %s
+
+; Make sure this does not crash.
+define i32 @test(i32 %arg) {
+; CHECK-LABEL: test:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    rlwinm 4, 3, 13, 19, 19
+; CHECK-NEXT:    rlwinm 3, 3, 2, 30, 30
+; CHECK-NEXT:    xori 4, 4, 4096
+; CHECK-NEXT:    xori 3, 3, 2
+; CHECK-NEXT:    rlwimi 3, 4, 0, 31, 29
+; CHECK-NEXT:    blr
+  %icmp = icmp sgt i32 %arg, -1
+  %select = select i1 %icmp, i16 1, i16 0
+  %select1 = select i1 %icmp, i16 16384, i16 0
+  %lshr = lshr i16 %select1, 1
+  %zext = zext i16 %lshr to i32
+  %lshr2 = lshr i32 %zext, 1
+  %shl = shl i16 %select, 1
+  %zext3 = zext i16 %shl to i32
+  %or = or i32 %lshr2, %zext3
+  ret i32 %or
+}

Copy link
Collaborator

@RolandF77 RolandF77 left a comment

Choose a reason for hiding this comment

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

LGTM

@nikic nikic merged commit 8b824f3 into llvm:main Sep 22, 2025
11 checks passed
@nikic nikic deleted the ppc-combine-fix branch September 22, 2025 19:37
@nikic nikic added this to the LLVM 21.x Release milestone Sep 22, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Sep 22, 2025
@nikic
Copy link
Contributor Author

nikic commented Sep 22, 2025

/cherry-pick 8b824f3

@llvmbot
Copy link
Member

llvmbot commented Sep 22, 2025

/pull-request #160187

@llvmbot llvmbot moved this from Needs Triage to Done in LLVM Release Status Sep 22, 2025
swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Sep 23, 2025
…vm#160050)

This code was already creating HandleSDNodes to handle the case where a
node gets replaced with an equivalent node. However, the code before the
handles are created also performs RAUW operations, which can end up
CSEing and deleting nodes.

Fix this issue by moving the handle creation earlier.

Fixes llvm#160040.

(cherry picked from commit 8b824f3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

[PowerPC] Assertion failure in DAGCombineExtBoolTrunc
3 participants