[llvm] 5b86eae - Reapply [LowerTypeTests] Avoid creation of select constant expression
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 14 04:06:34 PDT 2023
Author: Nikita Popov
Date: 2023-03-14T12:06:24+01:00
New Revision: 5b86eaeb7e4d0b508e8dc9592b63361c5abb1e48
URL: https://github.com/llvm/llvm-project/commit/5b86eaeb7e4d0b508e8dc9592b63361c5abb1e48
DIFF: https://github.com/llvm/llvm-project/commit/5b86eaeb7e4d0b508e8dc9592b63361c5abb1e48.diff
LOG: Reapply [LowerTypeTests] Avoid creation of select constant expression
Reapply with a fix for phi handling: For phis, we need to insert
into the incoming block, not above the phi. This is especially
tricky if there are multiple incoming values from the same
predecessor, because these must all use the same value.
-----
LowerTypeTests replaces weak declarations with an icmp+select
constant expressions. As this is not a relocatable expression,
it additionally promotes initializers using it to global ctors.
As part of https://discourse.llvm.org/t/rfc-remove-most-constant-expressions/63179,
I would like to remove the select constant expression, of which LTT
is now the last user. This is a bit tricky, because we now need to
replace a constant with an instruction, which might require
converting intermediate constant expression users to instructions as
well.
We do this using the convertUsersOfConstantsToInstructions() helper.
However, it needs to be slightly extended to also support expansion
of ConstantAggregates. These are important in this context, because
the promotion of initializers to global ctors will produce stores
of such aggregates.
Differential Revision: https://reviews.llvm.org/D145247
Added:
Modified:
llvm/lib/IR/ReplaceConstant.cpp
llvm/lib/Transforms/IPO/LowerTypeTests.cpp
llvm/test/Transforms/LowerTypeTests/cfi-direct-call1.ll
llvm/test/Transforms/LowerTypeTests/function-weak.ll
Removed:
################################################################################
diff --git a/llvm/lib/IR/ReplaceConstant.cpp b/llvm/lib/IR/ReplaceConstant.cpp
index 549883094802b..8c4abc749cfac 100644
--- a/llvm/lib/IR/ReplaceConstant.cpp
+++ b/llvm/lib/IR/ReplaceConstant.cpp
@@ -134,36 +134,58 @@ void collectConstantExprPaths(
}
}
+static bool isExpandableUser(User *U) {
+ return isa<ConstantExpr>(U) || isa<ConstantAggregate>(U);
+}
+
+static Instruction *expandUser(Instruction *InsertPt, Constant *C) {
+ if (auto *CE = dyn_cast<ConstantExpr>(C)) {
+ return CE->getAsInstruction(InsertPt);
+ } else if (isa<ConstantStruct>(C) || isa<ConstantArray>(C)) {
+ Value *V = PoisonValue::get(C->getType());
+ for (auto [Idx, Op] : enumerate(C->operands()))
+ V = InsertValueInst::Create(V, Op, Idx, "", InsertPt);
+ return cast<Instruction>(V);
+ } else if (isa<ConstantVector>(C)) {
+ Type *IdxTy = Type::getInt32Ty(C->getContext());
+ Value *V = PoisonValue::get(C->getType());
+ for (auto [Idx, Op] : enumerate(C->operands()))
+ V = InsertElementInst::Create(V, Op, ConstantInt::get(IdxTy, Idx), "",
+ InsertPt);
+ return cast<Instruction>(V);
+ } else {
+ llvm_unreachable("Not an expandable user");
+ }
+}
+
bool convertUsersOfConstantsToInstructions(ArrayRef<Constant *> Consts) {
- // Find all ConstantExpr that are direct users Consts.
- SmallVector<ConstantExpr *> Stack;
+ // Find all expandable direct users of Consts.
+ SmallVector<Constant *> Stack;
for (Constant *C : Consts)
for (User *U : C->users())
- if (ConstantExpr *CE = dyn_cast<ConstantExpr>(U))
- Stack.push_back(CE);
+ if (isExpandableUser(U))
+ Stack.push_back(cast<Constant>(U));
- // Expand to include constexpr users of direct users
- SetVector<ConstantExpr *> ConstExprUsers;
+ // Include transitive users.
+ SetVector<Constant *> ExpandableUsers;
while (!Stack.empty()) {
- ConstantExpr *V = Stack.pop_back_val();
- if (ConstExprUsers.contains(V))
+ Constant *C = Stack.pop_back_val();
+ if (!ExpandableUsers.insert(C))
continue;
- ConstExprUsers.insert(V);
-
- for (auto *Nested : V->users())
- if (ConstantExpr *CE = dyn_cast<ConstantExpr>(Nested))
- Stack.push_back(CE);
+ for (auto *Nested : C->users())
+ if (isExpandableUser(Nested))
+ Stack.push_back(cast<Constant>(Nested));
}
- // Find all instructions that use any of the ConstExpr users
+ // Find all instructions that use any of the expandable users
SetVector<Instruction *> InstructionWorklist;
- for (ConstantExpr *CE : ConstExprUsers)
- for (User *U : CE->users())
+ for (Constant *C : ExpandableUsers)
+ for (User *U : C->users())
if (auto *I = dyn_cast<Instruction>(U))
InstructionWorklist.insert(I);
- // Replace those ConstExpr operands with instructions
+ // Replace those expandable operands with instructions
bool Changed = false;
while (!InstructionWorklist.empty()) {
Instruction *I = InstructionWorklist.pop_back_val();
@@ -176,18 +198,20 @@ bool convertUsersOfConstantsToInstructions(ArrayRef<Constant *> Consts) {
BI = &*It;
}
- if (ConstantExpr *C = dyn_cast<ConstantExpr>(U.get())) {
- if (ConstExprUsers.contains(C)) {
+ if (auto *C = dyn_cast<Constant>(U.get())) {
+ if (ExpandableUsers.contains(C)) {
Changed = true;
- Instruction *NI = C->getAsInstruction(BI);
+ Instruction *NI = expandUser(BI, C);
InstructionWorklist.insert(NI);
U.set(NI);
- C->removeDeadConstantUsers();
}
}
}
}
+ for (Constant *C : Consts)
+ C->removeDeadConstantUsers();
+
return Changed;
}
diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
index 9ea05f975f4f9..8380afaaaf982 100644
--- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -51,6 +51,7 @@
#include "llvm/IR/ModuleSummaryIndexYAML.h"
#include "llvm/IR/Operator.h"
#include "llvm/IR/PassManager.h"
+#include "llvm/IR/ReplaceConstant.h"
#include "llvm/IR/Type.h"
#include "llvm/IR/Use.h"
#include "llvm/IR/User.h"
@@ -1368,11 +1369,27 @@ void LowerTypeTestsModule::replaceWeakDeclarationWithJumpTablePtr(
F->getAddressSpace(), "", &M);
replaceCfiUses(F, PlaceholderFn, IsJumpTableCanonical);
- Constant *Target = ConstantExpr::getSelect(
- ConstantExpr::getICmp(CmpInst::ICMP_NE, F,
- Constant::getNullValue(F->getType())),
- JT, Constant::getNullValue(F->getType()));
- PlaceholderFn->replaceAllUsesWith(Target);
+ convertUsersOfConstantsToInstructions(PlaceholderFn);
+ // Don't use range based loop, because use list will be modified.
+ while (!PlaceholderFn->use_empty()) {
+ Use &U = *PlaceholderFn->use_begin();
+ auto *InsertPt = dyn_cast<Instruction>(U.getUser());
+ assert(InsertPt && "Non-instruction users should have been eliminated");
+ auto *PN = dyn_cast<PHINode>(InsertPt);
+ if (PN)
+ InsertPt = PN->getIncomingBlock(U)->getTerminator();
+ IRBuilder Builder(InsertPt);
+ Value *ICmp = Builder.CreateICmp(CmpInst::ICMP_NE, F,
+ Constant::getNullValue(F->getType()));
+ Value *Select = Builder.CreateSelect(ICmp, JT,
+ Constant::getNullValue(F->getType()));
+ // For phi nodes, we need to update the incoming value for all operands
+ // with the same predecessor.
+ if (PN)
+ PN->setIncomingValueForBlock(InsertPt->getParent(), Select);
+ else
+ U.set(Select);
+ }
PlaceholderFn->eraseFromParent();
}
diff --git a/llvm/test/Transforms/LowerTypeTests/cfi-direct-call1.ll b/llvm/test/Transforms/LowerTypeTests/cfi-direct-call1.ll
index a684527313e67..1b7039f905cc6 100644
--- a/llvm/test/Transforms/LowerTypeTests/cfi-direct-call1.ll
+++ b/llvm/test/Transforms/LowerTypeTests/cfi-direct-call1.ll
@@ -56,7 +56,8 @@ entry:
; FULL: %fptr1 = select i1 %cmp.i, ptr @local_func1, ptr @local_func2
; Indirect references to extern_weak and extern_decl must go through jump table
-; FULL: %fptr2 = select i1 %cmp.i, ptr select (i1 icmp ne (ptr @extern_weak, ptr null), ptr getelementptr inbounds ([4 x [8 x i8]], ptr @.cfi.jumptable, i64 0, i64 2), ptr null), ptr getelementptr inbounds ([4 x [8 x i8]], ptr @.cfi.jumptable, i64 0, i64 3)
+; FULL: %0 = select i1 icmp ne (ptr @extern_weak, ptr null), ptr getelementptr inbounds ([4 x [8 x i8]], ptr @.cfi.jumptable, i64 0, i64 2), ptr null
+; FULL: %fptr2 = select i1 %cmp.i, ptr %0, ptr getelementptr inbounds ([4 x [8 x i8]], ptr @.cfi.jumptable, i64 0, i64 3)
; Direct calls to extern_weak and extern_decl should go to original names
; FULL: %call5 = tail call i32 @extern_decl()
@@ -82,7 +83,8 @@ entry:
; THIN: %fptr1 = select i1 %cmp.i, ptr @local_func1, ptr @local_func2
; Indirect references to extern_weak and extern_decl must go through jump table
-; THIN: %fptr2 = select i1 %cmp.i, ptr select (i1 icmp ne (ptr @extern_weak, ptr null), ptr @extern_weak.cfi_jt, ptr null), ptr @extern_decl.cfi_jt
+; THIN: %0 = select i1 icmp ne (ptr @extern_weak, ptr null), ptr @extern_weak.cfi_jt, ptr null
+; THIN: %fptr2 = select i1 %cmp.i, ptr %0, ptr @extern_decl.cfi_jt
; Direct calls to extern_weak and extern_decl should go to original names
; THIN: %call5 = tail call i32 @extern_decl()
diff --git a/llvm/test/Transforms/LowerTypeTests/function-weak.ll b/llvm/test/Transforms/LowerTypeTests/function-weak.ll
index 670c0fc7475e8..ff69abacc8e93 100644
--- a/llvm/test/Transforms/LowerTypeTests/function-weak.ll
+++ b/llvm/test/Transforms/LowerTypeTests/function-weak.ll
@@ -33,7 +33,9 @@ declare !type !0 extern_weak void @f()
; CHECK: define zeroext i1 @check_f()
define zeroext i1 @check_f() {
entry:
-; CHECK: ret i1 icmp ne (ptr select (i1 icmp ne (ptr @f, ptr null), ptr @[[JT:.*]], ptr null), ptr null)
+; CHECK: %0 = select i1 icmp ne (ptr @f, ptr null), ptr @[[JT:.*]], ptr null
+; CHECK: %1 = icmp ne ptr %0, null
+; ret i1 %1
ret i1 icmp ne (ptr @f, ptr null)
}
@@ -45,6 +47,65 @@ entry:
ret void
}
+define void @phi(i1 %c) {
+; CHECK-LABEL: define void @phi(i1 %c) {
+; CHECK: entry:
+; CHECK: %0 = select i1 icmp ne (ptr @f, ptr null), ptr @.cfi.jumptable, ptr null
+; CHECK: br i1 %c, label %if, label %join
+; CHECK: if:
+; CHECK: %1 = select i1 icmp ne (ptr @f, ptr null), ptr @.cfi.jumptable, ptr null
+; CHECK: br label %join
+; CHECK: join:
+; CHECK: %phi = phi ptr [ %1, %if ], [ null, %entry ]
+; CHECK: %phi2 = phi ptr [ null, %if ], [ %0, %entry ]
+
+entry:
+ br i1 %c, label %if, label %join
+
+if:
+ br label %join
+
+join:
+ %phi = phi ptr [ @f, %if ], [ null, %entry ]
+ %phi2 = phi ptr [ null, %if ], [ @f, %entry ]
+ ret void
+}
+
+define void @phi2(i1 %c, i32 %x) {
+; CHECK-LABEL: define void @phi2(i1 %c, i32 %x) {
+; CHECK: entry:
+; CHECK: br i1 %c, label %if, label %else
+; CHECK: if: ; preds = %entry
+; CHECK: %0 = select i1 icmp ne (ptr @f, ptr null), ptr @.cfi.jumptable, ptr null
+; CHECK: switch i32 %x, label %join [
+; CHECK: i32 0, label %join
+; CHECK: ]
+; CHECK: else: ; preds = %entry
+; CHECK: %1 = select i1 icmp ne (ptr @f, ptr null), ptr @.cfi.jumptable, ptr null
+; CHECK: switch i32 %x, label %join [
+; CHECK: i32 0, label %join
+; CHECK: ]
+; CHECK: join: ; preds = %else, %else, %if, %if
+; CHECK: %phi2 = phi ptr [ %0, %if ], [ %0, %if ], [ %1, %else ], [ %1, %else ]
+
+entry:
+ br i1 %c, label %if, label %else
+
+if:
+ switch i32 %x, label %join [
+ i32 0, label %join
+ ]
+
+else:
+ switch i32 %x, label %join [
+ i32 0, label %join
+ ]
+
+join:
+ %phi2 = phi ptr [ @f, %if ], [ @f, %if ], [ @f, %else ], [ @f, %else ]
+ ret void
+}
+
declare i1 @llvm.type.test(ptr %ptr, metadata %bitset) nounwind readnone
define i1 @foo(ptr %p) {
@@ -58,11 +119,21 @@ define i1 @foo(ptr %p) {
; CHECK: define internal void @__cfi_global_var_init() section ".text.startup" {
; CHECK-NEXT: entry:
-; CHECK-NEXT: store ptr select (i1 icmp ne (ptr @f, ptr null), ptr @[[JT]], ptr null), ptr @x, align 8
-; CHECK-NEXT: store ptr select (i1 icmp ne (ptr @f, ptr null), ptr @[[JT]], ptr null), ptr @x2, align 8
-; CHECK-NEXT: store ptr select (i1 icmp ne (ptr @f, ptr null), ptr @[[JT]], ptr null), ptr @x3, align 8
-; CHECK-NEXT: store ptr getelementptr (i8, ptr select (i1 icmp ne (ptr @f, ptr null), ptr @[[JT]], ptr null), i64 42), ptr @x4, align 8
-; CHECK-NEXT: store { ptr, ptr, i32 } { ptr select (i1 icmp ne (ptr @f, ptr null), ptr @[[JT]], ptr null), ptr select (i1 icmp ne (ptr @f, ptr null), ptr @[[JT]], ptr null), i32 42 }, ptr @s, align 8
+; CHECK-NEXT: %0 = select i1 icmp ne (ptr @f, ptr null), ptr @[[JT]], ptr null
+; CHECK-NEXT: store ptr %0, ptr @x, align 8
+; CHECK-NEXT: %1 = select i1 icmp ne (ptr @f, ptr null), ptr @[[JT]], ptr null
+; CHECK-NEXT: store ptr %1, ptr @x2, align 8
+; CHECK-NEXT: %2 = select i1 icmp ne (ptr @f, ptr null), ptr @[[JT]], ptr null
+; CHECK-NEXT: store ptr %2, ptr @x3, align 8
+; CHECK-NEXT: %3 = select i1 icmp ne (ptr @f, ptr null), ptr @[[JT]], ptr null
+; CHECK-NEXT: %4 = getelementptr i8, ptr %3, i64 42
+; CHECK-NEXT: store ptr %4, ptr @x4, align 8
+; CHECK-NEXT: %5 = select i1 icmp ne (ptr @f, ptr null), ptr @[[JT]], ptr null
+; CHECK-NEXT: %6 = insertvalue { ptr, ptr, i32 } poison, ptr %5, 0
+; CHECK-NEXT: %7 = select i1 icmp ne (ptr @f, ptr null), ptr @[[JT]], ptr null
+; CHECK-NEXT: %8 = insertvalue { ptr, ptr, i32 } %6, ptr %7, 1
+; CHECK-NEXT: %9 = insertvalue { ptr, ptr, i32 } %8, i32 42, 2
+; CHECK-NEXT: store { ptr, ptr, i32 } %9, ptr @s, align 8
; CHECK-NEXT: ret void
; CHECK-NEXT: }
More information about the llvm-commits
mailing list