Skip to content

Conversation

XChy
Copy link
Member

@XChy XChy commented Aug 28, 2025

Fixes #155738.
The original assumption "we already replaced its users with a constant" for the global variable becomes incorrect after #154668. The users in the dead function are not simplified, in fact.
This patch poisons all the unsimplified constant global variable users.

@llvmbot
Copy link
Member

llvmbot commented Aug 28, 2025

@llvm/pr-subscribers-function-specialization

@llvm/pr-subscribers-llvm-transforms

Author: XChy (XChy)

Changes

The original assumption "we already replaced its users with a constant" for the global variable becomes incorrect after #154668. The users in the dead function are not simplified, in fact.
This patch poisons all the unsimplified constant global variable users.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/IPO/SCCP.cpp (+6-3)
  • (added) llvm/test/Transforms/FunctionSpecialization/dead-gv-load.ll (+31)
diff --git a/llvm/lib/Transforms/IPO/SCCP.cpp b/llvm/lib/Transforms/IPO/SCCP.cpp
index e98a70f228ada..f4961faf7ad18 100644
--- a/llvm/lib/Transforms/IPO/SCCP.cpp
+++ b/llvm/lib/Transforms/IPO/SCCP.cpp
@@ -330,12 +330,15 @@ static bool runIPSCCP(
     LLVM_DEBUG(dbgs() << "Found that GV '" << GV->getName()
                       << "' is constant!\n");
     for (User *U : make_early_inc_range(GV->users())) {
-      // We can remove LoadInst here, because we already replaced its users
-      // with a constant.
+      // We can remove LoadInst here. The LoadInsts in dead functions marked by
+      // FuncSpec are not simplified to constants, thus poison them.
       assert((isa<StoreInst>(U) || isa<LoadInst>(U)) &&
              "Only Store|Load Instruction can be user of GlobalVariable at "
              "reaching here.");
-      cast<Instruction>(U)->eraseFromParent();
+      Instruction *I = cast<Instruction>(U);
+      if (isa<LoadInst>(I))
+        I->replaceAllUsesWith(PoisonValue::get(I->getType()));
+      I->eraseFromParent();
     }
 
     // Try to create a debug constant expression for the global variable
diff --git a/llvm/test/Transforms/FunctionSpecialization/dead-gv-load.ll b/llvm/test/Transforms/FunctionSpecialization/dead-gv-load.ll
new file mode 100644
index 0000000000000..29c052a5a9bac
--- /dev/null
+++ b/llvm/test/Transforms/FunctionSpecialization/dead-gv-load.ll
@@ -0,0 +1,31 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -passes=ipsccp  --funcspec-min-function-size=1 -S < %s | FileCheck %s
+
+@gv = internal global ptr null
+
+define i8 @caller() {
+; CHECK-LABEL: define i8 @caller() {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[CALL1:%.*]] = call i8 @callee.specialized.1(i32 1)
+; CHECK-NEXT:    [[CALL2:%.*]] = call i8 @callee.specialized.2(i32 0)
+; CHECK-NEXT:    ret i8 undef
+;
+entry:
+  %call1 = call i8 @callee(i32 1)
+  %call2 = call i8 @callee(i32 0)
+  ret i8 %call2
+}
+
+define internal i8 @callee(i32 %arg) {
+entry:
+  %useless = icmp ne i32 %arg, 0
+  br label %loop
+
+loop:                                       ; preds = %cleanup424, %entry
+  br label %loop
+
+dead_bb:                                       ; No predecessors!
+  %l1 = load ptr, ptr @gv, align 8
+  %l2 = load ptr, ptr %l1, align 8
+  ret i8 0
+}

Copy link
Contributor

@antoniofrighetto antoniofrighetto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@XChy XChy enabled auto-merge (squash) August 28, 2025 10:48
@XChy XChy merged commit 0ace96c into llvm:main Aug 28, 2025
9 checks passed
t-a-james pushed a commit to t-a-james/llvm-project that referenced this pull request Aug 28, 2025
…oject into bugprone-method-hiding

* 'bugprone-method-hiding' of github.com:t-a-james/llvm-project: (230 commits)
  [SimplifyCFG] Move token type check into canReplaceOperandWithVariable()
  [ADT] Fix signed integer overflow (llvm#155826)
  [Offload] Update LIBOMPTARGET_INFO text for `attach` map-type. (llvm#155509)
  [CMake][AIX] Enable CMP0182: Create shared library archives by default (llvm#155686)
  AMDGPU: Add tests for atomics with AGPR operands (llvm#155820)
  [AArch64] Split zero cycle zeoring per register class (llvm#154561)
  [gn build] Port fa883e1
  [mlir][tosa] Allow shift operand of tosa::MulOp as non-constant (llvm#155197)
  [AArch64][NFC] Add MCInstrAnalysis unittests (llvm#155609)
  [Offload][OpenMP] Tests require libc on GPU for printf (llvm#155785)
  AMDGPU: Add missing verifier tests for load/store AGPR case (llvm#155815)
  [lldb-mcp] Fix building for Windows
  Revert "[lldb] Correct a usage after a rename was merged. (llvm#155720)"
  Revert "[lldb] NFC Moving mcp::Transport into its own file. (llvm#155711)"
  [lldb][test] Run ranges::ref_vew test only for libc++ (llvm#155813)
  [SCCP][FuncSpec] Poison unreachable constant global variable user (llvm#155753)
  [LoongArch] Lowering v32i8 vector mask generation to `VMSKLTZ` (llvm#149953)
  [flang][docs][NFC] Remove stray backtick (llvm#154974)
  [MLIR] Apply clang-tidy fixes for misc-use-internal-linkage in LinalgOps.cpp (NFC)
  [MLIR] Apply clang-tidy fixes for performance-move-const-arg in VariantValue.cpp (NFC)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[IPSCCP] Use still stuck around after Def is destroyed
3 participants