[PATCH] D100721: [SCEVExpander] Try to create ASHR instr for expanded SCEV expr.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D100721#2697174 <https://reviews.llvm.org/D100721#2697174>, @nikic wrote:

>> Should we just add a flag to SCEVUDivExpr? Or mark UDivs as exact in a
>> separate table?
>
> Due to SCEV unification, it is not possible to transfer poison-generating flags from IR to SCEV expressions, without proving that a poison result would always cause UB in any scope for which the SCEV expression is valid. For non-addrecs this is only possible in very narrow situations (instruction in loop header, based on addrec in that loop, poison causes UB), so it's not really useful in this situation. Certainly wouldn't cover your examples.

In the examples, wouldn't overflow of the `ASHR` be  UB in all scopes, as each path from the `ashr ` to the exit needs to go through a branch which compares the result of the `ashr`? (ignoring fact that branch on undef/poison is not yet considered UB by ValueTracking)

> I would recommend reverting rGec54867df5e7f20e12146e628af34f0384308bcb <https://reviews.llvm.org/rGec54867df5e7f20e12146e628af34f0384308bcb>.

That's certainly a good way forward for now IMO, as I definitely agree that the improved modeling would be a lot of work and even then it is not clear how feasible this would be for real-world cases.

@ebedev.ri Given that ec54867df5e7 <https://reviews.llvm.org/rGec54867df5e7f20e12146e628af34f0384308bcb>  can have a noticeable negative impact on performance, WDYT about a revert?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100721