From 1178db4cc62ad4abdac2f8e653fda94cbfc49142 Mon Sep 17 00:00:00 2001 From: Andreas Jonson Date: Sat, 31 May 2025 10:42:44 +0200 Subject: [PATCH 1/4] [InstCombine] Change value to avoid change of purpose of test with new fold (NFC) --- llvm/test/DebugInfo/X86/instcombine-fold-cast-into-phi.ll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/test/DebugInfo/X86/instcombine-fold-cast-into-phi.ll b/llvm/test/DebugInfo/X86/instcombine-fold-cast-into-phi.ll index a91a983be49bc..3ec226fc58d6b 100644 --- a/llvm/test/DebugInfo/X86/instcombine-fold-cast-into-phi.ll +++ b/llvm/test/DebugInfo/X86/instcombine-fold-cast-into-phi.ll @@ -4,7 +4,7 @@ ;; user (otherwise the dbg use becomes poison after the original phi is ;; deleted). Check the new phi inherits the DebugLoc. -; CHECK: %[[phi:.*]] = phi i8 [ 1, %{{.*}} ], [ 0, %{{.*}} ], !dbg ![[dbg:[0-9]+]] +; CHECK: %[[phi:.*]] = phi i8 [ 2, %{{.*}} ], [ 0, %{{.*}} ], !dbg ![[dbg:[0-9]+]] ; CHECK: #dbg_value(i8 %[[phi]], ![[#]], !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value) ; CHECK: ![[dbg]] = !DILocation(line: 123, @@ -19,7 +19,7 @@ if.then: ; preds = entry br label %if.end if.end: ; preds = %if.then, %entry - %p.0 = phi i32 [ 1, %if.then ], [ 0, %entry ], !dbg !13 + %p.0 = phi i32 [ 2, %if.then ], [ 0, %entry ], !dbg !13 call void @llvm.dbg.value(metadata i32 %p.0, metadata !4, metadata !DIExpression()), !dbg !13 %x = trunc i32 %p.0 to i8 %callff = call float @ff(i8 %x) From b9cd63ea1d6332b1bcf1184a068083b96a80e46d Mon Sep 17 00:00:00 2001 From: Andreas Jonson Date: Sat, 7 Jun 2025 11:45:25 +0200 Subject: [PATCH 2/4] [InstCombine] Add cast support in simplifyUsingControlFlow --- .../Transforms/InstCombine/InstCombinePHI.cpp | 117 ++++++++++++------ .../Transforms/InstCombine/phi-int-users.ll | 2 +- .../InstCombine/simple_phi_condition.ll | 19 +-- 3 files changed, 91 insertions(+), 47 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp index a842a5edcb8a3..18e94e94ab788 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp @@ -1301,6 +1301,7 @@ static Value *simplifyUsingControlFlow(InstCombiner &Self, PHINode &PN, // Determine which value the condition of the idom has for which successor. LLVMContext &Context = PN.getContext(); + unsigned PNBitWidth = PN.getType()->getScalarSizeInBits(); auto *IDom = DT.getNode(BB)->getIDom()->getBlock(); Value *Cond; SmallDenseMap SuccForValue; @@ -1318,61 +1319,101 @@ static Value *simplifyUsingControlFlow(InstCombiner &Self, PHINode &PN, AddSucc(ConstantInt::getFalse(Context), BI->getSuccessor(1)); } else if (auto *SI = dyn_cast(IDom->getTerminator())) { Cond = SI->getCondition(); + unsigned CondBitWidth = Cond->getType()->getScalarSizeInBits(); ++SuccCount[SI->getDefaultDest()]; - for (auto Case : SI->cases()) - AddSucc(Case.getCaseValue(), Case.getCaseSuccessor()); + for (auto Case : SI->cases()) { + ConstantInt *CaseValue = Case.getCaseValue(); + if (CondBitWidth > PNBitWidth) { + CaseValue = ConstantInt::get( + Context, Case.getCaseValue()->getValue().trunc(PNBitWidth)); + } + AddSucc(CaseValue, Case.getCaseSuccessor()); + } } else { return nullptr; } - if (Cond->getType() != PN.getType()) - return nullptr; + unsigned CondBitWidth = Cond->getType()->getScalarSizeInBits(); // Check that edges outgoing from the idom's terminators dominate respective // inputs of the Phi. - std::optional Invert; - for (auto Pair : zip(PN.incoming_values(), PN.blocks())) { - auto *Input = cast(std::get<0>(Pair)); - BasicBlock *Pred = std::get<1>(Pair); - auto IsCorrectInput = [&](ConstantInt *Input) { - // The input needs to be dominated by the corresponding edge of the idom. - // This edge cannot be a multi-edge, as that would imply that multiple - // different condition values follow the same edge. - auto It = SuccForValue.find(Input); - return It != SuccForValue.end() && SuccCount[It->second] == 1 && - DT.dominates(BasicBlockEdge(IDom, It->second), - BasicBlockEdge(Pred, BB)); - }; - - // Depending on the constant, the condition may need to be inverted. - bool NeedsInvert; - if (IsCorrectInput(Input)) - NeedsInvert = false; - else if (IsCorrectInput(cast(ConstantExpr::getNot(Input)))) - NeedsInvert = true; - else - return nullptr; + auto CheckInputs = [&](Instruction::CastOps CastType) -> std::optional { + std::optional Invert; + for (auto Pair : zip(PN.incoming_values(), PN.blocks())) { + auto *Input = cast(std::get<0>(Pair)); + BasicBlock *Pred = std::get<1>(Pair); + + auto IsCorrectInput = [&](ConstantInt *Input) { + if (CondBitWidth < PNBitWidth) { + if ((CastType == Instruction::SExt && + !Input->getValue().isSignedIntN(CondBitWidth)) || + (CastType == Instruction::ZExt && + !Input->getValue().isIntN(CondBitWidth))) + return false; + + Input = + ConstantInt::get(Context, Input->getValue().trunc(CondBitWidth)); + } + // The input needs to be dominated by the corresponding edge of the + // idom. This edge cannot be a multi-edge, as that would imply that + // multiple different condition values follow the same edge. + auto It = SuccForValue.find(Input); + return It != SuccForValue.end() && SuccCount[It->second] == 1 && + DT.dominates(BasicBlockEdge(IDom, It->second), + BasicBlockEdge(Pred, BB)); + }; + + // Depending on the constant, the condition may need to be inverted. + bool NeedsInvert; + if (IsCorrectInput(Input)) + NeedsInvert = false; + else if (IsCorrectInput(cast(ConstantExpr::getNot(Input)))) + NeedsInvert = true; + else + return std::nullopt; - // Make sure the inversion requirement is always the same. - if (Invert && *Invert != NeedsInvert) - return nullptr; + // Make sure the inversion requirement is always the same. + if (Invert && *Invert != NeedsInvert) + return std::nullopt; + + Invert = NeedsInvert; + } + return Invert; + }; + + Instruction::CastOps CastType = + CondBitWidth == PNBitWidth ? Instruction::BitCast + : CondBitWidth < PNBitWidth ? Instruction::ZExt + : Instruction::Trunc; - Invert = NeedsInvert; + auto Result = CheckInputs(CastType); + if (!Result && CondBitWidth < PNBitWidth) { + CastType = Instruction::SExt; + Result = CheckInputs(CastType); } + if (!Result) + return nullptr; + bool Invert = *Result; - if (!*Invert) + if (!Invert && CastType == Instruction::BitCast) return Cond; - // This Phi is actually opposite to branching condition of IDom. We invert - // the condition that will potentially open up some opportunities for - // sinking. auto InsertPt = BB->getFirstInsertionPt(); - if (InsertPt != BB->end()) { - Self.Builder.SetInsertPoint(&*BB, InsertPt); - return Self.Builder.CreateNot(Cond); + if (InsertPt == BB->end()) + return nullptr; + + Self.Builder.SetInsertPoint(&*BB, InsertPt); + + if (CastType != Instruction::BitCast) { + Cond = Self.Builder.CreateCast(CastType, Cond, PN.getType()); + if (!Invert) + return Cond; } - return nullptr; + // This Phi is actually opposite to branching condition of IDom. We invert + // the condition that will potentially open up some opportunities for + // sinking. + return Self.Builder.CreateNot(Cond); } // Fold iv = phi(start, iv.next = iv2.next op start) diff --git a/llvm/test/Transforms/InstCombine/phi-int-users.ll b/llvm/test/Transforms/InstCombine/phi-int-users.ll index 6c98cc8a1c900..cc70feb160a08 100644 --- a/llvm/test/Transforms/InstCombine/phi-int-users.ll +++ b/llvm/test/Transforms/InstCombine/phi-int-users.ll @@ -13,7 +13,7 @@ define void @f1(i1 %a) { ; CHECK: [[BB2]]: ; CHECK-NEXT: br label %[[BB3]] ; CHECK: [[BB3]]: -; CHECK-NEXT: [[PHI:%.*]] = phi i64 [ 0, %[[BB2]] ], [ 1, %[[BB1]] ] +; CHECK-NEXT: [[PHI:%.*]] = zext i1 [[A]] to i64 ; CHECK-NEXT: [[INTTOPTR:%.*]] = inttoptr i64 [[PHI]] to ptr ; CHECK-NEXT: store i32 0, ptr [[INTTOPTR]], align 4 ; CHECK-NEXT: br label %[[BB1]] diff --git a/llvm/test/Transforms/InstCombine/simple_phi_condition.ll b/llvm/test/Transforms/InstCombine/simple_phi_condition.ll index e1a90d6a1c688..98ec7d217e4c1 100644 --- a/llvm/test/Transforms/InstCombine/simple_phi_condition.ll +++ b/llvm/test/Transforms/InstCombine/simple_phi_condition.ll @@ -669,7 +669,7 @@ define i32 @test_phi_to_zext(i8 noundef %0) { ; CHECK: sw.1: ; CHECK-NEXT: br label [[MERGE]] ; CHECK: merge: -; CHECK-NEXT: [[DOT0:%.*]] = phi i32 [ 1, [[SW_1]] ], [ 0, [[SW_0]] ], [ 255, [[SW_255]] ] +; CHECK-NEXT: [[DOT0:%.*]] = zext i8 [[TMP0]] to i32 ; CHECK-NEXT: ret i32 [[DOT0]] ; entry: @@ -713,7 +713,8 @@ define i32 @test_phi_to_zext_inverted(i8 noundef %0) { ; CHECK: sw.1: ; CHECK-NEXT: br label [[MERGE]] ; CHECK: merge: -; CHECK-NEXT: [[DOT0:%.*]] = phi i32 [ -256, [[SW_255]] ], [ -2, [[SW_1]] ], [ -1, [[SW_0]] ] +; CHECK-NEXT: [[TMP1:%.*]] = zext i8 [[TMP0]] to i32 +; CHECK-NEXT: [[DOT0:%.*]] = xor i32 [[TMP1]], -1 ; CHECK-NEXT: ret i32 [[DOT0]] ; entry: @@ -753,7 +754,7 @@ define i8 @test_multiple_predecessors_phi_to_zext(i1 %cond, i1 %cond2) { ; CHECK: if2.false: ; CHECK-NEXT: br label [[MERGE]] ; CHECK: merge: -; CHECK-NEXT: [[RET:%.*]] = phi i8 [ 0, [[IF_FALSE]] ], [ 1, [[IF2_TRUE]] ], [ 1, [[IF2_FALSE]] ] +; CHECK-NEXT: [[RET:%.*]] = zext i1 [[COND]] to i8 ; CHECK-NEXT: ret i8 [[RET]] ; entry: @@ -793,7 +794,7 @@ define i32 @test_phi_to_sext(i8 noundef %0) { ; CHECK: sw.1: ; CHECK-NEXT: br label [[MERGE]] ; CHECK: merge: -; CHECK-NEXT: [[DOT0:%.*]] = phi i32 [ 1, [[SW_1]] ], [ 0, [[SW_0]] ], [ -1, [[SW_255]] ] +; CHECK-NEXT: [[DOT0:%.*]] = sext i8 [[TMP0]] to i32 ; CHECK-NEXT: ret i32 [[DOT0]] ; entry: @@ -837,7 +838,8 @@ define i32 @test_phi_to_sext_inverted(i8 noundef %0) { ; CHECK: sw.1: ; CHECK-NEXT: br label [[MERGE]] ; CHECK: merge: -; CHECK-NEXT: [[DOT0:%.*]] = phi i32 [ -2, [[SW_1]] ], [ -1, [[SW_0]] ], [ 0, [[SW_255]] ] +; CHECK-NEXT: [[TMP1:%.*]] = xor i8 [[TMP0]], -1 +; CHECK-NEXT: [[DOT0:%.*]] = sext i8 [[TMP1]] to i32 ; CHECK-NEXT: ret i32 [[DOT0]] ; entry: @@ -877,7 +879,7 @@ define i8 @test_multiple_predecessors_phi_to_sext(i1 %cond, i1 %cond2) { ; CHECK: if2.false: ; CHECK-NEXT: br label [[MERGE]] ; CHECK: merge: -; CHECK-NEXT: [[RET:%.*]] = phi i8 [ 0, [[IF_FALSE]] ], [ -1, [[IF2_TRUE]] ], [ -1, [[IF2_FALSE]] ] +; CHECK-NEXT: [[RET:%.*]] = sext i1 [[COND]] to i8 ; CHECK-NEXT: ret i8 [[RET]] ; entry: @@ -995,7 +997,7 @@ define i8 @test_phi_to_trunc(i32 %0) { ; CHECK: sw.255: ; CHECK-NEXT: br label [[MERGE]] ; CHECK: merge: -; CHECK-NEXT: [[DOT0:%.*]] = phi i8 [ -1, [[SW_255]] ], [ 1, [[SW_1]] ], [ 0, [[ENTRY:%.*]] ] +; CHECK-NEXT: [[DOT0:%.*]] = trunc i32 [[TMP0]] to i8 ; CHECK-NEXT: ret i8 [[DOT0]] ; entry: @@ -1034,7 +1036,8 @@ define i8 @test_phi_to_trunc_inverted(i32 %0) { ; CHECK: sw.255: ; CHECK-NEXT: br label [[MERGE]] ; CHECK: merge: -; CHECK-NEXT: [[DOT0:%.*]] = phi i8 [ 0, [[SW_255]] ], [ -2, [[SW_1]] ], [ -1, [[ENTRY:%.*]] ] +; CHECK-NEXT: [[TMP1:%.*]] = trunc i32 [[TMP0]] to i8 +; CHECK-NEXT: [[DOT0:%.*]] = xor i8 [[TMP1]], -1 ; CHECK-NEXT: ret i8 [[DOT0]] ; entry: From c5d037c13bfac3904e592a7137a2f520c59ec592 Mon Sep 17 00:00:00 2001 From: Andreas Jonson Date: Thu, 19 Jun 2025 22:10:29 +0200 Subject: [PATCH 3/4] fixup! [InstCombine] Add cast support in simplifyUsingControlFlow --- llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp index 18e94e94ab788..2a55279663495 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp @@ -1290,6 +1290,8 @@ static Value *simplifyUsingControlFlow(InstCombiner &Self, PHINode &PN, // ... ... // \ / // phi [v1] [v2] + if (!PN.getType()->isIntegerTy()) + return nullptr; // Make sure all inputs are constants. if (!all_of(PN.operands(), [](Value *V) { return isa(V); })) return nullptr; From f3ade791fe0a62af6a956f71d4a3181c4662b1f8 Mon Sep 17 00:00:00 2001 From: Andreas Jonson Date: Sun, 29 Jun 2025 23:48:03 +0200 Subject: [PATCH 4/4] fixup! [InstCombine] Add cast support in simplifyUsingControlFlow --- .../Transforms/InstCombine/InstCombinePHI.cpp | 4 ++ .../Transforms/InstCombine/phi-int-users.ll | 2 +- .../InstCombine/simple_phi_condition.ll | 58 +++++++++++++++---- 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp index 2a55279663495..b82dd88309614 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp @@ -1317,6 +1317,10 @@ static Value *simplifyUsingControlFlow(InstCombiner &Self, PHINode &PN, return nullptr; Cond = BI->getCondition(); + + if (Cond->getType() != PN.getType()) + return nullptr; + AddSucc(ConstantInt::getTrue(Context), BI->getSuccessor(0)); AddSucc(ConstantInt::getFalse(Context), BI->getSuccessor(1)); } else if (auto *SI = dyn_cast(IDom->getTerminator())) { diff --git a/llvm/test/Transforms/InstCombine/phi-int-users.ll b/llvm/test/Transforms/InstCombine/phi-int-users.ll index cc70feb160a08..6c98cc8a1c900 100644 --- a/llvm/test/Transforms/InstCombine/phi-int-users.ll +++ b/llvm/test/Transforms/InstCombine/phi-int-users.ll @@ -13,7 +13,7 @@ define void @f1(i1 %a) { ; CHECK: [[BB2]]: ; CHECK-NEXT: br label %[[BB3]] ; CHECK: [[BB3]]: -; CHECK-NEXT: [[PHI:%.*]] = zext i1 [[A]] to i64 +; CHECK-NEXT: [[PHI:%.*]] = phi i64 [ 0, %[[BB2]] ], [ 1, %[[BB1]] ] ; CHECK-NEXT: [[INTTOPTR:%.*]] = inttoptr i64 [[PHI]] to ptr ; CHECK-NEXT: store i32 0, ptr [[INTTOPTR]], align 4 ; CHECK-NEXT: br label %[[BB1]] diff --git a/llvm/test/Transforms/InstCombine/simple_phi_condition.ll b/llvm/test/Transforms/InstCombine/simple_phi_condition.ll index 98ec7d217e4c1..61f834fa984eb 100644 --- a/llvm/test/Transforms/InstCombine/simple_phi_condition.ll +++ b/llvm/test/Transforms/InstCombine/simple_phi_condition.ll @@ -741,12 +741,20 @@ merge: ret i32 %.0 } -define i8 @test_multiple_predecessors_phi_to_zext(i1 %cond, i1 %cond2) { +define i8 @test_multiple_predecessors_phi_to_zext(i2 %cond, i2 %cond2) { ; CHECK-LABEL: @test_multiple_predecessors_phi_to_zext( ; CHECK-NEXT: entry: -; CHECK-NEXT: br i1 [[COND:%.*]], label [[IF_TRUE:%.*]], label [[IF_FALSE:%.*]] +; CHECK-NEXT: switch i2 [[COND:%.*]], label [[DEFAULT:%.*]] [ +; CHECK-NEXT: i2 1, label [[IF_TRUE:%.*]] +; CHECK-NEXT: i2 0, label [[IF_FALSE:%.*]] +; CHECK-NEXT: ] ; CHECK: if.true: -; CHECK-NEXT: br i1 [[COND2:%.*]], label [[IF2_TRUE:%.*]], label [[IF2_FALSE:%.*]] +; CHECK-NEXT: switch i2 [[COND2:%.*]], label [[DEFAULT]] [ +; CHECK-NEXT: i2 1, label [[IF2_TRUE:%.*]] +; CHECK-NEXT: i2 0, label [[IF2_FALSE:%.*]] +; CHECK-NEXT: ] +; CHECK: default: +; CHECK-NEXT: unreachable ; CHECK: if.false: ; CHECK-NEXT: br label [[MERGE:%.*]] ; CHECK: if2.true: @@ -754,14 +762,23 @@ define i8 @test_multiple_predecessors_phi_to_zext(i1 %cond, i1 %cond2) { ; CHECK: if2.false: ; CHECK-NEXT: br label [[MERGE]] ; CHECK: merge: -; CHECK-NEXT: [[RET:%.*]] = zext i1 [[COND]] to i8 +; CHECK-NEXT: [[RET:%.*]] = zext i2 [[COND]] to i8 ; CHECK-NEXT: ret i8 [[RET]] ; entry: - br i1 %cond, label %if.true, label %if.false + switch i2 %cond, label %default [ + i2 1, label %if.true + i2 0, label %if.false + ] if.true: - br i1 %cond2, label %if2.true, label %if2.false + switch i2 %cond2, label %default [ + i2 1, label %if2.true + i2 0, label %if2.false + ] + +default: + unreachable if.false: br label %merge @@ -866,12 +883,20 @@ merge: ret i32 %.0 } -define i8 @test_multiple_predecessors_phi_to_sext(i1 %cond, i1 %cond2) { +define i8 @test_multiple_predecessors_phi_to_sext(i2 %cond, i2 %cond2) { ; CHECK-LABEL: @test_multiple_predecessors_phi_to_sext( ; CHECK-NEXT: entry: -; CHECK-NEXT: br i1 [[COND:%.*]], label [[IF_TRUE:%.*]], label [[IF_FALSE:%.*]] +; CHECK-NEXT: switch i2 [[COND:%.*]], label [[DEFAULT:%.*]] [ +; CHECK-NEXT: i2 -1, label [[IF_TRUE:%.*]] +; CHECK-NEXT: i2 0, label [[IF_FALSE:%.*]] +; CHECK-NEXT: ] ; CHECK: if.true: -; CHECK-NEXT: br i1 [[COND2:%.*]], label [[IF2_TRUE:%.*]], label [[IF2_FALSE:%.*]] +; CHECK-NEXT: switch i2 [[COND2:%.*]], label [[DEFAULT]] [ +; CHECK-NEXT: i2 -1, label [[IF2_TRUE:%.*]] +; CHECK-NEXT: i2 0, label [[IF2_FALSE:%.*]] +; CHECK-NEXT: ] +; CHECK: default: +; CHECK-NEXT: unreachable ; CHECK: if.false: ; CHECK-NEXT: br label [[MERGE:%.*]] ; CHECK: if2.true: @@ -879,14 +904,23 @@ define i8 @test_multiple_predecessors_phi_to_sext(i1 %cond, i1 %cond2) { ; CHECK: if2.false: ; CHECK-NEXT: br label [[MERGE]] ; CHECK: merge: -; CHECK-NEXT: [[RET:%.*]] = sext i1 [[COND]] to i8 +; CHECK-NEXT: [[RET:%.*]] = sext i2 [[COND]] to i8 ; CHECK-NEXT: ret i8 [[RET]] ; entry: - br i1 %cond, label %if.true, label %if.false + switch i2 %cond, label %default [ + i2 3, label %if.true + i2 0, label %if.false + ] if.true: - br i1 %cond2, label %if2.true, label %if2.false + switch i2 %cond2, label %default [ + i2 3, label %if2.true + i2 0, label %if2.false + ] + +default: + unreachable if.false: br label %merge