On Mon, Apr 28, 2025 at 07:27:31PM +0200, Josef Melcr wrote:
> As for the attribute, I am honestly not too sure about what to do, as clang
> is
> not consistent in with its own indexing, be it with the unknown values, or
> with
> 'this'. I've tried to remain consistent with GCC's indexing style. I guess
> I'll
> leave up to you and the other maintainers to decide. I can implement clangs
> version 1:1, put the attribute in our namespace or rename it. I don't mind
> either way. Another option would be to patch clang to get in line with the
> rest
> of its attributes. It seems like the best option to me, as it would make
> being
> consistent way easier, but it would be problematic, as all code using this
> attribute would need to be updated.

I'll talk to C/C++ FE maintainers what they think.
The attribute is after all not really OpenMP related, it is something
that can/should be used on qsort_r and the likes.

> > Another question is about GOMP_task.  Can you handle any constant
> > propagation into that?  I see you've tried to deal with it by using 2
> > callback attributes but with using 0 for the argument.  Wouldn't it be
> > better to just special case BUILT_IN_GOMP_TASK in IPA?
> Propagating into the body function is currently working. Propagating into
> the
> copy function apparently still needs some work, as the example below causes
> a
> crash, sorry about that. I'll fix that and add tests for GOMP_task.

I think the most important at least for the first version is to handle
propagation into body function if copy function is NULL.  That is quite
common case and easily handleable (though I'd think GOMP_task should be a
special case and not use the attribute at all, just the compiler should
treat it as if it has one if the cpyfn argument is NULL).  The rest could
be handled incrementally.

As for the more complex case, I'm not really sure how can propagation into
body work and propagation into the cpyfn not.  In that case propagation
into cpyfn is the easy case, the type of the passed in structure is the same
as the cpyfn receives.  The cpyfn then fills in a different structure and
that is passed to the body.  So, either some analysis would need to go
through the cpyfn and see ok, we've been able to propagate constant here,
and that constant is then stored into this member of the other argument and
not really modified elsewhere, so we can propagate that further.
Or we could go with extra attributes on the FIELD_DECLs of the 2 structures
from omp lowering in that case, mark for the cpyfn case fields which are
just copied through by the cpyfn unmodified, then one can simply find
FIELD_DECL with the same DECL_NAME/type and propagate that.

> I will look into that to see what can be done, though I'd like to introduce
> such extensions incrementally, as the patch is
> already large enough :)

Sure.

        Jakub

Reply via email to