diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -3520,7 +3520,7 @@ // guaranteed-non-poison operands then push the freeze through to the one // operand that is not guaranteed non poison. The actual transform is as // follows. - // Op1 = ... ; ; Op1 can be posion + // Op1 = ... ; Op1 can be posion // Op0 = Inst(Op1, NonPoisonOps...) ; Op0 has only one use and only have // ; single guaranteed-non-poison operands // ... = Freeze(Op0) @@ -3529,30 +3529,46 @@ // Op1.fr = Freeze(Op1) // ... = Inst(Op1.fr, NonPoisonOps...) auto* OrigOp = OrigFI.getOperand(0); - if (auto *OrigOpInst = dyn_cast(OrigOp)) { - if (OrigOpInst->hasOneUse() && !canCreateUndefOrPoison(dyn_cast(OrigOp))) { - Optional MaybePoisonOperand; - for (Use &U : OrigOpInst->operands()) { - if (isGuaranteedNotToBeUndefOrPoison(U.get())) - continue; - if (!MaybePoisonOperand) - MaybePoisonOperand = &U; - else if(MaybePoisonOperand) - return nullptr; - } - - if (!MaybePoisonOperand.hasValue()) - return nullptr; + auto *OrigOpInst = dyn_cast(OrigOp); - auto *MaybePoisonUse = MaybePoisonOperand.getValue(); - auto *FrozenMaybePoisonOperand = new FreezeInst(MaybePoisonUse->get(), MaybePoisonUse->getUser()->getName() + ".fr"); + // If Op0 is used in places other then freeze, wrong transformation will occur as below. + // Op1 = ... + // Op0 = Inst(Op1, NonPoisonOps...) + // Use(Op0) ; Op0 is not frozen. + // ... = Freeze(Op0) + // => + // Op1 = ... + // Op1.fr = Freeze(Op1) + // Op0.fr = Inst(Op1.fr, NonPoisonOps...) + // Use(Op0.fr) ; Op0.fr is already frozen. Wrong Transformation. + if (!OrigOpInst || !OrigOpInst->hasOneUse() || + canCreateUndefOrPoison(dyn_cast(OrigOp))) + return nullptr; - replaceUse(*MaybePoisonUse, FrozenMaybePoisonOperand); - FrozenMaybePoisonOperand->insertBefore(OrigOpInst); - return OrigOp; - } + // 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 guranteed to be + // poison. + Use *MaybePoisonOperand = nullptr; + for (Use &U : OrigOpInst->operands()) { + if (isGuaranteedNotToBeUndefOrPoison(U.get())) + continue; + if (!MaybePoisonOperand) + MaybePoisonOperand = &U; + else + return nullptr; } - return nullptr; + + // If all operands are guaranteed to be non-poison, we can drop freeze. + if (!MaybePoisonOperand) + return OrigOp; + + auto *FrozenMaybePoisonOperand = + new FreezeInst(MaybePoisonOperand->get(), + MaybePoisonOperand->get()->getName() + ".fr"); + + replaceUse(*MaybePoisonOperand, FrozenMaybePoisonOperand); + FrozenMaybePoisonOperand->insertBefore(OrigOpInst); + return OrigOp; } Instruction *InstCombinerImpl::visitFreeze(FreezeInst &I) { diff --git a/llvm/test/Transforms/InstCombine/freeze.ll b/llvm/test/Transforms/InstCombine/freeze.ll --- a/llvm/test/Transforms/InstCombine/freeze.ll +++ b/llvm/test/Transforms/InstCombine/freeze.ll @@ -105,6 +105,7 @@ } define i1 @early_freeze_test2(i32* %ptr) { +; ; CHECK-LABEL: @early_freeze_test2( ; CHECK-NEXT: [[V1:%.*]] = load i32, i32* [[PTR:%.*]], align 4 ; CHECK-NEXT: [[V1_FR:%.*]] = freeze i32 [[V1]]