On Thu, Aug 13, 2020 at 01:58:31PM -0500, Peter Bergner wrote:
> On 8/12/20 8:59 PM, Peter Bergner wrote:
> > On 8/12/20 8:00 PM, Segher Boessenkool wrote:
> >> On Wed, Aug 12, 2020 at 03:32:18PM -0500, Peter Bergner wrote:
> > Ok, how about this comment then?
> >
> > @@ -6444,8 +6444,30 @@ machine_mode
> > rs6000_promote_function_mode (const_tree type ATTRIBUTE_UNUSED,
> > machine_mode mode,
> > int *punsignedp ATTRIBUTE_UNUSED,
> > - const_tree, int)
> > + const_tree, int for_return)
> > {
> > + /* Warning: this is a static local variable and not always NULL!
> > + This function is called multiple times for the same function
> > + and return value. PREV_FUNC is used to keep track of the
> > + first time we encounter a function's return value in order
> > + to not report an error with that return value multiple times. */
> > + static struct function *prev_func = NULL;
>
> Approved offline, so I pushed this to trunk. Thanks!
>
> Are we ok to backport this to GCC 10? If you don't want this
> trickery in GCC 10, we could just backport the param handling
> which doesn't use the trickery and leave the return value
> unhandled.
It's okay for backporting as well. It's all kind of wrong, but it will
in practice just work anyway:
1) struct function is GTY, and you don't mark the static variable here
as root, so the thing it points into might have gone away; but we really
only use the pointer here, we don't deref anything explicitly, so the
generated program won't either (hopefully, etc.)
2) Similarly, some other struct function for another function may (in
theory) be allocated at the same address when next we are called; that
other function will then never get the warning.
We'd need to store a flag in the struct function (or similarly) itself,
to make things kosher. How do similar warnings elsewhere handle this?
Anyway, okay for trunk and backports. Thanks!
Segher
