> -----Original Message-----
> From: Jonathan Wakely <jwak...@redhat.com>
> Sent: Tuesday, May 13, 2025 11:34 AM
> To: Tamar Christina <tamar.christ...@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; rguent...@suse.de
> Subject: Re: [PATCH 1/4]middle-end: document pragma unroll n
> <requested|preferred> [PR116140]
> 
> On Tue, 13 May 2025 at 11:26, Tamar Christina <tamar.christ...@arm.com>
> wrote:
> >
> > > -----Original Message-----
> > > From: Jonathan Wakely <jwak...@redhat.com>
> > > Sent: Tuesday, May 13, 2025 11:01 AM
> > > To: Tamar Christina <tamar.christ...@arm.com>
> > > Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; rguent...@suse.de
> > > Subject: Re: [PATCH 1/4]middle-end: document pragma unroll n
> > > <requested|preferred> [PR116140]
> > >
> > > On 13/05/25 10:39 +0100, Tamar Christina wrote:
> > > >Hi All,
> > > >
> > > >In PR116140 it was brought up that adding pragma GCC unroll in std::find
> makes
> > > >it so that you can't use a larger unroll factor if you wanted to.  This 
> > > >is
> > > >because the value can't be overriden by the other unrolling flags such as
> > > >-funroll-loops.
> > > >
> > > >To know whether this should be possible to do or not this proposes an
> extension
> > > >to the pragma GCC unroll with an argument to indicate if we can override 
> > > >the
> > > >value or not.
> > > >
> > > >* requested: means that we cannot override the value.   If we can unroll 
> > > >the
> > > >          unroll, we must unroll by the amount specified.
> > > >* preferred: means that we can override the value.  Effectively we 
> > > >ignore the
> > > >          count if -funrol-loops is specified and leave it up to costing 
> > > > and
> > >
> > > Typo: "unrol"
> > >
> > > >          the max unroll parameters.
> > > >
> > > >The default is "requested" to match what it does today.
> > >
> > > I don't find the names "requested" and "preferred" very clear, I think
> > > I would always need to check the docs to see what they mean.
> >
> > Yeah, I realized that as well but was having trouble thinking of better 
> > names :)
> >
> > >
> > > For example, does "preferred" mean the pragma's unroll factor should
> > > always be preferred over the cost measurements and max unroll params?
> > > Does "requested" mean the pragma's unroll factor is a request, but
> > > might not be honoured?
> > >
> >
> > Yeah, I initially had "required" instead of "requested" but Richi didn't 
> > like
> > that naming because it gave the impression that the loop must be unrolled,
> > but if cunroll decides it can't, or there's not enough iterations it could 
> > fail.
> 
> Ah yes, good point.
> 
> > Similarly "preferred" could unroll less, more or none at all, it 
> > essentially leaves
> > it up to the target cost model and the target's default unroll amount.
> >
> > > Maybe some other terms with unambiguous meanings can be found,
> > > although you've probably already spent far longer thinking about the
> > > names than I have :-)
> > > Off the top of my head "fixed" and "overridable" could work?
> > > Or "exact" and "hint", or "string" and "weak", ...
> 
> Oops, that was meant to be "strong" not "string".
> 
> > >
> >
> > I think overridable works well instead of preferred! But I'm not sure what 
> > to do
> > about "requested" given that the unrolling is not guaranteed.
> 
> Is it necessary to have a name for the "requested" semantics? If you
> want that, you could just not add the optional argument. So maybe use
> nothing for the current behaviour, and something like "overridable" or
> "suggestion" or "weak" to distinguish the new semantics from the
> original ones.

That's true.  The names are already optional, I can just drop the "requested"
all together.

I'll give it a few to give others a chance to commit and I'll respin dropping 
"requested"

Thanks!

Tamar
> 
> >
> > Will fix the typos in the meantime :)
> >
> > Cheers,
> > Tamar
> >
> > > >Bootstrapped Regtested on aarch64-none-linux-gnu,
> > > >arm-none-linux-gnueabihf, x86_64-pc-linux-gnu
> > > >-m32, -m64 and no issues.
> > > >
> > > >Ok for master?
> > > >
> > > >Thanks,
> > > >Tamar
> > > >
> > > >gcc/ChangeLog:
> > > >
> > > >     PR libstdc++/116140
> > > >     * doc/extend.texi (pragma GCC unroll): Document extension.
> > > >
> > > >---
> > > >diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > > >index
> > >
> 40ccf22b29f4316928f905ec2c978fdaf30a55ec..e87a3c271f8420d8fd175823b5
> > > bb655f76c89afe 100644
> > > >--- a/gcc/doc/extend.texi
> > > >+++ b/gcc/doc/extend.texi
> > > >@@ -10384,14 +10384,19 @@ void foo (int n, int *a, int *b, int *c)
> > > > @}
> > > > @end smallexample
> > > >
> > > >-@cindex pragma GCC unroll @var{n}
> > > >-@item #pragma GCC unroll @var{n}
> > > >+@cindex pragma GCC unroll @var{n} [@var{requested|preferred}]
> > > >+@item #pragma GCC unroll @var{n} [@var{requested|preferred}]
> > > >
> > > > You can use this pragma to control how many times a loop should be
> unrolled.
> > > > It must be placed immediately before a @code{for}, @code{while} or
> @code{do}
> > > > loop or a @code{#pragma GCC ivdep}, and applies only to the loop that
> follows.
> > > > @var{n} is an integer constant expression specifying the unrolling 
> > > > factor.
> > > > The values of @math{0} and @math{1} block any unrolling of the loop.
> > > >+The optional argument indicates whether the user can still override the
> amount.
> > >
> > > s/amount/factor/ ?
> > >
> > > >+When the optional argument is @var{requested} (default) the loop will
> always
> > > be
> > > >+unrolled @var{n} times regardless of any commandline arguments.
> > >
> > > I think this would read better if "(default)" was moved to the end as
> > > "(this is the default)".
> > >
> > > >+When the option is @var{preferred} then the user is allowed to override 
> > > >the
> > > >+unroll amount through commandline options.
> > >
> > > s/amount/factor/ ?
> > >
> > > > @end table
> > > >
> > > >
> > > >
> >

Reply via email to