[llvm-branch-commits] [llvm] release/18.x: [PPCMergeStringPool] Avoid replacing constant with instruction (#88846) (PR #91557)
tstellar wrote: @nikic (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. https://github.com/llvm/llvm-project/pull/91557 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] release/18.x: [PPCMergeStringPool] Avoid replacing constant with instruction (#88846) (PR #91557)
https://github.com/tstellar closed https://github.com/llvm/llvm-project/pull/91557 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] release/18.x: [PPCMergeStringPool] Avoid replacing constant with instruction (#88846) (PR #91557)
https://github.com/tstellar updated
https://github.com/llvm/llvm-project/pull/91557
>From 1184a9cb30e6a12c883b918867f2f06bc3096fc0 Mon Sep 17 00:00:00 2001
From: Nikita Popov
Date: Thu, 9 May 2024 13:27:20 +0900
Subject: [PATCH] [PPCMergeStringPool] Avoid replacing constant with
instruction (#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 https://github.com/llvm/llvm-project/issues/88844.
(cherry picked from commit 3a3aeb8eba40e981d3a9ff92175f949c2f3d4434)
---
.../lib/Target/PowerPC/PPCMergeStringPool.cpp | 57 ++-
.../mergeable-string-pool-exceptions.ll | 47 +++
.../PowerPC/mergeable-string-pool-large.ll| 14 ++---
.../mergeable-string-pool-pass-only.mir | 18 +++---
.../CodeGen/PowerPC/mergeable-string-pool.ll | 14 ++---
5 files changed, 87 insertions(+), 63 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 d9465e86d8966..ebd876d50c44e 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"
@@ -116,9 +117,20 @@ class PPCMergeStringPool : public ModulePass {
// sure that they can be replaced.
static bool hasReplaceableUsers(GlobalVariable ) {
for (User *CurrentUser : GV.users()) {
-// Instruction users are always valid.
-if (isa(CurrentUser))
+if (auto *I = dyn_cast(CurrentUser)) {
+ // Do not merge globals in exception pads.
+ if (I->isEHPad())
+return false;
+
+ if (auto *II = dyn_cast(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
@@ -302,14 +314,6 @@ void PPCMergeStringPool::replaceUsesWithGEP(GlobalVariable
*GlobalToReplace,
Users.push_back(CurrentUser);
for (User *CurrentUser : Users) {
-Instruction *UserInstruction = dyn_cast(CurrentUser);
-Constant *UserConstant = dyn_cast(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;
@@ -318,38 +322,13 @@ void
PPCMergeStringPool::replaceUsesWithGEP(GlobalVariable *GlobalToReplace,
if (isa(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(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-branch-commits] [llvm] release/18.x: [PPCMergeStringPool] Avoid replacing constant with instruction (#88846) (PR #91557)
https://github.com/chenzheng1030 approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/91557 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] release/18.x: [PPCMergeStringPool] Avoid replacing constant with instruction (#88846) (PR #91557)
https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/91557
>From d17c08705c750388a4ae586dc9fb892d81dba5ca Mon Sep 17 00:00:00 2001
From: Nikita Popov
Date: Thu, 9 May 2024 13:27:20 +0900
Subject: [PATCH] [PPCMergeStringPool] Avoid replacing constant with
instruction (#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 https://github.com/llvm/llvm-project/issues/88844.
(cherry picked from commit 3a3aeb8eba40e981d3a9ff92175f949c2f3d4434)
---
.../lib/Target/PowerPC/PPCMergeStringPool.cpp | 57 ++-
.../mergeable-string-pool-exceptions.ll | 47 +++
.../PowerPC/mergeable-string-pool-large.ll| 14 ++---
.../mergeable-string-pool-pass-only.mir | 18 +++---
.../CodeGen/PowerPC/mergeable-string-pool.ll | 14 ++---
5 files changed, 87 insertions(+), 63 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 d9465e86d8966..ebd876d50c44e 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"
@@ -116,9 +117,20 @@ class PPCMergeStringPool : public ModulePass {
// sure that they can be replaced.
static bool hasReplaceableUsers(GlobalVariable ) {
for (User *CurrentUser : GV.users()) {
-// Instruction users are always valid.
-if (isa(CurrentUser))
+if (auto *I = dyn_cast(CurrentUser)) {
+ // Do not merge globals in exception pads.
+ if (I->isEHPad())
+return false;
+
+ if (auto *II = dyn_cast(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
@@ -302,14 +314,6 @@ void PPCMergeStringPool::replaceUsesWithGEP(GlobalVariable
*GlobalToReplace,
Users.push_back(CurrentUser);
for (User *CurrentUser : Users) {
-Instruction *UserInstruction = dyn_cast(CurrentUser);
-Constant *UserConstant = dyn_cast(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;
@@ -318,38 +322,13 @@ void
PPCMergeStringPool::replaceUsesWithGEP(GlobalVariable *GlobalToReplace,
if (isa(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(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-branch-commits] [llvm] release/18.x: [PPCMergeStringPool] Avoid replacing constant with instruction (#88846) (PR #91557)
https://github.com/nikic created https://github.com/llvm/llvm-project/pull/91557
Backport of 3a3aeb8eba40e981d3a9ff92175f949c2f3d4434 to the release branch.
>From 7764bb3a47f241ca4e4d3fe42e96ab6bdecbdbe0 Mon Sep 17 00:00:00 2001
From: Nikita Popov
Date: Thu, 9 May 2024 13:27:20 +0900
Subject: [PATCH] [PPCMergeStringPool] Avoid replacing constant with
instruction (#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 https://github.com/llvm/llvm-project/issues/88844.
(cherry picked from commit 3a3aeb8eba40e981d3a9ff92175f949c2f3d4434)
---
.../lib/Target/PowerPC/PPCMergeStringPool.cpp | 57 ++-
.../mergeable-string-pool-exceptions.ll | 47 +++
.../mergeable-string-pool-pass-only.mir | 18 +++---
3 files changed, 73 insertions(+), 49 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 d9465e86d8966..ebd876d50c44e 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"
@@ -116,9 +117,20 @@ class PPCMergeStringPool : public ModulePass {
// sure that they can be replaced.
static bool hasReplaceableUsers(GlobalVariable ) {
for (User *CurrentUser : GV.users()) {
-// Instruction users are always valid.
-if (isa(CurrentUser))
+if (auto *I = dyn_cast(CurrentUser)) {
+ // Do not merge globals in exception pads.
+ if (I->isEHPad())
+return false;
+
+ if (auto *II = dyn_cast(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
@@ -302,14 +314,6 @@ void PPCMergeStringPool::replaceUsesWithGEP(GlobalVariable
*GlobalToReplace,
Users.push_back(CurrentUser);
for (User *CurrentUser : Users) {
-Instruction *UserInstruction = dyn_cast(CurrentUser);
-Constant *UserConstant = dyn_cast(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;
@@ -318,38 +322,13 @@ void
PPCMergeStringPool::replaceUsesWithGEP(GlobalVariable *GlobalToReplace,
if (isa(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(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());
-
-//
[llvm-branch-commits] [llvm] release/18.x: [PPCMergeStringPool] Avoid replacing constant with instruction (#88846) (PR #91557)
https://github.com/nikic milestoned https://github.com/llvm/llvm-project/pull/91557 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
