This is an archive of the discontinued LLVM Phabricator instance.

[flang][openacc] Lower acc declare to the new acc.declare function
ClosedPublic

Authored by clementval on Aug 18 2023, 2:35 PM.

Details

Summary

Lower the acc delcare directive in function/subroutine
to the newly introduced acc.declare operation. Only a single
acc.declare operation is procduced in a function or subroutine
so they don't end up nested.

Depends on D158314

Diff Detail

Event Timeline

clementval created this revision.Aug 18 2023, 2:35 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 18 2023, 2:35 PM
Herald added a subscriber: mehdi_amini. · View Herald Transcript
clementval requested review of this revision.Aug 18 2023, 2:35 PM

Looks good to me. Thank you!

Only a single acc.declare operation is procduced in a function or subroutine so they don't end up nested.

If this is a hard requirement, then we should probably assert it in the verifier code in D158314.

Looks good to me. Thank you!

Only a single acc.declare operation is procduced in a function or subroutine so they don't end up nested.

If this is a hard requirement, then we should probably assert it in the verifier code in D158314.

I am hoping its just for readability. It should NOT be a hard requirement IMO. We should be able to inline functions with declare clauses and allow nesting.

Looks good to me. Thank you!

Only a single acc.declare operation is procduced in a function or subroutine so they don't end up nested.

If this is a hard requirement, then we should probably assert it in the verifier code in D158314.

This is not really a requirement of the dialect itself but it's more to simplify the lowering and especially the finalization of the implicit region.

razvanlupusoru accepted this revision.Aug 18 2023, 2:49 PM

I like this better - it feels more idiomatic MLIR. Thank you!

This revision is now accepted and ready to land.Aug 18 2023, 2:49 PM

Looks good to me. Thank you!

Only a single acc.declare operation is procduced in a function or subroutine so they don't end up nested.

If this is a hard requirement, then we should probably assert it in the verifier code in D158314.

I am hoping its just for readability. It should NOT be a hard requirement IMO. We should be able to inline functions with declare clauses and allow nesting.

Sounds reasonable. Thank you!