[PATCH] D70030: [MachineBlockPlacement] Fix UnscheduledPredecessors counter to reflect tail duplication.
Huihui Zhang via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 10 09:59:57 PST 2019
huihuiz marked an inline comment as done.
huihuiz added inline comments.
================
Comment at: llvm/lib/CodeGen/MachineBlockPlacement.cpp:2976
+ continue;
+ if (&BBChain != BlockToChain[Pred] && BBChain.UnscheduledPredecessors != 0)
+ --BBChain.UnscheduledPredecessors;
----------------
Carrot wrote:
> huihuiz wrote:
> > Carrot wrote:
> > > Here if BBChain.UnscheduledPredecessors == 0, then there is something wrong.
> > already guarded under "BBChain.UnscheduledPredecessors != 0"
> You should not hide this kind of compiler internal error. Even do nothing is better than hide it. Of course a better choice is assert it.
>
I don't think so. Assert here may not be good idea.
Refer to line:620, this also make sure not decrementing on an already zero UnscheduledPredecessors counter.
```
// This is a cross-chain edge that is within the loop, so decrement the
// loop predecessor count of the destination chain.
if (SuccChain.UnscheduledPredecessors == 0 ||
--SuccChain.UnscheduledPredecessors > 0)
continue;
```
Please let me know if you can agree with this? I am trying not to waste each other's time on endless arguing.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70030/new/
https://reviews.llvm.org/D70030
More information about the llvm-commits
mailing list