[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 llvm-commits at lists.llvm.org
Wed Sep 28 00:50:38 PDT 2022


mingmingl added a comment.

In D134152#3820125 <https://reviews.llvm.org/D134152#3820125>, @nikic wrote:

> It's fine to work around this in the meantime (it is a correctness problem when it comes to mustprogress at least) -- but shouldn't this be done by dropping the loop metadata? I believe that's the general rule for metadata: If a transform cannot safely preserve metadata, this is always resolved in favor of dropping the metadata, not preventing the transform.

(Oh we sent out the comment around the same time and I quoted the previous comment :))

> that's the general rule for metadata: If a transform cannot safely preserve metadata, this is always resolved in favor of dropping the metadata, not preventing the transform.

Got it. This would make sense to me.

I once thought hard about how to 'concat' loop metadata correctly when drafting something, but that effort ended up nowhere.

TBH I haven't thought through what it would take (maybe clang and llvm IR first, while keeping IR compatibility for frontends of other languages) to implement header-based loop metadata. However, I concur that

1. in C++ code a loop could easily have many latches, compared with headers
2. Even when inner loop and outer loop have the same IR header [1], the inner-loop and outer-loop have headers (in c++ form) respectively.

[1]

This example from https://llvm.org/docs/LoopTerminology.html#important-notes

  for (int i = 0; i < 128; ++i)
    for (int j = 0; j < 128; ++j)
      body(i,j);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134152



More information about the llvm-commits mailing list