[PATCH] D134152: [SimplifyCFG][TranformUtils]Do not simplify away a trivial basic block if both this block and at least one of its predecessors are loop latches.

[Mingming Liu via Phabricator via llvm-commits]

mingmingl added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Utils/Local.h:160
+//
+// FIXME: Currently the conditions diverge when this function is called by
+// 'JumpThreading' and 'SimplifyCFG'. The divergence should really be unified.
----------------
davidxl wrote:
> This fixme seems out of context. Is it needed?
It's verbose for a fix, but related with test coverage

1) When called by 'JumpThreading', loop latches are not attempted by `TryToSimplifyUncondBranchFromEmptyBlock`  ([code and comment](https://github.com/llvm/llvm-project/blob/47c998cc2e3fea8047868c0394b752d9d63c2329/llvm/lib/Transforms/Scalar/JumpThreading.cpp#L460-L463))
2) When called by 'SimplifyCFG', loop latches with more than two predecessors are not attempted by `TryToSimplifyUncondBranchFromEmptyBlock`  ([code](https://github.com/llvm/llvm-project/blob/47c998cc2e3fea8047868c0394b752d9d63c2329/llvm/lib/Transforms/Utils/SimplifyCFG.cpp#L6861) and [patch](https://reviews.llvm.org/D42691))

As a result of 1) and 2), test cases are possible for SimplifyCFG but not straightforward (if makes sense at all) for JumpThreading -> what that means is shown in the relevant test case (`test2`), empty block `BB` has one predecessor, but it shouldn't be optimized away (for inner-loop metadata).

Removed this comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134152/new/

https://reviews.llvm.org/D134152