[PATCH] D106852: [SCEV] Fix getAddExpr for adding loop invariants into start of some AddRec

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

Spent some time trying to get my head around this review, and how we generally handle flags on SCEV objects used in multiple contexts.  https://github.com/preames/public-notes/blob/master/llvm-loop-opt-ideas.rst#scev-wrap-flags

My conclusion so far is that this patch is not a complete fix.  Or at least, I found an analogous bit of code which includes several preconditions this code does not.

I'll also note that I'm not yet at a point of really having a feeling for what path forward we should take.  The way flags are handled currently appears to be largely the inverse of how flags in IR CSE are handled.  I'm still wrapping my head around the implications of a localized fix (how much opt quality do we loose?) and deeper changes to the representation of flags (how scary is it).


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

https://reviews.llvm.org/D106852