[PATCH] D17201: [SCEV] Introduce a guarded backedge taken count and use it in LAA and LV

[Sanjoy Das via llvm-commits]

sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

Hi Silviu,

This lgtm now!  Thanks for bearing with the long review process.  I have a minor "messaging" comment inline, but otherwise things look great!


================
Comment at: include/llvm/Analysis/ScalarEvolution.h:281-282
@@ +280,4 @@
+  /// not imply that X is equal to the backedge taken count. This means that
+  /// if we have a nusw Wrap predicate for i32 {0,+,1} with a predicated
+  /// backedge taken count of X, we only guarantee that the {0,+,1} has nusw
+  /// in the first X iterations. {0,+,1} may still wrap in the loop if
----------------
What I have in mind is slightly different (same content, said
differently):

 1. If a SCEVPredicate says `AR` is `nssw` then `AR` *is* `nssw` for
    the entire iteration space of the loop.  No exceptions.

 2. The predicated BE taken computation logic now needs to be aware of
    a possible circular logic issue -- if it adds a predicate forcing
    an `AR` to be `nssw` then `AR` is `nssw` only for the trip count
    it itself computes.  This means it cannot use the no-overflow
    property in certain ways the non-predicated BE taken computation
    can.

If (1) is ever incorrect (i.e. the predicate fails to hold) then we've
already "lost" (i.e. miscompiled) because
`getPredicatedBackedgeTakenCount` gave us the wrong answer.

In other words, I think you're downplaying the fact that the "burden"
is really on `getPredicatedBackedgeTakenCount`, and not on the general
notion of predicate itself. For instance, there is no complication at
all if we don't also use predicated backedge taken counts (i.e. we use
normal backedge taken counts) with `SCEVPredicate`.


http://reviews.llvm.org/D17201