[llvm] r369191 - [CodeGen] Do the Simple Early Return in block-placement pass to optimize the blocks
Fedor Sergeev via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 26 15:01:52 PDT 2019
Hmm... we are seeing problems with X86 backend (and our Java JIT
frontend) that we managed
to bisect down to this change.
I might be able to provide a clean reproducer tomorrow.
Meanwhile, Mikael, was there a bug filed?
I dont see any followup commits related to this....
regards,
Fedor.
On 8/23/19 4:36 PM, Mikael Holmén via llvm-commits wrote:
> Hi,
>
> We're seeing a problem with this commit for our out-of-tree backend.
>
> Before 'Early Branch Probability Basic Block Placement':
>
> bb.2.cond.end.thread:
> ; predecessors: %bb.1
> successors: %bb.5(0x80000000); %bb.5(100.00%)
>
> brr_uncond %bb.5
>
> [...]
>
> bb.5.land.lhs.true28:
> ; predecessors: %bb.2, %bb.3
> successors: %bb.6(0x30000000), %bb.7(0x50000000); %bb.6(37.50%),
> %bb.7(62.50%)
>
> %8:an32_rn_pairs = ...
> cmp ...
> brr_cond %bb.7
> brr_uncond %bb.6
>
> [...]
>
> bb.7.cond.end38:
> ; predecessors: %bb.5, %bb.4, %bb.6
> successors: %bb.8(0x00000001), %bb.9(0x7fffffff); %bb.8(0.00%),
> %bb.9(100.00%)
>
> %2:an32_0_7 = PHI %8:an32_rn_pairs, %bb.5, %11:an32_rn_pairs, %bb.4,
> %0:an32_rn_pairs, %bb.6
> cmps ...
> brr_cond %bb.9
> brr_uncond %bb.8
>
>
> and after:
>
> bb.2.cond.end.thread:
> ; predecessors: %bb.1, %bb.3
> successors: %bb.7(0x50000000), %bb.6(0x30000000); %bb.7(62.50%),
> %bb.6(37.50%)
>
> %8:an32_rn_pairs =...
> cmp ...
> brr_cond %bb.7
> brr_uncond %bb.6
>
> [...]
>
> bb.7.cond.end38:
> ; predecessors: %bb.4, %bb.6, %bb.2
> successors: %bb.8(0x00000001), %bb.9(0x7fffffff); %bb.8(0.00%),
> %bb.9(100.00%)
>
> %2:an32_0_7 = PHI %8:an32_rn_pairs, %bb.-1, %11:an32_rn_pairs, %bb.4,
> %0:an32_rn_pairs, %bb.6
> cmps ...
> brr_cond %bb.8
>
>
> So it looks like it's failed to update the PHI operand for %bb.5 in %2
> properly leaving it now with %bb.-1?
>
> Any idea?
>
> /Mikael
>
> On 2019-08-17 16:37, Kang Zhang via llvm-commits wrote:
>> Author: zhangkang
>> Date: Sat Aug 17 07:37:05 2019
>> New Revision: 369191
>>
>> URL: https://protect2.fireeye.com/url?k=6f40a04c-3393b024-6f40e0d7-8691b328a8b8-90715b879d6c2a6b&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%3Frev%3D369191%26view%3Drev
>> Log:
>> [CodeGen] Do the Simple Early Return in block-placement pass to optimize the blocks
>>
>> Summary:
>>
>> Fix a bug of preducessors.
>>
>> In `block-placement` pass, it will create some patterns for unconditional we can do the simple early retrun.
>> But the `early-ret` pass is before `block-placement`, we don't want to run it again.
>> This patch is to do the simple early return to optimize the blocks at the last of `block-placement`.
>>
>> Reviewed By: efriedma
>>
>> Differential Revision: https://protect2.fireeye.com/url?k=af91b373-f342a31b-af91f3e8-8691b328a8b8-a356f6b65b5e7513&q=1&u=https%3A%2F%2Freviews.llvm.org%2FD63972
>>
>> Modified:
>> llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp
>> llvm/trunk/test/CodeGen/PowerPC/block-placement.mir
>>
>> Modified: llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp
>> URL: https://protect2.fireeye.com/url?k=44a01ed0-18730eb8-44a05e4b-8691b328a8b8-590610f76548b1fa&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fllvm%2Ftrunk%2Flib%2FCodeGen%2FMachineBlockPlacement.cpp%3Frev%3D369191%26r1%3D369190%26r2%3D369191%26view%3Ddiff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp Sat Aug 17 07:37:05 2019
>> @@ -2711,6 +2711,7 @@ void MachineBlockPlacement::optimizeBran
>> // cannot because all branches may not be analyzable.
>> // E.g., the target may be able to remove an unconditional branch to
>> // a fallthrough when it occurs after predicated terminators.
>> + SmallVector<MachineBasicBlock*, 4> EmptyBB;
>> for (MachineBasicBlock *ChainBB : FunctionChain) {
>> Cond.clear();
>> MachineBasicBlock *TBB = nullptr, *FBB = nullptr; // For AnalyzeBranch.
>> @@ -2730,9 +2731,45 @@ void MachineBlockPlacement::optimizeBran
>> TII->removeBranch(*ChainBB);
>> TII->insertBranch(*ChainBB, FBB, TBB, Cond, dl);
>> ChainBB->updateTerminator();
>> + } else if (Cond.empty() && TBB && ChainBB != TBB && !TBB->empty() &&
>> + !TBB->canFallThrough()) {
>> + // When ChainBB is unconditional branch to the TBB, and TBB has no
>> + // fallthrough predecessor and fallthrough successor, try to merge
>> + // ChainBB and TBB. This is legal under the one of following conditions:
>> + // 1. ChainBB is empty except for an unconditional branch.
>> + // 2. TBB has only one predecessor.
>> + MachineFunction::iterator I(TBB);
>> + if (((TBB == &*F->begin()) || !std::prev(I)->canFallThrough()) &&
>> + (TailDup.isSimpleBB(ChainBB) || (TBB->pred_size() == 1))) {
>> + TII->removeBranch(*ChainBB);
>> + ChainBB->removeSuccessor(TBB);
>> +
>> + // Update the CFG.
>> + while (!TBB->pred_empty()) {
>> + MachineBasicBlock *Pred = *(TBB->pred_end() - 1);
>> + Pred->ReplaceUsesOfBlockWith(TBB, ChainBB);
>> + }
>> +
>> + while (!TBB->succ_empty()) {
>> + MachineBasicBlock *Succ = *(TBB->succ_end() - 1);
>> + ChainBB->addSuccessor(Succ, MBPI->getEdgeProbability(TBB, Succ));
>> + TBB->removeSuccessor(Succ);
>> + }
>> +
>> + // Move all the instructions of TBB to ChainBB.
>> + ChainBB->splice(ChainBB->end(), TBB, TBB->begin(), TBB->end());
>> + EmptyBB.push_back(TBB);
>> + }
>> }
>> }
>> }
>> +
>> + for (auto BB: EmptyBB) {
>> + MLI->removeBlock(BB);
>> + FunctionChain.remove(BB);
>> + BlockToChain.erase(BB);
>> + F->erase(BB);
>> + }
>> }
>>
>> void MachineBlockPlacement::alignBlocks() {
>> @@ -3052,6 +3089,9 @@ bool MachineBlockPlacement::runOnMachine
>> }
>> }
>>
>> + // optimizeBranches() may change the blocks, but we haven't updated the
>> + // post-dominator tree. Because the post-dominator tree won't be used after
>> + // this function and this pass don't preserve the post-dominator tree.
>> optimizeBranches();
>> alignBlocks();
>>
>>
>> Modified: llvm/trunk/test/CodeGen/PowerPC/block-placement.mir
>> URL: https://protect2.fireeye.com/url?k=c2447a57-9e976a3f-c2443acc-8691b328a8b8-7cd3fcb165ec6683&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fllvm%2Ftrunk%2Ftest%2FCodeGen%2FPowerPC%2Fblock-placement.mir%3Frev%3D369191%26r1%3D369190%26r2%3D369191%26view%3Ddiff
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/PowerPC/block-placement.mir (original)
>> +++ llvm/trunk/test/CodeGen/PowerPC/block-placement.mir Sat Aug 17 07:37:05 2019
>> @@ -209,14 +209,10 @@ body: |
>> BLR8 implicit $lr8, implicit $rm, implicit killed $x3
>>
>> ; CHECK: bb.5.if.else.i:
>> - ; CHECK: successors: %bb.11(0x80000000)
>> - ; CHECK: B %bb.11
>> + ; CHECK-NEXT: renamable $x3 = LI8 1
>> + ; CHECK-NEXT: BLR8 implicit $lr8, implicit $rm, implicit killed $x3
>>
>> ; CHECK: bb.8.while.body.i (align 4):
>> - ; CHECK: successors: %bb.11(0x04000000), %bb.9(0x7c000000)
>> - ; CHECK: BCC 76, killed renamable $cr0, %bb.11
>> -
>> - ; CHECK: bb.11:
>> - ; CHECK: renamable $x3 = LI8 1
>> - ; CHECK-NEXT: BLR8 implicit $lr8, implicit $rm, implicit killed $x3
>> + ; CHECK: successors: %bb.5(0x04000000), %bb.9(0x7c000000)
>> + ; CHECK: BCC 76, killed renamable $cr0, %bb.5
>> ...
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://protect2.fireeye.com/url?k=b7a41100-eb770168-b7a4519b-8691b328a8b8-00280ce3331de496&q=1&u=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fllvm-commits
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list