release_35 patches for unroll pragma

[Hal Finkel]

----- Original Message -----
> From: "Eric Christopher" <echristo at gmail.com>
> To: "Aaron Ballman" <aaron.ballman at gmail.com>
> Cc: "Mark Heffernan" <meheff at google.com>, "Hal Finkel" <hfinkel at anl.gov>, "cfe-commits" <cfe-commits at cs.uiuc.edu>,
> llvm-commits at cs.uiuc.edu
> Sent: Monday, August 4, 2014 12:50:21 PM
> Subject: Re: release_35 patches for unroll pragma
> 
> On Mon, Aug 4, 2014 at 10:45 AM, Aaron Ballman
> <aaron.ballman at gmail.com> wrote:
> > On Mon, Aug 4, 2014 at 1:42 PM, Mark Heffernan <meheff at google.com>
> > wrote:
> >> On Mon, Aug 4, 2014 at 10:35 AM, Aaron Ballman
> >> <aaron.ballman at gmail.com>
> >> wrote:
> >>>
> >>> On Mon, Aug 4, 2014 at 1:33 PM, Mark Heffernan
> >>> <meheff at google.com> wrote:
> >>> > On Mon, Aug 4, 2014 at 10:24 AM, Aaron Ballman
> >>> > <aaron.ballman at gmail.com>
> >>> > wrote:
> >>> >>
> >>> >> On Mon, Aug 4, 2014 at 1:22 PM, Eric Christopher
> >>> >> <echristo at gmail.com>
> >>> >> wrote:
> >>> >> > Any reason that we need them in 3.5? Correctness?
> >>> >>
> >>> >> My only concern is that the feature is partially in 3.5, but a
> >>> >> user-facing part of that feature was changed once the freeze
> >>> >> happened.
> >>> >> Eg)  #pragma clang loop unroll(enable) became  #pragma clang
> >>> >> loop
> >>> >> unroll(full)
> >>> >
> >>> >
> >>> > That's my primary concern as well.  Having one release with one
> >>> > particular
> >>> > syntax, then switch it to something else for the next release
> >>> > is not
> >>> > great.
> >>> > All-in-all I'd probably prefer not supporting the unroll pragma
> >>> > at all
> >>> > in
> >>> > 3.5 than have a (slightly) buggy one whose syntax will change.
> >>> >  However,
> >>> > rolling back support completely would be a bigger change than
> >>> > these
> >>> > patches.
> >>>
> >>> An alternate option would be to update the documentation to
> >>> remove
> >>> mention of the feature. That's a much smaller change. ;-)
> >>>
> >>> ~Aaron
> >>
> >>
> >> If having a stealth feature like that is reasonable, I'm happy to
> >> remove
> >> mention of it from the docs.  More specifically any mention of the
> >> following
> >> would be removed: '#pragma unroll', '#pragma clang loop unroll',
> >> '#pragma
> >> clang loop unroll_count', and 'llvm.loop.unroll.*' metadata.
> >
> > My gut feeling is: given that the feature isn't complete in 3.5,
> > it's
> > not really a stealth feature so much as an incomplete
> > work-in-progress
> > that people should not rely on since we're not documenting it. If
> > we
> > have it documented, then it's arguable that we should be supporting
> > it
> > as a feature and not changing the syntax.
> >
> > Others may have different opinions.
> >
> 
> *shrug* I'm fine with the work in progress aspect of it, probably
> shouldn't be documented. Hal has been fairly heavily involved so I'll
> want to wait for him to weigh in though.

Unfortunately, I was on vacation that week, and I'm still playing catch-up on my e-mail...

At this point I think it is too late to pull in these kinds of changes, but, if we didn't previously, we should add auto-upgrade support for the renamed metadata.

 -Hal 

> 
> -eric
> 

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