[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.

Wed Sep 28 09:08:30 PDT 2022

davidxl added a comment.

In D134152#3821184 <https://reviews.llvm.org/D134152#3821184>, @fhahn wrote:

> In D134152#3821142 <https://reviews.llvm.org/D134152#3821142>, @davidxl wrote:
>
>> 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.
>>
>> Dropping the metadata in this case changes the program semantics (unlike dropping profile data), thus the transformation in this case is unsafe so it should be disabled as this patch does.
>
> I think I am missing what the issue with dropping the loop metadata would be, could you elaborate? If we drop the metadata we should only lose information that may have enabled additional transformations, but shouldn't change the result of the program.

True - I should have been clearer. The behavior of the program does not change. What changes is the 'expected' shape of the affected loop.  This is similar in spirit for 'always_inline' /'noinline' etc.   We should have strong reason to 'not'  preserve user directives.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134152