[llvm] 528c53e - Revert "[LowerTypeTests] Avoid creation of select constant expression"
Zequan Wu via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 13 09:47:14 PDT 2023
Author: Zequan Wu
Date: 2023-03-13T12:46:59-04:00
New Revision: 528c53ee28475ca4fc1d88adb9fb059c51b21a22
URL: https://github.com/llvm/llvm-project/commit/528c53ee28475ca4fc1d88adb9fb059c51b21a22
DIFF: https://github.com/llvm/llvm-project/commit/528c53ee28475ca4fc1d88adb9fb059c51b21a22.diff
LOG: Revert "[LowerTypeTests] Avoid creation of select constant expression"
This reverts commit 0317147a2848547ec97d8e76782f7dc38267a21f.
It causes broken module error when building chromium media_unittests
PHI nodes not grouped at top of basic block!
%19 = phi ptr [ %16, %15 ], [ %18, %12 ], !dbg !16
label %17
LLVM ERROR: Broken module found, compilation aborted!
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 8c4abc749cfac..549883094802b 100644
--- a/llvm/lib/IR/ReplaceConstant.cpp
+++ b/llvm/lib/IR/ReplaceConstant.cpp
@@ -134,58 +134,36 @@ 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 expandable direct users of Consts.
- SmallVector<Constant *> Stack;
+ // Find all ConstantExpr that are direct users Consts.
+ SmallVector<ConstantExpr *> Stack;
for (Constant *C : Consts)
for (User *U : C->users())
- if (isExpandableUser(U))
- Stack.push_back(cast<Constant>(U));
+ if (ConstantExpr *CE = dyn_cast<ConstantExpr>(U))
+ Stack.push_back(CE);
- // Include transitive users.
- SetVector<Constant *> ExpandableUsers;
+ // Expand to include constexpr users of direct users
+ SetVector<ConstantExpr *> ConstExprUsers;
while (!Stack.empty()) {
- Constant *C = Stack.pop_back_val();
- if (!ExpandableUsers.insert(C))
+ ConstantExpr *V = Stack.pop_back_val();
+ if (ConstExprUsers.contains(V))
continue;
- for (auto *Nested : C->users())
- if (isExpandableUser(Nested))
- Stack.push_back(cast<Constant>(Nested));
+ ConstExprUsers.insert(V);
+
+ for (auto *Nested : V->users())
+ if (ConstantExpr *CE = dyn_cast<ConstantExpr>(Nested))
+ Stack.push_back(CE);
}
- // Find all instructions that use any of the expandable users
+ // Find all instructions that use any of the ConstExpr users
SetVector<Instruction *> InstructionWorklist;
- for (Constant *C : ExpandableUsers)
- for (User *U : C->users())
+ for (ConstantExpr *CE : ConstExprUsers)
+ for (User *U : CE->users())
if (auto *I = dyn_cast<Instruction>(U))
InstructionWorklist.insert(I);
- // Replace those expandable operands with instructions
+ // Replace those ConstExpr operands with instructions
bool Changed = false;
while (!InstructionWorklist.empty()) {
Instruction *I = InstructionWorklist.pop_back_val();
@@ -198,20 +176,18 @@ bool convertUsersOfConstantsToInstructions(ArrayRef<Constant *> Consts) {
BI = &*It;
}
- if (auto *C = dyn_cast<Constant>(U.get())) {
- if (ExpandableUsers.contains(C)) {
+ if (ConstantExpr *C = dyn_cast<ConstantExpr>(U.get())) {
+ if (ConstExprUsers.contains(C)) {
Changed = true;
- Instruction *NI = expandUser(BI, C);
+ Instruction *NI = C->getAsInstruction(BI);
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 ccb824595cfcf..9ea05f975f4f9 100644
--- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -51,7 +51,6 @@
#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"
@@ -1369,17 +1368,11 @@ void LowerTypeTestsModule::replaceWeakDeclarationWithJumpTablePtr(
F->getAddressSpace(), "", &M);
replaceCfiUses(F, PlaceholderFn, IsJumpTableCanonical);
- convertUsersOfConstantsToInstructions(PlaceholderFn);
- for (Use &U : make_early_inc_range(PlaceholderFn->uses())) {
- auto *I = dyn_cast<Instruction>(U.getUser());
- assert(I && "Non-instruction users should have been eliminated");
- IRBuilder Builder(I);
- Value *ICmp = Builder.CreateICmp(CmpInst::ICMP_NE, F,
- Constant::getNullValue(F->getType()));
- Value *Select = Builder.CreateSelect(ICmp, JT,
- Constant::getNullValue(F->getType()));
- U.set(Select);
- }
+ Constant *Target = ConstantExpr::getSelect(
+ ConstantExpr::getICmp(CmpInst::ICMP_NE, F,
+ Constant::getNullValue(F->getType())),
+ JT, Constant::getNullValue(F->getType()));
+ PlaceholderFn->replaceAllUsesWith(Target);
PlaceholderFn->eraseFromParent();
}
diff --git a/llvm/test/Transforms/LowerTypeTests/cfi-direct-call1.ll b/llvm/test/Transforms/LowerTypeTests/cfi-direct-call1.ll
index 1b7039f905cc6..a684527313e67 100644
--- a/llvm/test/Transforms/LowerTypeTests/cfi-direct-call1.ll
+++ b/llvm/test/Transforms/LowerTypeTests/cfi-direct-call1.ll
@@ -56,8 +56,7 @@ 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: %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)
+; 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)
; Direct calls to extern_weak and extern_decl should go to original names
; FULL: %call5 = tail call i32 @extern_decl()
@@ -83,8 +82,7 @@ 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: %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
+; 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
; 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 f372fa0c8990a..670c0fc7475e8 100644
--- a/llvm/test/Transforms/LowerTypeTests/function-weak.ll
+++ b/llvm/test/Transforms/LowerTypeTests/function-weak.ll
@@ -33,9 +33,7 @@ declare !type !0 extern_weak void @f()
; CHECK: define zeroext i1 @check_f()
define zeroext i1 @check_f() {
entry:
-; CHECK: %0 = select i1 icmp ne (ptr @f, ptr null), ptr @[[JT:.*]], ptr null
-; CHECK: %1 = icmp ne ptr %0, null
-; ret i1 %1
+; CHECK: ret i1 icmp ne (ptr select (i1 icmp ne (ptr @f, ptr null), ptr @[[JT:.*]], ptr null), ptr null)
ret i1 icmp ne (ptr @f, ptr null)
}
@@ -60,21 +58,11 @@ define i1 @foo(ptr %p) {
; CHECK: define internal void @__cfi_global_var_init() section ".text.startup" {
; CHECK-NEXT: entry:
-; 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: 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: ret void
; CHECK-NEXT: }
More information about the llvm-commits
mailing list