On Thu, Jan 21, 2021 at 08:32:52AM +0100, Thomas Schwinge wrote:
> > and which of those depend, priority and detach argument is present depends
> > on the bits in flags.
> > I'm afraid the compiler just decided to spill the detach = NULL store in
> >   if ((flags & GOMP_TASK_FLAG_DETACH) == 0)
> >     detach = NULL;
> > on s390x into the argument stack slot.  Not a problem if the caller passes
> > all those 10 arguments, but if not, can clobber random stack location.
> >
> > This hack should fix it up.  Priority doesn't need changing, but I've
> > changed it anyway just to be safe.  With the patch none of the 3 arguments
> > are ever modified, so I'd hope gcc doesn't decide to spill something
> > unrelated there.
> 
> That still seems fragile; is "hope gcc doesn't decide to spill" really
> sufficient?
> 
> Cannot we (easily) use symbol versioning to introduce new entry point
> variants (which then internally all route to the same function)?

Not all targets support GNU style function versioning where one can have
multiple symbol versions for the same symbol name, and I wanted to
avoid GOMP_task2, GOMP_task3 etc.
In retrospect, it would have been probably better to make the function
void (*fn) (void *), /* ... */, unsigned flags, ...)
but it is too late to change that now.
And we can't really change at least the GOMP_task with just flags, with also
depend and with also priority, so we have that potential problem anyway.

On the other side, I really don't see a reason why gcc would try to spill
something that has never been changed.

        Jakub

Reply via email to