[PATCH] Fix loop rerolling pass failure with non-consant loop lower

[David Peixotto]

Opened Bug 18374 for the 64-bit issue (http://llvm.org/bugs/show_bug.cgi?id=18374). I see the problem at least on x86_64 and AARch64. It looks like a problem where the induction variable is converted to 64-bits, but the loop bound remains 32-bits.

> -----Original Message-----
> From: Hal Finkel [mailto:hfinkel at anl.gov]
> Sent: Friday, January 03, 2014 10:48 AM
> To: David Peixotto
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH] Fix loop rerolling pass failure with non-consant loop
> lower
> 
> ----- Original Message -----
> > From: "David Peixotto" <dpeixott at codeaurora.org>
> > To: "Hal Finkel" <hfinkel at anl.gov>
> > Cc: llvm-commits at cs.uiuc.edu
> > Sent: Friday, January 3, 2014 11:25:32 AM
> > Subject: RE: [PATCH] Fix loop rerolling pass failure with non-consant
> > loop lower
> >
> > Hal, thanks for the review. I made all your suggested changes and
> > committed in r198425.
> 
> Great! Can you open a new bug report about the fact that the loop is not
> rerolled on 64-bit targets (is this all 64-bit targets, or just 64-bit
> variants of a particular target?)
> 
> Thanks again,
> Hal
> 
> >
> > > -----Original Message-----
> > > From: Hal Finkel [mailto:hfinkel at anl.gov]
> > > Sent: Wednesday, January 01, 2014 4:39 PM
> > > To: David Peixotto
> > > Cc: llvm-commits at cs.uiuc.edu
> > > Subject: Re: [PATCH] Fix loop rerolling pass failure with
> > > non-consant loop lower
> > >
> > > David,
> > >
> > > Thanks for working on this! This looks good; a few minor comments:
> > >
> > > +        // Iteration count scev minus 1
> > >
> > > Please capitalize SCEV.
> > >
> > > +        const SCEV *ICSCEVm1 = SE->getMinusSCEV(ICSCEV,
> > > + SE->getConstant(ICSCEV->getType(), 1));
> > >
> > > This line is too long. Also, please name this variable ICMinus1SCEV
> > > for consistency with the other variables.
> > >
> > > +        Value *ICm1; // Iteration count minus 1
> > >
> > > Please name this ICMinus1.
> > >
> > > +
> > > +
> > > +attributes #0 = { nounwind "less-precise-fpmad"="false"
> > > +"no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf"
> > > +"no-infs-fp-math"="false" "no-nans-fp-math"="false"
> > > +"stack-protector-buffer-size"="8" "unsafe-fp-math"="false"
> > > +"use-soft-float"="true" }
> > > +
> > > +!llvm.ident = !{!0}
> > > +
> > > +!0 = metadata !{metadata !"Clang 3.1"}
> > > +!1 = metadata !{metadata !2, metadata !2, i64 0}
> > > +!2 = metadata !{metadata !"int", metadata !3, i64 0}
> > > +!3 = metadata !{metadata !"omnipotent char", metadata !4, i64 0}
> > > +!4 = metadata !{metadata !"Simple C/C++ TBAA"}
> > > +!5 = metadata !{metadata !6, metadata !6, i64 0}
> > > +!6 = metadata !{metadata !"float", metadata !3, i64 0}
> > >
> > > Please remove any unnecessary attributes and metadata.
> > >
> > > With these changes, LGTM.
> > >
> > >  -Hal
> > >
> > > ----- Original Message -----
> > > > From: "David Peixotto" <dpeixott at codeaurora.org>
> > > > To: llvm-commits at cs.uiuc.edu
> > > > Cc: "Hal Finkel" <hfinkel at anl.gov>
> > > > Sent: Friday, December 20, 2013 7:09:41 PM
> > > > Subject: [PATCH] Fix loop rerolling pass failure with non-consant
> > > > loop lower
> > > >
> > > > Proposed fix for PR18290:
> > > > http://llvm.org/bugs/show_bug.cgi?id=18290
> > > >
> > > > The loop rerolling pass was failing with an assertion failure from
> > > > a failed cast on loops like this:
> > > >
> > > >   void foo(int *A, int *B, int m, int n) {
> > > >     for (int i = m; i < n; i+=4) {
> > > >       A[i+0] = B[i+0] * 4;
> > > >       A[i+1] = B[i+1] * 4;
> > > >       A[i+2] = B[i+2] * 4;
> > > >       A[i+3] = B[i+3] * 4;
> > > >     }
> > > >   }
> > > >
> > > > The code was casting the SCEV-expanded code for the new induction
> > > > variable to a phi-node. When the loop had a non-constant lower
> > > > bound, the SCEV expander would end the code expansion with an add
> > > > insted of a phi node and the cast would fail.
> > > >
> > > > It looks like the cast to a phi node was only needed to get the
> > > > induction variable value coming from the backedge to compute the
> > > > end of loop condition. This patch changes the loop reroller to
> > > > compare the induction variable to the number of times the backedge
> > > > is taken instead of the iteration count of the loop. In other
> > > > words, we stop the loop when the current value of the induction
> > > > variable == IterationCount-1. Previously, the comparison was
> > > > comparing the induction variable value from the next iteration ==
> > > > IterationCount.
> > > >
> > > > This problem only seems to occur on 32-bit targets. For some
> > > > reason, the loop is not rerolled on 64-bit targets.
> > > >
> > >
> > > --
> > > Hal Finkel
> > > Assistant Computational Scientist
> > > Leadership Computing Facility
> > > Argonne National Laboratory
> >
> >
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory