[Patch][LoopInfo] Use CGLoopInfo to generate loop hints

[Hal Finkel]

----- Original Message -----
> From: "Tyler Nowicki" <tnowicki at apple.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "llvm cfe" <cfe-commits at cs.uiuc.edu>, "Gerolf Hoflehner" <ghoflehner at apple.com>
> Sent: Thursday, July 2, 2015 12:27:00 PM
> Subject: Re: [Patch][LoopInfo] Use CGLoopInfo to generate loop hints
> 
> Hi,
> 
> Could I get a review of these patches?

Yes, I'll look at them soon (sorry, I'm about a week behind on code reviews right now).

 -Hal

> 
> Thanks,
> 
> Tyler
> 
> > On Jun 24, 2015, at 4:25 PM, Tyler Nowicki <tnowicki at apple.com>
> > wrote:
> > 
> > Hi Hal,
> > 
> > Here are the split patches.
> > 
> > Thanks,
> > 
> > Tyler
> > 
> > <0001-Rename-Vectorizer-to-Vectorize-and-VectorizeUnroll-t.patch><0002-Moved-functionality-from-EmitCondBr-to-CGLoopInfo.patch>
> > 
> >> On Jun 22, 2015, at 5:40 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> >> 
> >> Hi Tyler,
> >> 
> >> -                            Type::getInt32Ty(Ctx),
> >> Attrs.VectorizerWidth))};
> >> +                            Type::getInt32Ty(Ctx),
> >> Attrs.VectorizeWidth))};
> >> 
> >> Could you please factor out the renamings from the functional
> >> change?
> >>>> 
> >>>> Hello,
> >>>> 
> >>>> I noticed a problem with my implementation of #pragma clang loop
> >>>> vectorize(assume_safety). When it was specified on a loop other
> >>>> loop hints were lost. The problem is that CGLoopInfo attaches
> >>>> metadata a little bit differently than EmitCondBrHints in
> >>>> CGStmt.
> >>>> For do-loops CGLoopInfo attaches metadata to the br in the body
> >>>> block and for while and for loops, the inc block.
> >>>> EmitCondBrHints
> >>>> on the other hand always attaches data to the br in the cond
> >>>> block. When specifying assume_safety CGLoopInfo emits an empty
> >>>> llvm.loop metadata shadowing the metadata in the cond block.
> >>>> Loop
> >>>> transformations like rotate and unswitch would then eliminate
> >>>> the
> >>>> cond block.
> >>>> 
> >>>> This patch unifies both approaches for adding metadata and
> >>>> modifies
> >>>> the existing safety tests to include non-assume_safety loop
> >>>> hints.
> 
> 

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