Page MenuHomePhabricator

[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

Unit TestsFailed

TimeTest
1,150 mslinux > libarcher.parallel::parallel-nosuppression.c
Script: -- : 'RUN: at line 15'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -g -O1 -fsanitize=thread -I /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests -I /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/src -L /mnt/disks/ssd0/agent/llvm-project/build/lib -Wl,-rpath,/mnt/disks/ssd0/agent/llvm-project/build/lib /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/parallel/parallel-nosuppression.c -o /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/parallel/Output/parallel-nosuppression.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=0' /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/parallel/Output/parallel-nosuppression.c.tmp 2>&1 | tee /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/parallel/Output/parallel-nosuppression.c.tmp.log | /mnt/disks/ssd0/agent/llvm-project/build/./bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/parallel/parallel-nosuppression.c

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

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.