This is an archive of the discontinued LLVM Phabricator instance.

[flang][openacc] Update materialization recipe for private copy in reduction init region
ClosedPublic

Authored by clementval on Jul 20 2023, 1:47 PM.

Details

Summary

Update the code generated in the init region to materialize the private
copy.

Diff Detail

Event Timeline

clementval created this revision.Jul 20 2023, 1:47 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 20 2023, 1:47 PM
clementval requested review of this revision.Jul 20 2023, 1:47 PM

Update test

razvanlupusoru accepted this revision.Jul 21 2023, 8:10 AM

Small nit on the title of this patch.
Materilize the private copy in reduction init region
->
Update materialization recipe for private copy in reduction init region

I would say that materialization happens when recipe is used, not created.

The other question is whether a name should also be passed to recipe for the fir.alloca. To me it makes sense since the recipe may be reused. If we did want to give a name to the private later when materialization actually happens, the recipe must be updated with a new fir.alloca and the materializer must actually understand the recipe to find the allocation - so hence why it makes sense to me to give it name as argument now. If you think so too, it can be something to follow-up on in the future - not in this patch.

From the current code perspective, it looks good to me.

This revision is now accepted and ready to land.Jul 21 2023, 8:10 AM

Small nit on the title of this patch.
Materilize the private copy in reduction init region
->
Update materialization recipe for private copy in reduction init region

I would say that materialization happens when recipe is used, not created.

The other question is whether a name should also be passed to recipe for the fir.alloca. To me it makes sense since the recipe may be reused. If we did want to give a name to the private later when materialization actually happens, the recipe must be updated with a new fir.alloca and the materializer must actually understand the recipe to find the allocation - so hence why it makes sense to me to give it name as argument now. If you think so too, it can be something to follow-up on in the future - not in this patch.

From the current code perspective, it looks good to me.

I agree. I'll make a follow up patch for this.

clementval retitled this revision from [flang][openacc] Materilize the private copy in reduction init region to [flang][openacc] Update materialization recipe for private copy in reduction init region.Jul 24 2023, 8:32 AM
This revision was landed with ongoing or failed builds.Jul 24 2023, 9:33 AM
This revision was automatically updated to reflect the committed changes.