From 97d3aec3c120741f36ec9e3724bfa155f639d82d Mon Sep 17 00:00:00 2001 From: Cullen Rhodes Date: Mon, 23 Jun 2025 14:30:29 +0000 Subject: [PATCH 1/4] [InstCombine] Precommit test --- llvm/test/Transforms/InstCombine/freeze.ll | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/llvm/test/Transforms/InstCombine/freeze.ll b/llvm/test/Transforms/InstCombine/freeze.ll index 9733f1b732c3f..426c4373a8782 100644 --- a/llvm/test/Transforms/InstCombine/freeze.ll +++ b/llvm/test/Transforms/InstCombine/freeze.ll @@ -142,6 +142,17 @@ define i32 @early_freeze_test3(i32 %v1) { ret i32 %v4.fr } +define i32 @early_freeze_test4(i32 %v1) { +; CHECK-LABEL: @early_freeze_test4( +; CHECK-NEXT: [[V2:%.*]] = mul i32 [[V1_FR:%.*]], [[V1_FR]] +; CHECK-NEXT: [[V2_FR:%.*]] = freeze i32 [[V2]] +; CHECK-NEXT: ret i32 [[V2_FR]] +; + %v2 = mul i32 %v1, %v1 + %v2.fr = freeze i32 %v2 + ret i32 %v2.fr +} + ; If replace all dominated uses of v to freeze(v). define void @freeze_dominated_uses_test1(i32 %v) { From 10e8258ca73ea709689e0104267935564830986a Mon Sep 17 00:00:00 2001 From: Cullen Rhodes Date: Mon, 23 Jun 2025 14:34:18 +0000 Subject: [PATCH 2/4] [InstCombine] Treat identical operands as one in pushFreezeToPreventPoisonFromPropagating To push a freeze through an instruction, only one operand may produce poison. However, this currently fails for identical operands which are treated as separate. This patch fixes this by treating them as a single operand. --- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 4 +++- llvm/test/Transforms/InstCombine/freeze.ll | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 91a1b61ddc483..8bb4d3bce4a5d 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -4902,7 +4902,9 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) { Use *MaybePoisonOperand = nullptr; for (Use &U : OrigOpInst->operands()) { if (isa(U.get()) || - isGuaranteedNotToBeUndefOrPoison(U.get())) + isGuaranteedNotToBeUndefOrPoison(U.get()) || + // Treat identical operands as a single operand. + (MaybePoisonOperand && MaybePoisonOperand->get() == U.get())) continue; if (!MaybePoisonOperand) MaybePoisonOperand = &U; diff --git a/llvm/test/Transforms/InstCombine/freeze.ll b/llvm/test/Transforms/InstCombine/freeze.ll index 426c4373a8782..3fedead2feab8 100644 --- a/llvm/test/Transforms/InstCombine/freeze.ll +++ b/llvm/test/Transforms/InstCombine/freeze.ll @@ -144,9 +144,9 @@ define i32 @early_freeze_test3(i32 %v1) { define i32 @early_freeze_test4(i32 %v1) { ; CHECK-LABEL: @early_freeze_test4( -; CHECK-NEXT: [[V2:%.*]] = mul i32 [[V1_FR:%.*]], [[V1_FR]] -; CHECK-NEXT: [[V2_FR:%.*]] = freeze i32 [[V2]] -; CHECK-NEXT: ret i32 [[V2_FR]] +; CHECK-NEXT: [[V2_FR:%.*]] = freeze i32 [[V2:%.*]] +; CHECK-NEXT: [[V3:%.*]] = mul i32 [[V2_FR]], [[V2_FR]] +; CHECK-NEXT: ret i32 [[V3]] ; %v2 = mul i32 %v1, %v1 %v2.fr = freeze i32 %v2 From 1e487705d7d393da457799fefde8b5507a4c6f8f Mon Sep 17 00:00:00 2001 From: Cullen Rhodes Date: Tue, 15 Jul 2025 08:21:05 +0000 Subject: [PATCH 3/4] Switch code from working on Use* to Value* and replace all uses --- .../Transforms/InstCombine/InstructionCombining.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 8bb4d3bce4a5d..0ef6cf893d67e 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -4899,15 +4899,15 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) { // If operand is guaranteed not to be poison, there is no need to add freeze // to the operand. So we first find the operand that is not guaranteed to be // poison. - Use *MaybePoisonOperand = nullptr; + Value *MaybePoisonOperand = nullptr; for (Use &U : OrigOpInst->operands()) { if (isa(U.get()) || isGuaranteedNotToBeUndefOrPoison(U.get()) || // Treat identical operands as a single operand. - (MaybePoisonOperand && MaybePoisonOperand->get() == U.get())) + (MaybePoisonOperand && MaybePoisonOperand == U.get())) continue; if (!MaybePoisonOperand) - MaybePoisonOperand = &U; + MaybePoisonOperand = U.get(); else return nullptr; } @@ -4919,10 +4919,10 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) { return OrigOp; Builder.SetInsertPoint(OrigOpInst); - auto *FrozenMaybePoisonOperand = Builder.CreateFreeze( - MaybePoisonOperand->get(), MaybePoisonOperand->get()->getName() + ".fr"); + Value *FrozenMaybePoisonOperand = Builder.CreateFreeze( + MaybePoisonOperand, MaybePoisonOperand->getName() + ".fr"); - replaceUse(*MaybePoisonOperand, FrozenMaybePoisonOperand); + OrigOpInst->replaceUsesOfWith(MaybePoisonOperand, FrozenMaybePoisonOperand); return OrigOp; } From bac3bf19a82ba97f71bfec5c377b0cfe30b698f4 Mon Sep 17 00:00:00 2001 From: Cullen Rhodes Date: Wed, 16 Jul 2025 12:27:00 +0000 Subject: [PATCH 4/4] address comments --- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 0ef6cf893d67e..5e868fae17296 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -4900,14 +4900,13 @@ InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating(FreezeInst &OrigFI) { // to the operand. So we first find the operand that is not guaranteed to be // poison. Value *MaybePoisonOperand = nullptr; - for (Use &U : OrigOpInst->operands()) { - if (isa(U.get()) || - isGuaranteedNotToBeUndefOrPoison(U.get()) || + for (Value *V : OrigOpInst->operands()) { + if (isa(V) || isGuaranteedNotToBeUndefOrPoison(V) || // Treat identical operands as a single operand. - (MaybePoisonOperand && MaybePoisonOperand == U.get())) + (MaybePoisonOperand && MaybePoisonOperand == V)) continue; if (!MaybePoisonOperand) - MaybePoisonOperand = U.get(); + MaybePoisonOperand = V; else return nullptr; }