IR: Simplify BlockAddress replacement #135360

arsenm · 2025-04-11T12:37:12Z

Don't repeatedly replaceAllUsesWith in a loop.

Use poison for the replacement value, and don't repeatedly replaceAllUsesWith in a loop.

arsenm · 2025-04-11T12:37:33Z

IR: Simplify BlockAddress replacement #135360 👈 (View in Graphite)
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

llvmbot · 2025-04-11T12:37:54Z

@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-backend-webassembly

@llvm/pr-subscribers-llvm-ir

Author: Matt Arsenault (arsenm)

Changes

Use poison for the replacement value, and don't repeatedly
replaceAllUsesWith in a loop.

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

7 Files Affected:

(modified) llvm/lib/IR/BasicBlock.cpp (+3-8)
(modified) llvm/test/CodeGen/RISCV/codemodel-lowering.ll (+5-10)
(modified) llvm/test/CodeGen/WebAssembly/indirectbr.ll (+1-1)
(modified) llvm/test/Transforms/Attributor/IPConstantProp/dangling-block-address.ll (+1-1)
(modified) llvm/test/Transforms/Attributor/liveness.ll (+1-1)
(modified) llvm/test/Transforms/JumpThreading/select.ll (+1-1)
(modified) llvm/test/Transforms/LoopDeletion/blockaddress.ll (+1-1)

diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index a6d16b157c0ad..31aaa6e68e994 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -223,14 +223,9 @@ BasicBlock::~BasicBlock() {
   // nodes.  There are no other possible uses at this point.
   if (hasAddressTaken()) {
     assert(!use_empty() && "There should be at least one blockaddress!");
-    Constant *Replacement =
-      ConstantInt::get(llvm::Type::getInt32Ty(getContext()), 1);
-    while (!use_empty()) {
-      BlockAddress *BA = cast<BlockAddress>(user_back());
-      BA->replaceAllUsesWith(ConstantExpr::getIntToPtr(Replacement,
-                                                       BA->getType()));
-      BA->destroyConstant();
-    }
+    BlockAddress *BA = cast<BlockAddress>(user_back());
+    BA->replaceAllUsesWith(PoisonValue::get(BA->getType()));
+    BA->destroyConstant();
   }
 
   assert(getParent() == nullptr && "BasicBlock still linked into the program!");
diff --git a/llvm/test/CodeGen/RISCV/codemodel-lowering.ll b/llvm/test/CodeGen/RISCV/codemodel-lowering.ll
index 4831f0b24c7fe..6082f08a3128a 100644
--- a/llvm/test/CodeGen/RISCV/codemodel-lowering.ll
+++ b/llvm/test/CodeGen/RISCV/codemodel-lowering.ll
@@ -69,31 +69,27 @@ define void @lower_blockaddress() nounwind {
 ; RV32I-SMALL-LABEL: lower_blockaddress:
 ; RV32I-SMALL:       # %bb.0:
 ; RV32I-SMALL-NEXT:    lui a0, %hi(addr)
-; RV32I-SMALL-NEXT:    li a1, 1
-; RV32I-SMALL-NEXT:    sw a1, %lo(addr)(a0)
+; RV32I-SMALL-NEXT:    sw a0, %lo(addr)(a0)
 ; RV32I-SMALL-NEXT:    ret
 ;
 ; RV32I-MEDIUM-LABEL: lower_blockaddress:
 ; RV32I-MEDIUM:       # %bb.0:
 ; RV32I-MEDIUM-NEXT:  .Lpcrel_hi1:
 ; RV32I-MEDIUM-NEXT:    auipc a0, %pcrel_hi(addr)
-; RV32I-MEDIUM-NEXT:    li a1, 1
-; RV32I-MEDIUM-NEXT:    sw a1, %pcrel_lo(.Lpcrel_hi1)(a0)
+; RV32I-MEDIUM-NEXT:    sw a0, %pcrel_lo(.Lpcrel_hi1)(a0)
 ; RV32I-MEDIUM-NEXT:    ret
 ;
 ; RV64I-SMALL-LABEL: lower_blockaddress:
 ; RV64I-SMALL:       # %bb.0:
 ; RV64I-SMALL-NEXT:    lui a0, %hi(addr)
-; RV64I-SMALL-NEXT:    li a1, 1
-; RV64I-SMALL-NEXT:    sd a1, %lo(addr)(a0)
+; RV64I-SMALL-NEXT:    sd a0, %lo(addr)(a0)
 ; RV64I-SMALL-NEXT:    ret
 ;
 ; RV64I-MEDIUM-LABEL: lower_blockaddress:
 ; RV64I-MEDIUM:       # %bb.0:
 ; RV64I-MEDIUM-NEXT:  .Lpcrel_hi1:
 ; RV64I-MEDIUM-NEXT:    auipc a0, %pcrel_hi(addr)
-; RV64I-MEDIUM-NEXT:    li a1, 1
-; RV64I-MEDIUM-NEXT:    sd a1, %pcrel_lo(.Lpcrel_hi1)(a0)
+; RV64I-MEDIUM-NEXT:    sd a0, %pcrel_lo(.Lpcrel_hi1)(a0)
 ; RV64I-MEDIUM-NEXT:    ret
 ;
 ; RV64I-LARGE-LABEL: lower_blockaddress:
@@ -101,8 +97,7 @@ define void @lower_blockaddress() nounwind {
 ; RV64I-LARGE-NEXT:  .Lpcrel_hi1:
 ; RV64I-LARGE-NEXT:    auipc a0, %pcrel_hi(.LCPI1_0)
 ; RV64I-LARGE-NEXT:    ld a0, %pcrel_lo(.Lpcrel_hi1)(a0)
-; RV64I-LARGE-NEXT:    li a1, 1
-; RV64I-LARGE-NEXT:    sd a1, 0(a0)
+; RV64I-LARGE-NEXT:    sd a0, 0(a0)
 ; RV64I-LARGE-NEXT:    ret
   store volatile ptr blockaddress(@lower_blockaddress, %block), ptr @addr
   ret void
diff --git a/llvm/test/CodeGen/WebAssembly/indirectbr.ll b/llvm/test/CodeGen/WebAssembly/indirectbr.ll
index 569d289d3d279..fb3586d8d03e0 100644
--- a/llvm/test/CodeGen/WebAssembly/indirectbr.ll
+++ b/llvm/test/CodeGen/WebAssembly/indirectbr.ll
@@ -32,7 +32,7 @@ target triple = "wasm32"
 ; CHECK-NEXT: .int32
 ; CHECK-NEXT: .int32
 ; CHECK-NEXT: .int32
-; CHECK-NEXT: .int32
+; CHECK-NEXT: .skip 4
 
 define void @test1(ptr readonly %p, ptr %sink) #0 {
 
diff --git a/llvm/test/Transforms/Attributor/IPConstantProp/dangling-block-address.ll b/llvm/test/Transforms/Attributor/IPConstantProp/dangling-block-address.ll
index b224b5e238e57..81c42ef46e662 100644
--- a/llvm/test/Transforms/Attributor/IPConstantProp/dangling-block-address.ll
+++ b/llvm/test/Transforms/Attributor/IPConstantProp/dangling-block-address.ll
@@ -11,7 +11,7 @@
 
 ;.
 ; TUNIT: @code = global [5 x i32] [i32 0, i32 0, i32 0, i32 0, i32 1], align 4
-; TUNIT: @bar.l = internal constant [2 x ptr] [ptr inttoptr (i32 1 to ptr), ptr inttoptr (i32 1 to ptr)]
+; TUNIT: @bar.l = internal constant [2 x ptr] undef
 ;.
 ; CGSCC: @code = global [5 x i32] [i32 0, i32 0, i32 0, i32 0, i32 1], align 4
 ; CGSCC: @bar.l = internal constant [2 x ptr] [ptr blockaddress(@bar, %lab0), ptr blockaddress(@bar, %end)]
diff --git a/llvm/test/Transforms/Attributor/liveness.ll b/llvm/test/Transforms/Attributor/liveness.ll
index 874eff661f053..e2c083bb9833d 100644
--- a/llvm/test/Transforms/Attributor/liveness.ll
+++ b/llvm/test/Transforms/Attributor/liveness.ll
@@ -24,7 +24,7 @@ declare i32 @bar() nosync readnone
 ; and nothing should be deduced for it.
 
 ;.
-; TUNIT: @dead_with_blockaddress_users.l = constant [2 x ptr] [ptr inttoptr (i32 1 to ptr), ptr inttoptr (i32 1 to ptr)]
+; TUNIT: @dead_with_blockaddress_users.l = constant [2 x ptr] undef
 ; TUNIT: @a1 = common global i8 0, align 8
 ; TUNIT: @a2 = common global i8 0, align 16
 ; TUNIT: @e = global ptr null
diff --git a/llvm/test/Transforms/JumpThreading/select.ll b/llvm/test/Transforms/JumpThreading/select.ll
index 4ec55a66bb8ac..185e3cc2dad19 100644
--- a/llvm/test/Transforms/JumpThreading/select.ll
+++ b/llvm/test/Transforms/JumpThreading/select.ll
@@ -21,7 +21,7 @@ declare void @quux()
 ; booleans where at least one operand is true/false/undef.
 
 ;.
-; CHECK: @[[ANCHOR:[a-zA-Z0-9_$"\\.-]+]] = constant [3 x ptr] [ptr blockaddress(@test_indirectbr, [[L1:%.*]]), ptr inttoptr (i32 1 to ptr), ptr blockaddress(@test_indirectbr, [[L3:%.*]])]
+; CHECK: @[[ANCHOR:[a-zA-Z0-9_$"\\.-]+]] = constant [3 x ptr] [ptr blockaddress(@test_indirectbr, [[L1:%.*]]), ptr poison, ptr blockaddress(@test_indirectbr, [[L3:%.*]])]
 ;.
 define void @test_br(i1 %cond, i1 %value) nounwind {
 ; CHECK-LABEL: @test_br(
diff --git a/llvm/test/Transforms/LoopDeletion/blockaddress.ll b/llvm/test/Transforms/LoopDeletion/blockaddress.ll
index e943b3f390414..7635ead68f127 100644
--- a/llvm/test/Transforms/LoopDeletion/blockaddress.ll
+++ b/llvm/test/Transforms/LoopDeletion/blockaddress.ll
@@ -6,7 +6,7 @@
 declare void @g(ptr)
 
 ;.
-; CHECK: @[[BA:[a-zA-Z0-9_$"\\.-]+]] = private constant ptr inttoptr (i32 1 to ptr)
+; CHECK: @[[BA:[a-zA-Z0-9_$"\\.-]+]] = private constant ptr poison
 ;.
 define void @f() {
 ; CHECK-LABEL: @f(

nikic

This isn't correct: If a block gets eliminated, uses of the blockaddress need to be replaced with a non-null value, which is why we're replacing with inttoptr 1 here.

arsenm · 2025-04-11T12:48:06Z

Why non null? If the block is deleted, how is it anything other than UB to make use of its address?

nikic · 2025-04-11T12:53:16Z

@arsenm See https://llvm.org/docs/LangRef.html#addresses-of-basic-blocks:

This value only has defined behavior when used as an operand to the ‘indirectbr’ or for comparisons against null. Pointer equality tests between labels addresses results in undefined behavior — though, again, comparison against null is ok, and no label is equal to the null pointer. This may be passed around as an opaque pointer sized value as long as the bits are not inspected. This allows ptrtoint and arithmetic to be performed on these values so long as the original value is reconstituted before the indirectbr instruction.

If a block is deleted, we still must guarantee that the corresponding blockaddress compares as non-null.

arsenm · 2025-04-11T14:00:25Z

Switched to just removing the loop

arsenm · 2025-04-11T15:00:47Z

Merge activity

Apr 11, 11:00 AM EDT: A user started a stack merge that includes this pull request via Graphite.
Apr 11, 11:02 AM EDT: A user merged this pull request with Graphite.

Don't repeatedly replaceAllUsesWith in a loop.

IR: Simplify BlockAddress replacement

776f5fa

Use poison for the replacement value, and don't repeatedly replaceAllUsesWith in a loop.

arsenm added the llvm:ir label Apr 11, 2025 — with Graphite App

arsenm requested review from dexonsmith, dtcxzyw, fhahn, jdoerfert and nikic April 11, 2025 12:38

arsenm marked this pull request as ready for review April 11, 2025 12:38

llvmbot added backend:WebAssembly llvm:transforms labels Apr 11, 2025

nikic requested changes Apr 11, 2025

View reviewed changes

Keep using inttoptr 1

57377bc

nikic approved these changes Apr 11, 2025

View reviewed changes

arsenm merged commit 61f95c6 into main Apr 11, 2025
11 checks passed

arsenm deleted the users/arsenm/ir/simplify-block-address-deletion branch April 11, 2025 15:02

var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025

IR: Simplify BlockAddress replacement (llvm#135360)

48b9896

Don't repeatedly replaceAllUsesWith in a loop.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

IR: Simplify BlockAddress replacement #135360

IR: Simplify BlockAddress replacement #135360

Uh oh!

arsenm commented Apr 11, 2025 •

edited

Loading

Uh oh!

arsenm commented Apr 11, 2025

Uh oh!

llvmbot commented Apr 11, 2025 •

edited

Loading

Uh oh!

nikic left a comment

Uh oh!

arsenm commented Apr 11, 2025

Uh oh!

nikic commented Apr 11, 2025

Uh oh!

arsenm commented Apr 11, 2025

Uh oh!

arsenm commented Apr 11, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

IR: Simplify BlockAddress replacement #135360

IR: Simplify BlockAddress replacement #135360

Uh oh!

Conversation

arsenm commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arsenm commented Apr 11, 2025

Uh oh!

llvmbot commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

arsenm commented Apr 11, 2025

Uh oh!

nikic commented Apr 11, 2025

Uh oh!

arsenm commented Apr 11, 2025

Uh oh!

arsenm commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Uh oh!

arsenm commented Apr 11, 2025 •

edited

Loading

llvmbot commented Apr 11, 2025 •

edited

Loading

arsenm commented Apr 11, 2025 •

edited

Loading