Skip to content

Commit

Permalink
[PPCMergeStringPool] Avoid replacing constant with instruction (llvm#…
Browse files Browse the repository at this point in the history 
…88846)

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 llvm#88844.

(cherry picked from commit 3a3aeb8)
  • Loading branch information
@nikic @tstellar
nikic authored and tstellar committed May 14, 2024
1 parent f1491c7 commit 1184a9c
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 63 deletions.
57 changes: 18 additions & 39 deletions llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -116,9 +117,20 @@ 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 (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;
}

// 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
Expand Down Expand Up @@ -302,14 +314,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;
Expand All @@ -318,38 +322,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);
}
}

Expand Down
47 changes: 47 additions & 0 deletions llvm/test/CodeGen/PowerPC/mergeable-string-pool-exceptions.ll
View file
Original file line number Diff line number Diff line change
@@ -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

@id = private unnamed_addr constant [4 x i8] c"@id\00", align 1
@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.
@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@toc@ha
; CHECK-NEXT: addi 3, 3, .L__ModuleStringPool@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()
14 changes: 7 additions & 7 deletions llvm/test/CodeGen/PowerPC/mergeable-string-pool-large.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,16 +319,16 @@ define dso_local signext i32 @array0() local_unnamed_addr #0 {
; AIX32-NEXT: mflr r0
; AIX32-NEXT: stwu r1, -96(r1)
; AIX32-NEXT: lis r6, 0
; AIX32-NEXT: lwz r4, L..C0(r2) # @__ModuleStringPool
; AIX32-NEXT: li r5, 12
; AIX32-NEXT: lwz r5, L..C0(r2) # @__ModuleStringPool
; AIX32-NEXT: li r4, 12
; AIX32-NEXT: addi r3, r1, 64
; AIX32-NEXT: stw r0, 104(r1)
; AIX32-NEXT: ori r7, r6, 35596
; AIX32-NEXT: rlwimi r5, r3, 0, 30, 27
; AIX32-NEXT: lxvw4x vs0, r4, r7
; AIX32-NEXT: stxvw4x vs0, 0, r5
; AIX32-NEXT: ori r5, r6, 35584
; AIX32-NEXT: lxvw4x vs0, r4, r5
; AIX32-NEXT: rlwimi r4, r3, 0, 30, 27
; AIX32-NEXT: lxvw4x vs0, r5, r7
; AIX32-NEXT: stxvw4x vs0, 0, r4
; AIX32-NEXT: ori r4, r6, 35584
; AIX32-NEXT: lxvw4x vs0, r5, r4
; AIX32-NEXT: stxvw4x vs0, 0, r3
; AIX32-NEXT: bl .calleeInt[PR]
; AIX32-NEXT: nop
Expand Down
18 changes: 8 additions & 10 deletions llvm/test/CodeGen/PowerPC/mergeable-string-pool-pass-only.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}

Expand All @@ -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
}

Expand All @@ -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
}

Expand All @@ -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
Expand Down
14 changes: 7 additions & 7 deletions llvm/test/CodeGen/PowerPC/mergeable-string-pool.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,16 +398,16 @@ define dso_local signext i32 @array1() local_unnamed_addr #0 {
; AIX32: # %bb.0: # %entry
; AIX32-NEXT: mflr r0
; AIX32-NEXT: stwu r1, -96(r1)
; AIX32-NEXT: lwz r4, L..C0(r2) # @__ModuleStringPool
; AIX32-NEXT: lwz r5, L..C0(r2) # @__ModuleStringPool
; AIX32-NEXT: li r6, 372
; AIX32-NEXT: li r5, 12
; AIX32-NEXT: li r4, 12
; AIX32-NEXT: addi r3, r1, 64
; AIX32-NEXT: stw r0, 104(r1)
; AIX32-NEXT: rlwimi r5, r3, 0, 30, 27
; AIX32-NEXT: lxvw4x vs0, r4, r6
; AIX32-NEXT: stxvw4x vs0, 0, r5
; AIX32-NEXT: li r5, 360
; AIX32-NEXT: lxvw4x vs0, r4, r5
; AIX32-NEXT: rlwimi r4, r3, 0, 30, 27
; AIX32-NEXT: lxvw4x vs0, r5, r6
; AIX32-NEXT: stxvw4x vs0, 0, r4
; AIX32-NEXT: li r4, 360
; AIX32-NEXT: lxvw4x vs0, r5, r4
; AIX32-NEXT: stxvw4x vs0, 0, r3
; AIX32-NEXT: bl .calleeInt[PR]
; AIX32-NEXT: nop
Expand Down

0 comments on commit 1184a9c

Please sign in to comment.