This is an archive of the discontinued LLVM Phabricator instance.

[OpenMPOpt] Move dedup runtime calls after init for target regions
ClosedPublic

Authored by ggeorgakoudis on Jul 22 2021, 8:16 AM.

Details

Summary

Deduplication in OpenMPOpt finds redundant OpenMP runtime calls and replaces them with a single call placed in the earliest safe location in the IR. When deduplication happens in a target region this patch makes sure replacement calls are put after target_init.

Diff Detail

Event Timeline

ggeorgakoudis created this revision.Jul 22 2021, 8:16 AM
ggeorgakoudis requested review of this revision.Jul 22 2021, 8:16 AM
Herald added a project: Restricted Project. · View Herald Transcript
ggeorgakoudis edited the summary of this revision. (Show Details)Jul 22 2021, 8:20 AM
jdoerfert accepted this revision.Jul 22 2021, 8:25 AM
jdoerfert added a subscriber: jhuber6.

LG, one comment.

llvm/lib/Transforms/IPO/OpenMPOpt.cpp
1586

@jhuber6 suggested to not emit a remark here and removed it earlier.

llvm/test/Transforms/OpenMP/deduplication_target_remarks.ll
1 ↗(On Diff #360823)

Probably don't need this anymore then.

This revision is now accepted and ready to land.Jul 22 2021, 8:25 AM
jhuber6 added inline comments.Jul 22 2021, 8:28 AM
llvm/lib/Transforms/IPO/OpenMPOpt.cpp
1586

Yeah, I decided that it wasn't really important for the user to know that the call was moved anywhere and just cluttered up the existing remarks. Mostly because the user has no clue where the beginning of the OpenMP region truly exists in their code and it's not really an optimization per-se. If you want to add it in you'd also need to give it an OMPxxx number and write some documentation for it.

jdoerfert added inline comments.Jul 22 2021, 9:16 AM
llvm/lib/Transforms/IPO/OpenMPOpt.cpp
1593

It could happen that this is empty, in the CGSCC pass for example.
In that case just continue and do not move anything.

ggeorgakoudis marked 3 inline comments as done.

Fix for comments

ggeorgakoudis added inline comments.Jul 22 2021, 7:10 PM
llvm/lib/Transforms/IPO/OpenMPOpt.cpp
1586

Makes sense, removed.

This revision was landed with ongoing or failed builds.Jul 23 2021, 5:54 AM
This revision was automatically updated to reflect the committed changes.