[llvm] [llvm][gvn-sink] Don't try to sink inline asm (PR #138414)

[Paul Kirth via llvm-commits]

https://github.com/ilovepi updated https://github.com/llvm/llvm-project/pull/138414

>From e680ddb6ce28e9e831438fc47673b8c89462b231 Mon Sep 17 00:00:00 2001
From: Paul Kirth <paulkirth at google.com>
Date: Sat, 3 May 2025 16:55:30 +0000
Subject: [PATCH] [llvm][gvn-sink] Don't try to sink inline asm

Fixes #138345. Before this patch, gvn-sink would try to sink inline
assembly statements. Other GVN passes avoid them (see
https://github.com/llvm/llvm-project/blob/b4fac94181c4cf17dbb7ecc2ae975712b0e4a6d1/llvm/lib/Transforms/Scalar/GVN.cpp#L2932
Similarly, gvn-sink should skip these instructions, since they are not
safe to move. We also update the early exit in
canReplaceOperandWithVariable, since it should have caught this case.
We keep the new logic in GVNSink.cpp, since its more efficient to skip
numbering those instructions.

The test added is reduced from a failure when compiling Fuchsia. There
were two distinct failure modes. One occurred when only running gvn-sink
alone, but a different failure happened when running
correlated-propagation afterwards.
---
 llvm/lib/Transforms/Scalar/GVNSink.cpp   |  3 ++
 llvm/lib/Transforms/Utils/Local.cpp      |  5 ++-
 llvm/test/Transforms/GVNSink/pr138345.ll | 46 ++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/Transforms/GVNSink/pr138345.ll

diff --git a/llvm/lib/Transforms/Scalar/GVNSink.cpp b/llvm/lib/Transforms/Scalar/GVNSink.cpp
index 2058df33ea331..40f7a61cb5717 100644
--- a/llvm/lib/Transforms/Scalar/GVNSink.cpp
+++ b/llvm/lib/Transforms/Scalar/GVNSink.cpp
@@ -548,6 +548,9 @@ class GVNSink {
     if (isa<PHINode>(I) || I->isEHPad() || isa<AllocaInst>(I) ||
         I->getType()->isTokenTy())
       return true;
+    // Inline asm can't be sunk either.
+    if (auto *CB = dyn_cast<CallBase>(I); CB->isInlineAsm())
+      return true;
     return false;
   }
 
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index b3204da93049b..4ff4f30ddd3d5 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -4220,8 +4220,11 @@ bool llvm::canReplaceOperandWithVariable(const Instruction *I, unsigned OpIdx) {
     return false;
 
   // Early exit.
-  if (!isa<Constant>(I->getOperand(OpIdx)))
+  if (!isa<Constant>(I->getOperand(OpIdx))) {
+    if(const auto *CB = dyn_cast<CallBase>(I))
+      return !CB->isInlineAsm();
     return true;
+  }
 
   switch (I->getOpcode()) {
   default:
diff --git a/llvm/test/Transforms/GVNSink/pr138345.ll b/llvm/test/Transforms/GVNSink/pr138345.ll
new file mode 100644
index 0000000000000..7a29d25d84eeb
--- /dev/null
+++ b/llvm/test/Transforms/GVNSink/pr138345.ll
@@ -0,0 +1,46 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN:  opt -passes="gvn-sink" -S %s | FileCheck %s
+
+;; See https://github.com/llvm/llvm-project/issues/138345 for details.
+;; The program below used to crash due to taking the address of the inline asm.
+;; gvn-sink shouldn't do anything in this case, so test that the pass no longer
+;; generates invalid IR and no longer crashes.
+
+define void @c(i64 %num, ptr %ptr) {
+; CHECK-LABEL: define void @c(
+; CHECK-SAME: i64 [[NUM:%.*]], ptr [[PTR:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    switch i64 [[NUM]], label %[[SW_EPILOG:.*]] [
+; CHECK-NEXT:      i64 1, label %[[SW_BB:.*]]
+; CHECK-NEXT:      i64 0, label %[[SW_BB1:.*]]
+; CHECK-NEXT:    ]
+; CHECK:       [[SW_BB]]:
+; CHECK-NEXT:    [[TMP1:%.*]] = load i8, ptr [[PTR]], align 1
+; CHECK-NEXT:    call void asm sideeffect "", "r,r,~{dirflag},~{fpsr},~{flags}"(i8 [[TMP1]], ptr @c)
+; CHECK-NEXT:    br label %[[SW_EPILOG]]
+; CHECK:       [[SW_BB1]]:
+; CHECK-NEXT:    [[TMP2:%.*]] = load i8, ptr [[PTR]], align 1
+; CHECK-NEXT:    call void asm sideeffect "movdqu 0 [[XMM0:%.*]] \0A\09", "r,r,~{dirflag},~{fpsr},~{flags}"(i8 [[TMP2]], ptr @c)
+; CHECK-NEXT:    br label %[[SW_EPILOG]]
+; CHECK:       [[SW_EPILOG]]:
+; CHECK-NEXT:    ret void
+;
+entry:
+  switch i64 %num, label %sw.epilog [
+  i64 1, label %sw.bb
+  i64 0, label %sw.bb1
+  ]
+
+sw.bb:                                            ; preds = %entry
+  %1 = load i8, ptr %ptr, align 1
+  call void asm sideeffect "", "r,r,~{dirflag},~{fpsr},~{flags}"(i8 %1, ptr @c)
+  br label %sw.epilog
+
+sw.bb1:                                           ; preds = %entry
+  %2 = load i8, ptr %ptr, align 1
+  call void asm sideeffect "movdqu 0 %xmm0           \0A\09", "r,r,~{dirflag},~{fpsr},~{flags}"(i8 %2, ptr @c)
+  br label %sw.epilog
+
+sw.epilog:                                        ; preds = %sw.bb1, %sw.bb, %entry
+  ret void
+}