[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