release/20.x: Do not treat llvm.fake.use as a debug instruction (#128684)

[llvmbot]

Backport af68927

Requested by: @SLTozer

[llvmbot]

@OCHyams What do you think about merging this PR to the release branch?

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-ir

Author: None (llvmbot)

Changes

Backport af68927

Requested by: @SLTozer

---

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

2 Files Affected:

• (modified) llvm/lib/IR/Instruction.cpp (+1-4)
• (added) llvm/test/Transforms/SimplifyCFG/X86/fake-use-considered-when-sinking.ll (+67)

diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 84d9306ca6700..7d43de982df0d 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -1233,10 +1233,7 @@ Instruction::getNextNonDebugInstruction(bool SkipPseudoOp) const {
 const Instruction *
 Instruction::getPrevNonDebugInstruction(bool SkipPseudoOp) const {
   for (const Instruction *I = getPrevNode(); I; I = I->getPrevNode())
-    if (!isa<DbgInfoIntrinsic>(I) &&
-        !(SkipPseudoOp && isa<PseudoProbeInst>(I)) &&
-        !(isa<IntrinsicInst>(I) &&
-          cast<IntrinsicInst>(I)->getIntrinsicID() == Intrinsic::fake_use))
+    if (!isa<DbgInfoIntrinsic>(I) && !(SkipPseudoOp && isa<PseudoProbeInst>(I)))
       return I;
   return nullptr;
 }
diff --git a/llvm/test/Transforms/SimplifyCFG/X86/fake-use-considered-when-sinking.ll b/llvm/test/Transforms/SimplifyCFG/X86/fake-use-considered-when-sinking.ll
new file mode 100644
index 0000000000000..63217315c978c
--- /dev/null
+++ b/llvm/test/Transforms/SimplifyCFG/X86/fake-use-considered-when-sinking.ll
@@ -0,0 +1,67 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -p="simplifycfg<sink-common-insts>" -S < %s | FileCheck %s
+
+;; Verify that fake uses are not ignored when sinking instructions in
+;; SimplifyCFG; when a fake use appears in only one incoming block they prevent
+;; further sinking, and when identical fake uses appear on both sides they
+;; are sunk normally.
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @foo(i1 %bool, ptr %p) {
+; CHECK-LABEL: define void @foo(
+; CHECK-SAME: i1 [[BOOL:%.*]], ptr [[P:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    br i1 [[BOOL]], label %[[IF_ELSE:.*]], label %[[IF_THEN:.*]]
+; CHECK:       [[COMMON_RET:.*]]:
+; CHECK-NEXT:    ret void
+; CHECK:       [[IF_THEN]]:
+; CHECK-NEXT:    store ptr [[P]], ptr [[P]], align 8
+; CHECK-NEXT:    br label %[[COMMON_RET]]
+; CHECK:       [[IF_ELSE]]:
+; CHECK-NEXT:    store ptr [[P]], ptr [[P]], align 8
+; CHECK-NEXT:    notail call void (...) @llvm.fake.use(ptr [[P]])
+; CHECK-NEXT:    br label %[[COMMON_RET]]
+;
+entry:
+  br i1 %bool, label %if.else, label %if.then
+
+common.ret:                                       ; preds = %if.else, %if.then
+  ret void
+
+if.then:                                          ; preds = %entry
+  store ptr %p, ptr %p, align 8
+  br label %common.ret
+
+if.else:                                          ; preds = %entry
+  store ptr %p, ptr %p, align 8
+  notail call void (...) @llvm.fake.use(ptr %p)
+  br label %common.ret
+}
+
+define void @bar(i1 %bool, ptr %p) {
+; CHECK-LABEL: define void @bar(
+; CHECK-SAME: i1 [[BOOL:%.*]], ptr [[P:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    store ptr [[P]], ptr [[P]], align 8
+; CHECK-NEXT:    notail call void (...) @llvm.fake.use(ptr [[P]])
+; CHECK-NEXT:    ret void
+;
+entry:
+  br i1 %bool, label %if.else, label %if.then
+
+common.ret:                                       ; preds = %if.else, %if.then
+  ret void
+
+if.then:                                          ; preds = %entry
+  store ptr %p, ptr %p, align 8
+  notail call void (...) @llvm.fake.use(ptr %p)
+  br label %common.ret
+
+if.else:                                          ; preds = %entry
+  store ptr %p, ptr %p, align 8
+  notail call void (...) @llvm.fake.use(ptr %p)
+  br label %common.ret
+}
+

[OCHyams]

LGTM - IIUC without this patch we run into failed verifier checks, e.g. in the SimplifyCFG case outlined in the test.

This patch could have a knock-on effect, blocking optimisations beyond the one in the test, but that's what the user signs up for with the -fextend-variable-liveness flag, so I think this is ok. Definitely better to do the right thing (this) now (in this release), and tweak specific optimisations later on a case by case basis, if we discover better trade offs.

[jmorse]

LGTM too as per Orlando's analysis

[github-actions]

@SLTozer (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.