[PATCH] D112026: Treat branch on poison as immediate UB (under an off by default flag)

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 24 06:39:45 PDT 2021


fhahn added a comment.

In D112026#3071195 <https://reviews.llvm.org/D112026#3071195>, @reames wrote:

> In D112026#3071193 <https://reviews.llvm.org/D112026#3071193>, @fhahn wrote:
>
>> Perhaps it would be worth to add the flag to one of the tests from D111246 <https://reviews.llvm.org/D111246>.
>
> Thought about it, but only the SCEV tests are really clear as to *why* the impact happens, and those tests are already complicated enough.  Having to version one which happens to change doesn't really seem to add much value as there's a *bunch* of SCEV implementation details you have to dance around, and any one of them could change at any time.  Just as an example, SCEV currently handrolls exactly this logic, but applies it only to expressions which happen to be addrecs.  An unrelated change to say trip count logic could totally change whether an expression folded to an addrec or not.

Agreed, it's no ideal to have this flag for the more complicated tests.

But I think it would still be good to have at least minimal test coverage for the new flag & code. Perhaps you could add new test with a single function that uses the flag?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112026



More information about the llvm-commits mailing list