[polly] r286430 - SCEVValidator: add new parameters resulting from constant extraction

[Friedman, Eli via llvm-commits]

On 11/9/2016 10:45 PM, Tobias Grosser via llvm-commits wrote:
> Author: grosser
> Date: Thu Nov 10 00:45:28 2016
> New Revision: 286430
>
> URL: http://llvm.org/viewvc/llvm-project?rev=286430&view=rev
> Log:
> SCEVValidator: add new parameters resulting from constant extraction
>
> When extracting constant expressions out of SCEVs, new parameters may be
> introduced, which have not been registered before. This change scans
> SCEV expressions after constant extraction again to make sure newly
> introduced parameters are registered.
>
> We may for example extract the constant '8' from the expression '((8 * ((%a *
> %b) + %c)) + (-8 * %a))' and obtain the expression '(((-1 + %b) * %a) + %c)'.
> The new expression has a new parameter '(-1 + %b) * %a)', which was not
> registered before, but must be registered to not crash.
>
> This closes http://llvm.org/PR30953
>
> Reported-by: Eli Friedman <efriedma at codeaurora.org>
>
> Added:
>      polly/trunk/test/ScopInfo/extract_constant_factor_introduces_new_parameter.ll
> Modified:
>      polly/trunk/lib/Support/SCEVAffinator.cpp
>
> Modified: polly/trunk/lib/Support/SCEVAffinator.cpp
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Support/SCEVAffinator.cpp?rev=286430&r1=286429&r2=286430&view=diff
> ==============================================================================
> --- polly/trunk/lib/Support/SCEVAffinator.cpp (original)
> +++ polly/trunk/lib/Support/SCEVAffinator.cpp Thu Nov 10 00:45:28 2016
> @@ -229,6 +229,9 @@ __isl_give PWACtx SCEVAffinator::visit(c
>     auto *Factor = ConstantAndLeftOverPair.first;
>     Expr = ConstantAndLeftOverPair.second;
>   
> +  auto *Scope = getScope();
> +  S->addParams(getParamsInAffineExpr(&S->getRegion(), Scope, Expr, SE));
> +

This isn't wrong, exactly... but I think it would be better to make the 
SCEVValidator visitor call extractConstantFactor.  That way, both 
visitors see the same SCEV expressions, and we don't revisit expressions 
multiple times in SCEVVValidator.

If we're going to go with this patch, we should get delete of the call 
to addParams in SCEVAffinator::getPwAff; it's redundant with the call in 
SCEVAffinator::visit.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project