[PATCH] D31238: [ScalarEvolution] Re-enable Predicate implication from operations
Max Kazantsev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 23 21:45:05 PDT 2017
mkazantsev added inline comments.
================
Comment at: lib/Analysis/ScalarEvolution.cpp:8549
+ unsigned Depth) {
+ // We want to avoid hurting the compile time with analysis of too big trees.
+ if (Depth > MaxSCEVOperationsImplicationDepth)
----------------
sanjoy wrote:
> How about asserting here that
>
> - The types of `LHS` and `RHS` match
> - The types of `FoundLHS` and `FoundRHS` match
>
> ?
This assertion actually fails on test Transforms/IndVarSimplify/2011-11-01-lftrptr.ll even if placed to the beginning of isImpliedCondOperandsHelper method (and ViaOperations stuff is disabled). I am not certain for 100% is it a bug or not, but potentially it is. Even if so, its reason lies outside this patch. I will try to investigate it.
Currently we do not require the match of these types for our code here (yet I assumed that they do match). All required type conversions within isImpliedViaOperations are done explicitly. So let us not do it here. Instead, I will try to understand whether the failure on this assertion on those test is a bug, and if it is, I will add them with the fix patch.
https://reviews.llvm.org/D31238
More information about the llvm-commits
mailing list