[PATCH] D17553: [SimplifyCFG] Speculatively flatten CFG based on profiling metadata (try 2)

[Sanjoy Das via llvm-commits]

sanjoy added inline comments.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2759
@@ +2758,3 @@
+    if (auto *OBO = dyn_cast<OverflowingBinaryOperator>(I))
+      if (OBO->hasNoUnsignedWrap() || OBO->hasNoSignedWrap())
+        // TODO: can we prove the flags are valid before PBI?
----------------
reames wrote:
> sanjoy wrote:
> > I'd say this is a little risky.  I think you're okay now since `isImpliedCondition` only looks at overflow flags, but if it is changed to consider, e.g., the `exact` flag on `udiv` or `inbounds` on `getelementptr` at some time in the future, this might subtly break.
> > 
> > I'd rather split out a helper routine, `GetCombinedCond` that takes two predicates, `P` and `Q` and a location, `Loc`, and returns a predicate that is equivalent to `P && Q` at `Loc`, if it can be cheaply computed.  Predicates like `5 u< L` and `6 u< L` can be combined into `6 u< L`; and predicates like `I u< L` and `(I +nuw 1) u< L` can be combined into `(I +nuw 1) u< L` if you can prove that the `nuw` holds at `Loc`, etc.
> To be clear, I'm not proposing that we *just* check nsw/nuw.  As you point out, that would be utterly unsound.  Assuming I extend the logic to include exact and inbounds (which I had completely forgotten about), what do you think of the approach?  (If I've forgotten other cases, please suggest them.)
> 
> I'm a bit leery of introducing the separate predicate.  I think that unless we're really careful here, we could end up duplicating a lot of code.  Now, having said that, I'll give it a bit more thought tomorrow and see what I think after writing some prototype code.  
I don't think we'll duplicate much code here, `GetCombinedCond` will just be a helper to simplify `X && Y` into something cheaper.  `X` implies `Y` (without exploiting invalid no-wrap) will then just be one case where it `X && Y` can be simplified to `X`.  It should also be a more natural extension point -- for instance, if we want to combine `0 < I < 20` and `10 < I < 50`, neither implies the other but their logical-and can be expressed as `10 < I < 20`.


http://reviews.llvm.org/D17553