[llvm] [Reassociate] Invalidate analysis passes after canonicalizeOperands (PR #136835)

[Björn Pettersson via llvm-commits]

https://github.com/bjope created https://github.com/llvm/llvm-project/pull/136835

When ranking operands for an expression tree the reassociate pass also perform canonicalization, putting constants on the right hand side. Such transforms was however not registered as modifying the IR. So at the end of the pass, if not having made any other changes, the pass returned that all analyses should be kept.

With this patch we make sure to set MadeChange to true when modifying the IR via canonicalizeOperands. This is to make sure analyses such as DemandedBits are properly invalidated when instructions are modified.

>From 8d2e11caf4a717b308f15c162efd966e48103199 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Wed, 23 Apr 2025 11:18:14 +0200
Subject: [PATCH] [Reassociate] Invalidate analysis passes after
 canonicalizeOperands

When ranking operands for an expression tree the reassociate pass
also perform canonicalization, putting constants on the right
hand side. Such transforms was however not registered as modifying
the IR. So at the end of the pass, if not having made any other
changes, the pass returned that all analyses should be kept.

With this patch we make sure to set MadeChange to true when
modifying the IR via canonicalizeOperands. This is to make sure
analyses such as DemandedBits are properly invalidated when
instructions are modified.
---
 llvm/lib/Transforms/Scalar/Reassociate.cpp    |  4 ++-
 .../Reassociate/canonicalize-made-change.ll   | 32 +++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/Transforms/Reassociate/canonicalize-made-change.ll

diff --git a/llvm/lib/Transforms/Scalar/Reassociate.cpp b/llvm/lib/Transforms/Scalar/Reassociate.cpp
index 90b891ac87a18..8e478b37574f6 100644
--- a/llvm/lib/Transforms/Scalar/Reassociate.cpp
+++ b/llvm/lib/Transforms/Scalar/Reassociate.cpp
@@ -241,8 +241,10 @@ void ReassociatePass::canonicalizeOperands(Instruction *I) {
   Value *RHS = I->getOperand(1);
   if (LHS == RHS || isa<Constant>(RHS))
     return;
-  if (isa<Constant>(LHS) || getRank(RHS) < getRank(LHS))
+  if (isa<Constant>(LHS) || getRank(RHS) < getRank(LHS)) {
     cast<BinaryOperator>(I)->swapOperands();
+    MadeChange = true;
+  }
 }
 
 static BinaryOperator *CreateAdd(Value *S1, Value *S2, const Twine &Name,
diff --git a/llvm/test/Transforms/Reassociate/canonicalize-made-change.ll b/llvm/test/Transforms/Reassociate/canonicalize-made-change.ll
new file mode 100644
index 0000000000000..64f1ede9a9ac4
--- /dev/null
+++ b/llvm/test/Transforms/Reassociate/canonicalize-made-change.ll
@@ -0,0 +1,32 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -passes="print<demanded-bits>,reassociate,bdce" -S < %s | FileCheck %s
+
+; We want to verify that demanded-bits analysis is invalidated when
+; reassociate is canonicalizing expressions (e.g. putting the constant on the
+; RHS of an OR).
+;
+; Printing demanded-bits will make sure a demanded-bits analysis is cached.
+; Then we run reassociate, followed by bdce. When not invalidating demanded-bits
+; while doing reassociation of the OR, we got this kind of error:
+;
+;   Running pass: BDCEPass on foo (4 instructions)
+;   While deleting: i1 %cmp1
+;   Use still stuck around after Def is destroyed:  %or = or i1 %cmp1, true
+;   UNREACHABLE executed at ../lib/IR/Value.cpp:102!
+;
+; Check that we get the expected result without failing on assert/unreachable.
+
+define i1 @foo(i1 %c) {
+; CHECK-LABEL: define i1 @foo(
+; CHECK-SAME: i1 [[C:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[OR:%.*]] = or i1 false, true
+; CHECK-NEXT:    [[AND:%.*]] = and i1 [[OR]], [[C]]
+; CHECK-NEXT:    ret i1 [[AND]]
+;
+entry:
+  %cmp = icmp ne i16 0, 1
+  %or = or i1 true, %cmp
+  %and = and i1 %c, %or
+  ret i1 %and
+}