[PATCH] D17553: [SimplifyCFG] Speculatively flatten CFG based on profiling metadata (try 2)
Sanjoy Das via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 26 14:07:54 PST 2016
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?
> 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`.
More information about the llvm-commits