This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP][RTL] Remove dead code
ClosedPublic

Authored by pdhaliwal on Oct 5 2020, 6:08 AM.

Details

Summary

RequiresDataSharing was always 0, resulting dead code in device runtime library.

Diff Detail

Event Timeline

pdhaliwal created this revision.Oct 5 2020, 6:08 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 5 2020, 6:08 AM
pdhaliwal requested review of this revision.Oct 5 2020, 6:08 AM

Change looks great to me.

Rolling the reduction in leading whitespace in nvptx_target_parallel_reduction_codegen.cpp in with the patch might be contentious, added a couple more reviewers to see if other people would prefer that part split out. I'll accept in a day or so if there are no comments on the whitespace.

Thanks!

openmp/libomptarget/deviceRTLs/common/src/omptarget.cu
135–136

This is interesting. This turns out to be the only call to RootS(), and that cascades through a bunch of other code removed in this patch.

grokos added a comment.Oct 5 2020, 6:46 AM

Rolling the reduction in leading whitespace in nvptx_target_parallel_reduction_codegen.cpp in with the patch might be contentious, added a couple more reviewers to see if other people would prefer that part split out. I'll accept in a day or so if there are no comments on the whitespace.

Fine by me, I don't think it's worth uploading an extra patch just for this minor formatting detail.

jdoerfert accepted this revision.Oct 5 2020, 7:05 AM

LGTM, thanks for removing dead code.

This revision is now accepted and ready to land.Oct 5 2020, 7:05 AM
JonChesterfield accepted this revision.Oct 5 2020, 7:09 AM

Thanks guys, will remember that as the local convention on rolling whitespace changes into other stuff.

This revision was landed with ongoing or failed builds.Oct 6 2020, 2:44 AM
This revision was automatically updated to reflect the committed changes.