[llvm] [PPCMergeStringPool] Avoid replacing constant with instruction (PR #88846)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 30 22:17:51 PDT 2024
https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/88846
>From ef11bccec5ecd5fba8a64cc2a81e74a83c083cfb Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Tue, 16 Apr 2024 14:46:40 +0900
Subject: [PATCH 1/3] [PPCMergeStringPool] Avoid replacing constant with
instruction
String pool merging currently, for a reason that's not entirely
clear to me, tries to create GEP instructions instead of GEP
constant expressions when replacing constant references. It only
uses constant expressions in cases where this is required. However,
it does not catch all cases where such a requirement exists. For
example, the landingpad catch clause has to be a constant.
Fix this by always using the constant expression variant, which
also makes the implementation simpler.
Additionally, there are some edge cases where even replacement
with a constant GEP is not legal. The one I am aware of is the
llvm.eh.typeid.for intrinsic, so add a special case to forbid
replacements for it.
Fixes https://github.com/llvm/llvm-project/issues/88844.
---
.../lib/Target/PowerPC/PPCMergeStringPool.cpp | 45 ++++++------------
.../PowerPC/merge-string-used-by-metadata.mir | 6 +--
.../mergeable-string-pool-exceptions.ll | 47 +++++++++++++++++++
.../mergeable-string-pool-pass-only.mir | 18 ++++---
4 files changed, 71 insertions(+), 45 deletions(-)
create mode 100644 llvm/test/CodeGen/PowerPC/mergeable-string-pool-exceptions.ll
diff --git a/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp b/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp
index 76d60c28f1e445..60db2772517b20 100644
--- a/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp
+++ b/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp
@@ -23,6 +23,7 @@
#include "llvm/Analysis/ScalarEvolutionAliasAnalysis.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/Instructions.h"
+#include "llvm/IR/IntrinsicInst.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/ValueSymbolTable.h"
#include "llvm/Pass.h"
@@ -117,9 +118,16 @@ class PPCMergeStringPool : public ModulePass {
// sure that they can be replaced.
static bool hasReplaceableUsers(GlobalVariable &GV) {
for (User *CurrentUser : GV.users()) {
- // Instruction users are always valid.
- if (isa<Instruction>(CurrentUser))
+ if (isa<Instruction>(CurrentUser)) {
+ if (auto *II = dyn_cast<IntrinsicInst>(CurrentUser)) {
+ // Some intrinsics require a plain global.
+ if (II->getIntrinsicID() == Intrinsic::eh_typeid_for)
+ return false;
+ }
+
+ // Other instruction users are always valid.
continue;
+ }
// We cannot replace GlobalValue users because they are not just nodes
// in IR. To replace a user like this we would need to create a new
@@ -330,38 +338,13 @@ void PPCMergeStringPool::replaceUsesWithGEP(GlobalVariable *GlobalToReplace,
if (isa<GlobalValue>(CurrentUser))
continue;
- if (!UserInstruction) {
- // User is a constant type.
- Constant *ConstGEP = ConstantExpr::getInBoundsGetElementPtr(
- PooledStructType, GPool, Indices);
- UserConstant->handleOperandChange(GlobalToReplace, ConstGEP);
- continue;
- }
-
- if (PHINode *UserPHI = dyn_cast<PHINode>(UserInstruction)) {
- // GEP instructions cannot be added before PHI nodes.
- // With getInBoundsGetElementPtr we create the GEP and then replace it
- // inline into the PHI.
- Constant *ConstGEP = ConstantExpr::getInBoundsGetElementPtr(
- PooledStructType, GPool, Indices);
- UserPHI->replaceUsesOfWith(GlobalToReplace, ConstGEP);
- continue;
- }
- // The user is a valid instruction that is not a PHINode.
- GetElementPtrInst *GEPInst =
- GetElementPtrInst::Create(PooledStructType, GPool, Indices);
- GEPInst->insertBefore(UserInstruction);
-
- LLVM_DEBUG(dbgs() << "Inserting GEP before:\n");
- LLVM_DEBUG(UserInstruction->dump());
-
+ Constant *ConstGEP = ConstantExpr::getInBoundsGetElementPtr(
+ PooledStructType, GPool, Indices);
LLVM_DEBUG(dbgs() << "Replacing this global:\n");
LLVM_DEBUG(GlobalToReplace->dump());
LLVM_DEBUG(dbgs() << "with this:\n");
- LLVM_DEBUG(GEPInst->dump());
-
- // After the GEP is inserted the GV can be replaced.
- CurrentUser->replaceUsesOfWith(GlobalToReplace, GEPInst);
+ LLVM_DEBUG(ConstGEP->dump());
+ GlobalToReplace->replaceAllUsesWith(ConstGEP);
}
}
diff --git a/llvm/test/CodeGen/PowerPC/merge-string-used-by-metadata.mir b/llvm/test/CodeGen/PowerPC/merge-string-used-by-metadata.mir
index 2a791966be4ecf..4a40974a2a227b 100644
--- a/llvm/test/CodeGen/PowerPC/merge-string-used-by-metadata.mir
+++ b/llvm/test/CodeGen/PowerPC/merge-string-used-by-metadata.mir
@@ -14,16 +14,14 @@
define noundef ptr @func1(ptr noundef nonnull align 8 dereferenceable(8) %this) #0 !dbg !6 {
; CHECK-LABEL: func1
- ; CHECK: %0 = getelementptr { [7 x i8], [7 x i8] }, ptr @__ModuleStringPool, i32 0, i32 1
- ; CHECK-NEXT: ret ptr %0, !dbg !14
+ ; CHECK: ret ptr getelementptr inbounds ({ [7 x i8], [7 x i8] }, ptr @__ModuleStringPool, i32 0, i32 1), !dbg !14
entry:
ret ptr @const.2, !dbg !14
}
define noundef ptr @func2(ptr noundef nonnull align 8 dereferenceable(8) %this) #0 {
; CHECK-LABEL: func2
- ; CHECK: %0 = getelementptr { [7 x i8], [7 x i8] }, ptr @__ModuleStringPool, i32 0, i32 0
- ; CHECK-NEXT: ret ptr %0
+ ; CHECK: ret ptr @__ModuleStringPool
entry:
ret ptr @const.1
}
diff --git a/llvm/test/CodeGen/PowerPC/mergeable-string-pool-exceptions.ll b/llvm/test/CodeGen/PowerPC/mergeable-string-pool-exceptions.ll
new file mode 100644
index 00000000000000..0489c74c0f819f
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/mergeable-string-pool-exceptions.ll
@@ -0,0 +1,47 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
+; RUN: llc -mtriple=ppc64le-unknown-linux-gnu < %s | FileCheck %s
+
+ at id = private unnamed_addr constant [4 x i8] c"@id\00", align 1
+ at id2 = private unnamed_addr constant [5 x i8] c"@id2\00", align 1
+
+; Higher-aligned dummy to make sure it is first in the string pool.
+ at dummy = private unnamed_addr constant [1 x i32] [i32 42], align 4
+
+define ptr @test1() personality ptr @__gnu_objc_personality_v0 {
+; CHECK-LABEL: test1:
+; CHECK: # %bb.0:
+; CHECK-NEXT: mflr 0
+; CHECK-NEXT: stdu 1, -32(1)
+; CHECK-NEXT: std 0, 48(1)
+; CHECK-NEXT: .cfi_def_cfa_offset 32
+; CHECK-NEXT: .cfi_offset lr, 16
+; CHECK-NEXT: addis 3, 2, .L__ModuleStringPool at toc@ha
+; CHECK-NEXT: addi 3, 3, .L__ModuleStringPool at toc@l
+; CHECK-NEXT: bl foo
+; CHECK-NEXT: nop
+ invoke void @foo(ptr @dummy)
+ to label %cont unwind label %unwind
+
+cont:
+ unreachable
+
+unwind:
+ %lp = landingpad { ptr, i32 }
+ catch ptr @id
+ resume { ptr, i32 } %lp
+}
+
+define i32 @test2() personality ptr @__gnu_objc_personality_v0 {
+; CHECK-LABEL: test2:
+; CHECK: # %bb.0:
+; CHECK-NEXT: li 3, 1
+; CHECK-NEXT: blr
+ %id = tail call i32 @llvm.eh.typeid.for(ptr @id2)
+ ret i32 %id
+}
+
+declare i32 @__gnu_objc_personality_v0(...)
+
+declare i32 @llvm.eh.typeid.for(ptr)
+
+declare void @foo()
diff --git a/llvm/test/CodeGen/PowerPC/mergeable-string-pool-pass-only.mir b/llvm/test/CodeGen/PowerPC/mergeable-string-pool-pass-only.mir
index e2fb0ced8f3490..3d8afb604fd3ca 100644
--- a/llvm/test/CodeGen/PowerPC/mergeable-string-pool-pass-only.mir
+++ b/llvm/test/CodeGen/PowerPC/mergeable-string-pool-pass-only.mir
@@ -35,8 +35,7 @@
ret i32 %call
; CHECK-LABEL: test1
- ; CHECK: %0 = getelementptr { [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 6
- ; CHECK: tail call signext i32 @calleeStr
+ ; CHECK: %call = tail call signext i32 @calleeStr(ptr noundef nonnull getelementptr inbounds ({ [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 6))
}
define dso_local signext i32 @test2() local_unnamed_addr #0 {
@@ -49,7 +48,7 @@
ret i32 %call
; CHECK-LABEL: test2
- ; CHECK: %0 = getelementptr { [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 2
+ ; CHECK: call void @llvm.memcpy.p0.p0.i64(ptr noundef nonnull align 4 dereferenceable(24) %A, ptr noundef nonnull align 4 dereferenceable(24) getelementptr inbounds ({ [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 2), i64 24, i1 false)
; CHECK: call signext i32 @calleeInt
}
@@ -62,7 +61,7 @@
call void @llvm.lifetime.end.p0(i64 28, ptr nonnull %A) #0
ret i32 %call
; CHECK-LABEL: test3
- ; CHECK: %0 = getelementptr { [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 4
+ ; CHECK: call void @llvm.memcpy.p0.p0.i64(ptr noundef nonnull align 4 dereferenceable(28) %A, ptr noundef nonnull align 4 dereferenceable(28) getelementptr inbounds ({ [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 4), i64 28, i1 false)
; CHECK: call signext i32 @calleeFloat
}
@@ -75,7 +74,7 @@
call void @llvm.lifetime.end.p0(i64 56, ptr nonnull %A) #0
ret i32 %call
; CHECK-LABEL: test4
- ; CHECK: %0 = getelementptr { [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 0
+ ; CHECK: call void @llvm.memcpy.p0.p0.i64(ptr noundef nonnull align 8 dereferenceable(56) %A, ptr noundef nonnull align 8 dereferenceable(56) @__ModuleStringPool, i64 56, i1 false)
; CHECK: call signext i32 @calleeDouble
}
@@ -102,11 +101,10 @@
call void @llvm.lifetime.end.p0(i64 24, ptr nonnull %B) #0
ret i32 %add7
; CHECK-LABEL: test5
- ; CHECK: %0 = getelementptr { [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 3
- ; CHECK: %1 = getelementptr { [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 5
- ; CHECK: %2 = getelementptr { [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 1
- ; CHECK: %3 = getelementptr { [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 7
- ; CHECK: call signext i32 @calleeStr
+ ; CHECK: call void @llvm.memcpy.p0.p0.i64(ptr noundef nonnull align 4 dereferenceable(24) %B, ptr noundef nonnull align 4 dereferenceable(24) getelementptr inbounds ({ [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 3), i64 24, i1 false)
+ ; CHECK: call void @llvm.memcpy.p0.p0.i64(ptr noundef nonnull align 4 dereferenceable(28) %C, ptr noundef nonnull align 4 dereferenceable(28) getelementptr inbounds ({ [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 5), i64 28, i1 false)
+ ; CHECK: call void @llvm.memcpy.p0.p0.i64(ptr noundef nonnull align 8 dereferenceable(56) %D, ptr noundef nonnull align 8 dereferenceable(56) getelementptr inbounds ({ [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 1), i64 56, i1 false)
+ ; CHECK: call signext i32 @calleeStr(ptr noundef nonnull getelementptr inbounds ({ [7 x double], [7 x double], [6 x i32], [6 x i32], [7 x float], [7 x float], [8 x i8], [16 x i8] }, ptr @__ModuleStringPool, i32 0, i32 7))
; CHECK: call signext i32 @calleeInt
; CHECK: call signext i32 @calleeFloat
; CHECK: call signext i32 @calleeDouble
>From bbf1144e545fc5d9bff9b97b1188ea413378b019 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 1 May 2024 14:09:07 +0900
Subject: [PATCH 2/3] drop assertion
---
llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp | 8 --------
1 file changed, 8 deletions(-)
diff --git a/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp b/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp
index 60db2772517b20..84e447fecbfab1 100644
--- a/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp
+++ b/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp
@@ -322,14 +322,6 @@ void PPCMergeStringPool::replaceUsesWithGEP(GlobalVariable *GlobalToReplace,
Users.push_back(CurrentUser);
for (User *CurrentUser : Users) {
- Instruction *UserInstruction = dyn_cast<Instruction>(CurrentUser);
- Constant *UserConstant = dyn_cast<Constant>(CurrentUser);
-
- // At this point we expect that the user is either an instruction or a
- // constant.
- assert((UserConstant || UserInstruction) &&
- "Expected the user to be an instruction or a constant.");
-
// The user was not found so it must have been replaced earlier.
if (!userHasOperand(CurrentUser, GlobalToReplace))
continue;
>From d7b746e0058289c6e699754ce0ac8ea7134ebea2 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 1 May 2024 14:17:31 +0900
Subject: [PATCH 3/3] Also check for EHPads
---
llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp | 8 ++++++--
.../CodeGen/PowerPC/mergeable-string-pool-exceptions.ll | 4 ++--
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp b/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp
index 84e447fecbfab1..abc5353e4a5e92 100644
--- a/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp
+++ b/llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp
@@ -118,8 +118,12 @@ class PPCMergeStringPool : public ModulePass {
// sure that they can be replaced.
static bool hasReplaceableUsers(GlobalVariable &GV) {
for (User *CurrentUser : GV.users()) {
- if (isa<Instruction>(CurrentUser)) {
- if (auto *II = dyn_cast<IntrinsicInst>(CurrentUser)) {
+ if (auto *I = dyn_cast<Instruction>(CurrentUser)) {
+ // Do not merge globals in exception pads.
+ if (I->isEHPad())
+ return false;
+
+ if (auto *II = dyn_cast<IntrinsicInst>(I)) {
// Some intrinsics require a plain global.
if (II->getIntrinsicID() == Intrinsic::eh_typeid_for)
return false;
diff --git a/llvm/test/CodeGen/PowerPC/mergeable-string-pool-exceptions.ll b/llvm/test/CodeGen/PowerPC/mergeable-string-pool-exceptions.ll
index 0489c74c0f819f..03a830e087d263 100644
--- a/llvm/test/CodeGen/PowerPC/mergeable-string-pool-exceptions.ll
+++ b/llvm/test/CodeGen/PowerPC/mergeable-string-pool-exceptions.ll
@@ -15,8 +15,8 @@ define ptr @test1() personality ptr @__gnu_objc_personality_v0 {
; CHECK-NEXT: std 0, 48(1)
; CHECK-NEXT: .cfi_def_cfa_offset 32
; CHECK-NEXT: .cfi_offset lr, 16
-; CHECK-NEXT: addis 3, 2, .L__ModuleStringPool at toc@ha
-; CHECK-NEXT: addi 3, 3, .L__ModuleStringPool at toc@l
+; CHECK-NEXT: addis 3, 2, .Ldummy at toc@ha
+; CHECK-NEXT: addi 3, 3, .Ldummy at toc@l
; CHECK-NEXT: bl foo
; CHECK-NEXT: nop
invoke void @foo(ptr @dummy)
More information about the llvm-commits
mailing list