[PATCH] D73181: [SCEV] Use backedge SCEV of PHI only if its input is loop invariant

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

This patch bugged me when I first saw it go by; finally got back to looking at it today.

After reading the code, I'm fairly sure this is papering over an issue.  The description given in the review and the patch discussion seem to indicate the believed problem was around using a loop variant value from the previous iteration of the loop as the value for a phi's exit, but the previous code appears to have handled that.  IsAvailableOnEntry should be returning true only for cases where the value is either currently invariant, or can be made invariant through SCEVExpander.

What I suspect is actually going on this test case is there's some mismatch between IsAvailableOnEntry and the actual expansion logic.  As a wild guess, we've seen issues around nsw/nuw flags in the past which have never been fully tracked down, it's possible this is another of those.

I spent some time today seeing if anything obvious fell out, but didn't make much progress on this investigation.  The test case is involved enough that the reasoning is quite complicated.

To be clear, I am not asking for a revert or any other specific action to be taken.  I'm just recording my concern for the record.  If someone later comes along tracking down a performance regression, I want the context to be on the review thread.  There's a decent chance that later person might be me.  :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73181