[llvm] r228024 - Allow PRE to insert no-cost phi nodes
Jiangning Liu
liujiangning1 at gmail.com
Sat Feb 14 17:26:55 PST 2015
Hi Daniel,
Ping...
Thanks,
-Jiangning
2015-02-10 14:34 GMT+08:00 Jiangning Liu <liujiangning1 at gmail.com>:
> Hi Daniel,
>
> This commit caused ~5% performance regression for spec2006.401.bzip2 on
> aarch64.
>
> Don't know the root cause yet, but looking at the binary of decompress.o,
> it seems the code change is significant.
>
> Have you noticed the same issue?
>
> Thanks,
> -Jiangning
>
>
>
> 2015-02-04 4:37 GMT+08:00 Daniel Berlin <dberlin at dberlin.org>:
>
>> Author: dannyb
>> Date: Tue Feb 3 14:37:08 2015
>> New Revision: 228024
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=228024&view=rev
>> Log:
>> Allow PRE to insert no-cost phi nodes
>>
>> Added:
>> llvm/trunk/test/Transforms/GVN/pre-no-cost-phi.ll
>> Modified:
>> llvm/trunk/lib/Transforms/Scalar/GVN.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=228024&r1=228023&r2=228024&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Tue Feb 3 14:37:08 2015
>> @@ -711,6 +711,8 @@ namespace {
>> bool iterateOnFunction(Function &F);
>> bool performPRE(Function &F);
>> bool performScalarPRE(Instruction *I);
>> + bool performScalarPREInsertion(Instruction *Instr, BasicBlock *Pred,
>> + unsigned int ValNo);
>> Value *findLeader(const BasicBlock *BB, uint32_t num);
>> void cleanupGlobalSets();
>> void verifyRemoved(const Instruction *I) const;
>> @@ -2454,6 +2456,43 @@ bool GVN::processBlock(BasicBlock *BB) {
>> return ChangedFunction;
>> }
>>
>> +// Instantiate an expression in a predecessor that lacked it.
>> +bool GVN::performScalarPREInsertion(Instruction *Instr, BasicBlock *Pred,
>> + unsigned int ValNo) {
>> + // Because we are going top-down through the block, all value numbers
>> + // will be available in the predecessor by the time we need them. Any
>> + // that weren't originally present will have been instantiated earlier
>> + // in this loop.
>> + bool success = true;
>> + for (unsigned i = 0, e = Instr->getNumOperands(); i != e; ++i) {
>> + Value *Op = Instr->getOperand(i);
>> + if (isa<Argument>(Op) || isa<Constant>(Op) || isa<GlobalValue>(Op))
>> + continue;
>> +
>> + if (Value *V = findLeader(Pred, VN.lookup(Op))) {
>> + Instr->setOperand(i, V);
>> + } else {
>> + success = false;
>> + break;
>> + }
>> + }
>> +
>> + // Fail out if we encounter an operand that is not available in
>> + // the PRE predecessor. This is typically because of loads which
>> + // are not value numbered precisely.
>> + if (!success)
>> + return false;
>> +
>> + Instr->insertBefore(Pred->getTerminator());
>> + Instr->setName(Instr->getName() + ".pre");
>> + Instr->setDebugLoc(Instr->getDebugLoc());
>> + VN.add(Instr, ValNo);
>> +
>> + // Update the availability map to include the new instruction.
>> + addToLeaderTable(ValNo, Instr, Pred);
>> + return true;
>> +}
>> +
>> bool GVN::performScalarPRE(Instruction *CurInst) {
>> SmallVector<std::pair<Value*, BasicBlock*>, 8> predMap;
>>
>> @@ -2520,60 +2559,43 @@ bool GVN::performScalarPRE(Instruction *
>>
>> // Don't do PRE when it might increase code size, i.e. when
>> // we would need to insert instructions in more than one pred.
>> - if (NumWithout != 1 || NumWith == 0)
>> - return false;
>> -
>> - // Don't do PRE across indirect branch.
>> - if (isa<IndirectBrInst>(PREPred->getTerminator()))
>> + if (NumWithout > 1 || NumWith == 0)
>> return false;
>>
>> - // We can't do PRE safely on a critical edge, so instead we schedule
>> - // the edge to be split and perform the PRE the next time we iterate
>> - // on the function.
>> - unsigned SuccNum = GetSuccessorNumber(PREPred, CurrentBlock);
>> - if (isCriticalEdge(PREPred->getTerminator(), SuccNum)) {
>> - toSplit.push_back(std::make_pair(PREPred->getTerminator(), SuccNum));
>> - return false;
>> - }
>> + // We may have a case where all predecessors have the instruction,
>> + // and we just need to insert a phi node. Otherwise, perform
>> + // insertion.
>> + Instruction *PREInstr = nullptr;
>>
>> - // Instantiate the expression in the predecessor that lacked it.
>> - // Because we are going top-down through the block, all value numbers
>> - // will be available in the predecessor by the time we need them. Any
>> - // that weren't originally present will have been instantiated earlier
>> - // in this loop.
>> - Instruction *PREInstr = CurInst->clone();
>> - bool success = true;
>> - for (unsigned i = 0, e = CurInst->getNumOperands(); i != e; ++i) {
>> - Value *Op = PREInstr->getOperand(i);
>> - if (isa<Argument>(Op) || isa<Constant>(Op) || isa<GlobalValue>(Op))
>> - continue;
>> + if (NumWithout != 0) {
>> + // Don't do PRE across indirect branch.
>> + if (isa<IndirectBrInst>(PREPred->getTerminator()))
>> + return false;
>>
>> - if (Value *V = findLeader(PREPred, VN.lookup(Op))) {
>> - PREInstr->setOperand(i, V);
>> - } else {
>> - success = false;
>> - break;
>> + // We can't do PRE safely on a critical edge, so instead we schedule
>> + // the edge to be split and perform the PRE the next time we iterate
>> + // on the function.
>> + unsigned SuccNum = GetSuccessorNumber(PREPred, CurrentBlock);
>> + if (isCriticalEdge(PREPred->getTerminator(), SuccNum)) {
>> + toSplit.push_back(std::make_pair(PREPred->getTerminator(),
>> SuccNum));
>> + return false;
>> + }
>> + // We need to insert somewhere, so let's give it a shot
>> + PREInstr = CurInst->clone();
>> + if (!performScalarPREInsertion(PREInstr, PREPred, ValNo)) {
>> + // If we failed insertion, make sure we remove the instruction.
>> + DEBUG(verifyRemoved(PREInstr));
>> + delete PREInstr;
>> + return false;
>> }
>> }
>>
>> - // Fail out if we encounter an operand that is not available in
>> - // the PRE predecessor. This is typically because of loads which
>> - // are not value numbered precisely.
>> - if (!success) {
>> - DEBUG(verifyRemoved(PREInstr));
>> - delete PREInstr;
>> - return false;
>> - }
>> + // Either we should have filled in the PRE instruction, or we should
>> + // not have needed insertions.
>> + assert (PREInstr != nullptr || NumWithout == 0);
>>
>> - PREInstr->insertBefore(PREPred->getTerminator());
>> - PREInstr->setName(CurInst->getName() + ".pre");
>> - PREInstr->setDebugLoc(CurInst->getDebugLoc());
>> - VN.add(PREInstr, ValNo);
>> ++NumGVNPRE;
>>
>> - // Update the availability map to include the new instruction.
>> - addToLeaderTable(ValNo, PREInstr, PREPred);
>> -
>> // Create a PHI to make the value available in this block.
>> PHINode *Phi =
>> PHINode::Create(CurInst->getType(), predMap.size(),
>> @@ -2609,6 +2631,8 @@ bool GVN::performScalarPRE(Instruction *
>> MD->removeInstruction(CurInst);
>> DEBUG(verifyRemoved(CurInst));
>> CurInst->eraseFromParent();
>> + ++NumGVNInstr;
>> +
>> return true;
>> }
>>
>>
>> Added: llvm/trunk/test/Transforms/GVN/pre-no-cost-phi.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/pre-no-cost-phi.ll?rev=228024&view=auto
>>
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/GVN/pre-no-cost-phi.ll (added)
>> +++ llvm/trunk/test/Transforms/GVN/pre-no-cost-phi.ll Tue Feb 3 14:37:08
>> 2015
>> @@ -0,0 +1,31 @@
>> +; RUN: opt < %s -gvn -S | FileCheck %s
>> +; This testcase tests insertion of no-cost phis. That is,
>> +; when the value is already available in every predecessor,
>> +; and we just need to insert a phi node to merge the available values.
>> +
>> + at c = global i32 0, align 4
>> + at d = global i32 0, align 4
>> +
>> +
>> +define i32 @mai(i32 %foo, i32 %a, i32 %b) {
>> + %1 = icmp ne i32 %foo, 0
>> + br i1 %1, label %bb1, label %bb2
>> +
>> +bb1:
>> + %2 = add nsw i32 %a, %b
>> + store i32 %2, i32* @c, align 4
>> + br label %mergeblock
>> +
>> +bb2:
>> + %3 = add nsw i32 %a, %b
>> + store i32 %3, i32* @d, align 4
>> + br label %mergeblock
>> +
>> +mergeblock:
>> +; CHECK: pre-phi = phi i32 [ %3, %bb2 ], [ %2, %bb1 ]
>> +; CHECK-NEXT: ret i32 %.pre-phi
>> + %4 = add nsw i32 %a, %b
>> + ret i32 %4
>> +}
>> +
>> +
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150215/3ed9e837/attachment.html>
More information about the llvm-commits
mailing list