[PATCH] D17553: [SimplifyCFG] Speculatively flatten CFG based on profiling metadata (try 2)
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 23 20:23:51 PST 2016
reames added a comment.
Responding only to the material design comment for the moment. Will address nits once the design question is resolved.
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?
> 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.
More information about the llvm-commits