[CodeGen][CodeLayout] Fix segfault on access to deleted block in MBP.

[ivafanas]

Problem 1: There is a typo which reassigns BlockWorkList to EHPadWorkList on attempt to remove RemBB from work lists.

Problem 2: Chain->UnscheduledPredecessors == 0 is an incorrect way to check whether RemBB is enqueued or not. The root cause is a postponed deletion of WorkList from already scheduled blocks in selectBestCandidateBlock. Bug happens in the following scenario:

• FunctionChain is being processed with non-zero UnscheduledPredecessors
• Block B' is added to the BlockWorkList
• Block B' is chosen as the best successor (selectBestSuccessor) for some another block and added into Chain
• Block B' is removed by tail duplicator.

RemovalCallback erroneously won't erase B' from BlockWorkList, because UnscheduledPredecessors value of FunctionChain is not zero (and it is allowed to be non-zero).

Proposed solution is to always cleanup worklists on block deletion by tail duplicator.

[ivafanas]

We have caught the bug on the custom out-of-tree backend in a ~200Kb function of ~70 basic blocks with several loops, switch instructions etc.. Any attempt to reduce the test case results in bug disappearing.

I'm afraid, I'm not able to provide the reproducer / regression test for any in-tree backend in a reasonable time.

[ivafanas]

@arsenm , @weiguozhi

Could you please review the PR?

[weiguozhi]

RemovalCallback erroneously won't erase B' from BlockWorkList, because UnscheduledPredecessors value of FunctionChain is not zero (and it is allowed to be non-zero).

From function markBlockSuccessors, a BB can be added to BlockWorkList only when UnscheduledPredecessors reaches 0. So how was B added to BlockWorkList in the first place?

[ivafanas]

From function markBlockSuccessors, a BB can be added to BlockWorkList only when UnscheduledPredecessors reaches 0. So how was B added to BlockWorkList in the first place?

Yes, it is a bit tricky:

• When block B' is added to the work list, it belongs to the chain with zero UnscheduledPredecessors
• When block B' is chosen as the best successor, it is merged into FunctionChain inside BlockChain::merge method. FunctionChain has non-zero UnscheduledPredecessor.

Work lists might contain blocks from chains with non-zero UnscheduledPredecessor. It happens for scheduled blocks which should be removed from work lists soon. However, postponed removal comes to play.

[ivafanas]

Some test in flang failed. Need to be investigated.

[ivafanas]

PR is ready for review.

Seems like flang test is flaky. I've failed to reproduce the problem locally and it disappeared after rerun.

[arsenm]

Missing test

[ivafanas]

Missing test

---

We have caught the bug on the custom out-of-tree backend in a ~200Kb function of ~70 basic blocks with several loops, switch instructions etc.. Any attempt to reduce the test case results in bug disappearing.

I'm afraid, I'm not able to provide the reproducer / regression test for any in-tree backend in a reasonable time.

---

If test is a mandatory requirement for this PR, it is ok for me to reject PR.

[arsenm]

Missing test

We have caught the bug on the custom out-of-tree backend in a ~200Kb function of ~70 basic blocks with several loops, switch instructions etc.. Any attempt to reduce the test case results in bug disappearing.
I'm afraid, I'm not able to provide the reproducer / regression test for any in-tree backend in a reasonable time.

If test is a mandatory requirement for this PR, it is ok for me to reject PR.

Did you try llvm-reduce on it?

[ivafanas]

Did you try llvm-reduce on it?

Yes. No effect.

I've spent ~1 day in different attempts to reduce the test case =)