[PATCH] D100663: [LV] Add undef incoming value to loop-exit phis for the middle-block.

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

In D100663#2697223 <https://reviews.llvm.org/D100663#2697223>, @fhahn wrote:

> Thanks for taking a look and apologies for not doing a good job at conveying my thinking. My rational was that at the point we add the undef incoming value, it is yet undetermined which edge will be taken, so SCEV will constrain the undef value, but only to the incoming value of the scalar loop. In the final IR, both incoming values should result in the same value at runtime. Does that make sense or am I still missing something?

I see where you're going here (assuming the fix you mention after this in your long comment), and this could maybe be made to work, but I'm still nervous.

I see two classes of potential issues.

1. SCEV could cache a result which was true before we rewrote the branch conditions, but not true afterwards.  (e.g. treated the phi as-if it were single entry)  We could invalidate to resolve this, but that means our placement of invalidates after *every* condition modification has to be perfect or we get nasty bugs.

2. The IR might not actually allow us to assume the scalar path and the vector path always produce the same values.  I don't know the existing code structure well enough, do we ever mutate the scalar loop based on the existence of the prechecks or vector bodies?  In particular, maybe when tail folding?   If we do, there is no safe value to use.

I think what we need here is an actual "unknown until later" value.  I'd be tempted to add an intrinsic for that, except I'm not quite sure if we need such a thing in a constant expression anywhere.   This was one of the things I'd been hoping to chat through with you offline when you had a moment.  :)

I'll also note that at the end of the day, I'm willing to defer to you if you think the approach here is good enough after reading the above.  I don't want to block "better" on "perfect" here.  If you do want to move forward with this approach, let me know and I'll do a review of the code (as opposed to the idea).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100663