Fwd: [polly] r255922 - Fix delinearization of fortran arrays

[Roman Gareev via llvm-commits]

2015-12-21 4:25 GMT+05:00 Johannes Doerfert <doerfert at cs.uni-saarland.de>:
> On 12/17, Roman Gareev via llvm-commits wrote:
>> Author: romangareev
>> Date: Thu Dec 17 14:37:17 2015
>> New Revision: 255922
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=255922&view=rev
>> Log:
>> Fix delinearization of fortran arrays
>>
>> The patch fixes Bug 25759 produced by inappropriate handling of unsigned
>> maximum SCEV expressions by SCEVRemoveMax. Without a fix, we get an infinite
>> loop and a segmentation fault, if we try to process, for example,
>> '((-1 + (-1 * %b1)) umax {(-1 + (-1 * %yStart)),+,-1}<%.preheader>)'.
> I get this part, the visitUMaxExpr was plain wrong.
>
>> It also fixes a potential issue related to signed maximum SCEV expressions.
> But I am confused by this part. Why do we check for two elements of the
> SMax now? I know that delinearization is black magic combined with good
> patterns but this seems to make things even more restrictive than they
> were. I do not know why we do not strip __all__ 0's from the SMax if
> that is what this piece of code is for and if it is sound:
>
> assert(Expr.getNumOperands() > 1);
>
> SmallVector<...> SMaxTerms;
> for (Operand : Expr)
>   if (!Operand.isZero())
>     SMaxTerms.push(Operand)
>
> NewExpr = getSMAx(SMaxTerms);
> if (Terms)
>   Terms.push_back(NewExpr)
> return NewExpr

Hi Johannes,

I discussed this with Tobias. Gfortran always produces at most one
zero for us. Hence, there is no use case that would make it worthwhile
to spend more time on this. The pattern we need is exactly one zero.
We can extend it if other use cases show up in practice. Also, we
would assume SCEV to already simplify away such zero's for us.

--
                                    Cheers, Roman Gareev.