Do not treat llvm.fake.use as a debug instruction #128684
Conversation
The llvm.fake.use intrinsic is used to prevent certain values from being optimized out for the benefit of debug info; it is not, however, a debug or pseudo instruction itself and necessarily must not be treated as one, since its purpose is to act like a normal instruction.
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-llvm-transforms Author: Stephen Tozer (SLTozer) ChangesThe llvm.fake.use intrinsic is used to prevent certain values from being optimized out for the benefit of debug info; it is not, however, a debug or pseudo instruction itself and necessarily must not be treated as one, since its purpose is to act like a normal instruction. In the original commit that added them, the IR intrinsic however was treated as one in Full diff: https://github.com/llvm/llvm-project/pull/128684.diff 2 Files Affected:
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
+}
+
|
| ;; 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. |
There was a problem hiding this comment.
Seems a bit fuzzy to me whether fake.use should block this optimisation or ignore the intrinsics and sink them into the suc. I don't find the docs particularly clarifying for this specific case.
You've been close to this work and have upstreamed it, so this is less me contesting the correct behaviour, more hoping for a bit more of rationale/explanation?
There was a problem hiding this comment.
The basic reason here is that fake uses must be treated as any other instruction by default. As an optimization we can create specific exceptions for fake uses in cases where we know it's either safe or beneficial to treat them differently, but we wouldn't want to do so "generally" as we do by skipping them in getPrevNonDebugInstruction.
For sinking common instructions specifically, I imagine it's safe to sink fake uses separate to the other instructions in their block, but that specialization should exist in the sinking code (and would be beyond the scope of this patch).
There was a problem hiding this comment.
Thanks for the inline explanation, no further concerns from me so LGTM
I suppose this might have knock on effects besides in SimplifyCFG, but as you have outlined if the previous behaviour is discovered to have been desirable it should be special-cased at those sites.
|
When merged, please file a pull-up for llvm20, IIRC these patchs are in the release. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/13573 Here is the relevant piece of the build log for the reference |
|
/cherry-pick af68927 |
|
/pull-request #128734 |
The llvm.fake.use intrinsic is used to prevent certain values from being optimized out for the benefit of debug info; it is not, however, a debug or pseudo instruction itself and necessarily must not be treated as one, since its purpose is to act like a normal instruction. In the original commit that added them, the IR intrinsic however was treated as one in `getPrevNonDebugInstruction` (but _not_ in `getNextNonDebugInstruction`, or in the MIR equivalents). This patch correctly treats it as a non-debug instruction. (cherry picked from commit af68927)
The llvm.fake.use intrinsic is used to prevent certain values from being optimized out for the benefit of debug info; it is not, however, a debug or pseudo instruction itself and necessarily must not be treated as one, since its purpose is to act like a normal instruction. In the original commit that added them, the IR intrinsic however was treated as one in
getPrevNonDebugInstruction(but not ingetNextNonDebugInstruction, or in the MIR equivalents). This patch correctly treats it as a non-debug instruction.