[llvm] [SDAG] Intersect poison-generating flags after CSE (PR #97434)

Yingwei Zheng via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 2 09:28:28 PDT 2024


https://github.com/dtcxzyw created https://github.com/llvm/llvm-project/pull/97434

This patch fixes a miscompilation when `N` gets CSEed to `Existing`:
```
Existing: t5: i32 = sub nuw Constant:i32<0>, t3
N: t30: i32 = sub Constant:i32<0>, t3
```

An alternative is to block CSE by taking SDFlags into account in `AddNodeIDNode`. It will enable more folds (e.g., `sub nuw 0, X -> 0` in this case).

Fixes https://github.com/llvm/llvm-project/issues/96366.


>From a9198b8a06ff0393e0b1cbfcecdc876925c0d239 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng <dtcxzyw2333 at gmail.com>
Date: Tue, 2 Jul 2024 23:36:35 +0800
Subject: [PATCH 1/2] [SDAG] Add pre-commit tests from PR96366. NFC.

---
 llvm/test/CodeGen/AArch64/pr96366.ll | 23 +++++++++++++++++++++++
 llvm/test/CodeGen/RISCV/pr96366.ll   | 25 +++++++++++++++++++++++++
 2 files changed, 48 insertions(+)
 create mode 100644 llvm/test/CodeGen/AArch64/pr96366.ll
 create mode 100644 llvm/test/CodeGen/RISCV/pr96366.ll

diff --git a/llvm/test/CodeGen/AArch64/pr96366.ll b/llvm/test/CodeGen/AArch64/pr96366.ll
new file mode 100644
index 0000000000000..fd13197ca396f
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/pr96366.ll
@@ -0,0 +1,23 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc < %s -mtriple=aarch64 | FileCheck %s
+
+declare void @mumble(i32)
+
+define i32 @f(i32 %0) nounwind {
+; CHECK-LABEL: f:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    str x30, [sp, #-16]! // 8-byte Folded Spill
+; CHECK-NEXT:    mov w0, wzr
+; CHECK-NEXT:    bl mumble
+; CHECK-NEXT:    mov w0, #4 // =0x4
+; CHECK-NEXT:    ldr x30, [sp], #16 // 8-byte Folded Reload
+; CHECK-NEXT:    ret
+  %2 = sub nuw i32 0, %0
+  call void @mumble(i32 %2)
+  %3 = sub i32 1, %0
+  %4 = sub i32 3, %0
+  %5 = mul i32 %0, 1
+  %6 = add i32 %3, %5
+  %7 = add i32 %6, %4
+  ret i32 %7
+}
diff --git a/llvm/test/CodeGen/RISCV/pr96366.ll b/llvm/test/CodeGen/RISCV/pr96366.ll
new file mode 100644
index 0000000000000..712d07b3599e0
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/pr96366.ll
@@ -0,0 +1,25 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc < %s -mtriple=riscv64 | FileCheck %s
+
+declare void @mumble(i32)
+
+define i32 @f(i32 %0) nounwind {
+; CHECK-LABEL: f:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    addi sp, sp, -16
+; CHECK-NEXT:    sd ra, 8(sp) # 8-byte Folded Spill
+; CHECK-NEXT:    li a0, 0
+; CHECK-NEXT:    call mumble
+; CHECK-NEXT:    li a0, 4
+; CHECK-NEXT:    ld ra, 8(sp) # 8-byte Folded Reload
+; CHECK-NEXT:    addi sp, sp, 16
+; CHECK-NEXT:    ret
+  %2 = sub nuw i32 0, %0
+  call void @mumble(i32 %2)
+  %3 = sub i32 1, %0
+  %4 = sub i32 3, %0
+  %5 = mul i32 %0, 1
+  %6 = add i32 %3, %5
+  %7 = add i32 %6, %4
+  ret i32 %7
+}

>From 10d216a51ed8eff6a9c57f3a5907853407090aa4 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng <dtcxzyw2333 at gmail.com>
Date: Tue, 2 Jul 2024 23:44:16 +0800
Subject: [PATCH 2/2] [SDAG] Intersect poison-generating flags after CSE

---
 llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp |  1 +
 llvm/test/CodeGen/AArch64/pr96366.ll           | 10 ++++++----
 llvm/test/CodeGen/RISCV/pr96366.ll             |  6 +++++-
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index bc16f885f6a04..96242305e9eab 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -1239,6 +1239,7 @@ SelectionDAG::AddModifiedNodeToCSEMaps(SDNode *N) {
       // If there was already an existing matching node, use ReplaceAllUsesWith
       // to replace the dead one with the existing one.  This can cause
       // recursive merging of other unrelated nodes down the line.
+      Existing->intersectFlagsWith(N->getFlags());
       ReplaceAllUsesWith(N, Existing);
 
       // N is now dead. Inform the listeners and delete it.
diff --git a/llvm/test/CodeGen/AArch64/pr96366.ll b/llvm/test/CodeGen/AArch64/pr96366.ll
index fd13197ca396f..392b7b66b0bb1 100644
--- a/llvm/test/CodeGen/AArch64/pr96366.ll
+++ b/llvm/test/CodeGen/AArch64/pr96366.ll
@@ -6,11 +6,13 @@ declare void @mumble(i32)
 define i32 @f(i32 %0) nounwind {
 ; CHECK-LABEL: f:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    str x30, [sp, #-16]! // 8-byte Folded Spill
-; CHECK-NEXT:    mov w0, wzr
+; CHECK-NEXT:    stp x30, x19, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:    mov w19, w0
+; CHECK-NEXT:    neg w0, w0
 ; CHECK-NEXT:    bl mumble
-; CHECK-NEXT:    mov w0, #4 // =0x4
-; CHECK-NEXT:    ldr x30, [sp], #16 // 8-byte Folded Reload
+; CHECK-NEXT:    mov w8, #4 // =0x4
+; CHECK-NEXT:    sub w0, w8, w19
+; CHECK-NEXT:    ldp x30, x19, [sp], #16 // 16-byte Folded Reload
 ; CHECK-NEXT:    ret
   %2 = sub nuw i32 0, %0
   call void @mumble(i32 %2)
diff --git a/llvm/test/CodeGen/RISCV/pr96366.ll b/llvm/test/CodeGen/RISCV/pr96366.ll
index 712d07b3599e0..05563d7bc14af 100644
--- a/llvm/test/CodeGen/RISCV/pr96366.ll
+++ b/llvm/test/CodeGen/RISCV/pr96366.ll
@@ -8,10 +8,14 @@ define i32 @f(i32 %0) nounwind {
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    addi sp, sp, -16
 ; CHECK-NEXT:    sd ra, 8(sp) # 8-byte Folded Spill
-; CHECK-NEXT:    li a0, 0
+; CHECK-NEXT:    sd s0, 0(sp) # 8-byte Folded Spill
+; CHECK-NEXT:    mv s0, a0
+; CHECK-NEXT:    negw a0, a0
 ; CHECK-NEXT:    call mumble
 ; CHECK-NEXT:    li a0, 4
+; CHECK-NEXT:    subw a0, a0, s0
 ; CHECK-NEXT:    ld ra, 8(sp) # 8-byte Folded Reload
+; CHECK-NEXT:    ld s0, 0(sp) # 8-byte Folded Reload
 ; CHECK-NEXT:    addi sp, sp, 16
 ; CHECK-NEXT:    ret
   %2 = sub nuw i32 0, %0



More information about the llvm-commits mailing list