[llvm] r259675 - Revert r259662, which caused regressions on polly tests.

[Hal Finkel via llvm-commits]

----- Original Message -----
> From: "Wei Mi" <wmi at google.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Tobias Grosser" <tobias at grosser.es>, "llvm-commits" <llvm-commits at lists.llvm.org>
> Sent: Wednesday, February 3, 2016 5:31:13 PM
> Subject: Re: [llvm] r259675 - Revert r259662, which caused regressions on polly tests.
> 
> > 1) test/Isl/CodeGen/non-affine-phi-node-expansion.ll
> 
> > In this test case LLVM after your patch actually produces more
> > complicated IR then before. Specifically, it does not inline the
> > 'true'
> > value any more. This is clearly a minor regression which will be
> > cleaned up by later passes, but it might be of interest to you

Sounds good. As far as I'm concerned, you can re-commit with the fixes (please make sure the explanations from these e-mails make it into the commit message along with the original text).

 -Hal

> 
> For this case, the SCEV is of scConstant type which is cheaper than
> normal value cached. Although in the cached value set in
> ExprValueMap,
> there is a Constant type value (which is const true), but it is not
> easy to find it out -- the cached Value set is not sorted according
> to
> the potential cost. Existing reuse logic in SCEVExpander::expand
> simply chooses the first legal element from the cached value set.
> 
> The workaround is: when the SCEV is of scConstant type, don't try the
> reuse logic. simply expand it.
> 
> --- lib/Analysis/ScalarEvolutionExpander.cpp 2016-02-03
> 15:29:14.715431108 -0800
> +++ lib/Analysis/ScalarEvolutionExpander.2.cpp 2016-02-03
> 15:28:57.075244422 -0800
> @@ -1649,7 +1649,8 @@
>    // type SCEV, it will be expanded literally, to prevent LSR's
> transformed SCEV
>    // from being reverted.
>    if (!(LSRMode && SE.containsAddRecurrence(S))) {
> -    if (Set) {
> +    // If S is scConstant, it may be worse to reuse an existing
> Value.
> +    if (S->getSCEVType() == scConstant && Set) {
>        // Choose a Value from the set which dominates the insertPt.
>        for (auto const &Ent : *Set) {
>          if (Ent && isa<Instruction>(Ent) && S->getType() ==
>          Ent->getType() &&
> 
> Thanks,
> Wei.
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory