[polly] r255922 - Fix delinearization of fortran arrays

Roman Gareev via llvm-commits llvm-commits at lists.llvm.org
Sun Dec 27 05:17:22 PST 2015

2015-12-21 16:33 GMT+05:00 Tobias Grosser <tobias at grosser.es>:
> On 12/21/2015 12:12 PM, Johannes Doerfert wrote:
>> On 12/21, Roman Gareev wrote:
>>> 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.
>> Even when we assume only a single Zero (which is probably a good
>> assumption due to ScalarEvolution), I wouldn't have expected it to be at
>> the first position all the time. Is that somehow guaranteed or could
>> that change to small changes in the IR (or ScalarEvolution)?
> That's indeed an interesting remark. gfortran probably does not generate
> such IR, but it might be worth checking if we see this small change if the
> operands in the input IR are exchange. If this is the case, we could
> possibly extend the pattern here. Roman, I think it would be a good idea to
> look at this limited case.

If I’m not mistaken, it isn’t possible to create such a test case
because of canonicalization, which is performed on operands of

If we modify its code, for example, to the following one

const SCEV *ScalarEvolution::getSMaxExpr(const SCEV *LHS,
                                         const SCEV *RHS) {
  SmallVector<const SCEV *, 2> Ops1;
  const SCEV *res1 = getSMaxExpr(Ops1);
  static_cast<const SCEVSMaxExpr *>(res1)->getOperand(0)->dump();
  static_cast<const SCEVSMaxExpr *>(res1)->getOperand(1)->dump();
  SmallVector<const SCEV *, 2> Ops2;
  const SCEV *res2 = getSMaxExpr(Ops2);
  static_cast<const SCEVSMaxExpr *>(res2)->getOperand(0)->dump();
  static_cast<const SCEVSMaxExpr *>(res2)->getOperand(1)->dump();
  return res2;

and pass multidim_fortran_2d.ll to opt with the following options

opt -load LLVMPolly.so -polly-detect

we get the following output, which probably means that, in case one of
the operands is constant and the other isn’t a constant, an order of
operands doesn’t influence on a result of

(0 smax %tmp)
(0 smax %tmp)

                                    Cheers, Roman Gareev.

More information about the llvm-commits mailing list