[PATCH] Fix loop rerolling pass failure with non-consant loop lower
Hal Finkel
hfinkel at anl.gov
Fri Jan 3 10:47:59 PST 2014
----- 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
More information about the llvm-commits
mailing list