Page MenuHomePhabricator

[OpenMPIRBuilder] Implement collapseLoops.
ClosedPublic

Authored by Meinersbur on Dec 14 2020, 7:41 PM.

Details

Summary

The collapseLoops method implements a transformations facilitating the implementation of the collapse-clause. It takes a list of loops from a loop nest and reduces it to a single loop that can be used by other methods that are implemented on just a single loop, such as createStaticWorkshareLoop.

This patch shares some changes with D92974 (such as adding some getters to CanonicalLoopNest), used by both patches.

Diff Detail

Unit TestsFailed

TimeTest
320 msx64 debian > Profile-x86_64.Profile-x86_64::instrprof-gc-sections.c
Script: -- : 'RUN: at line 3'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang -m64 -ldl -fprofile-instr-generate=/mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/profile/Profile-x86_64/Output/instrprof-gc-sections.c.tmp.profraw -fuse-ld=lld -fcoverage-mapping -mllvm -counter-link-order -mllvm -enable-name-compression=false -DCODE=1 -ffunction-sections -fdata-sections -Wl,--gc-sections -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/profile/Profile-x86_64/Output/instrprof-gc-sections.c.tmp /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/profile/instrprof-gc-sections.c
280 msx64 debian > libarcher.races::task-dependency.c
Script: -- : 'RUN: at line 13'; /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/races/task-dependency.c -o /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-dependency.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1' /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/deflake.bash /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-dependency.c.tmp 2>&1 | tee /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-dependency.c.tmp.log | /mnt/disks/ssd0/agent/llvm-project/build/./bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/races/task-dependency.c
250 msx64 debian > libarcher.races::task-taskgroup-unrelated.c
Script: -- : 'RUN: at line 13'; /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/races/task-taskgroup-unrelated.c -o /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-taskgroup-unrelated.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1' /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/deflake.bash /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-taskgroup-unrelated.c.tmp 2>&1 | tee /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-taskgroup-unrelated.c.tmp.log | /mnt/disks/ssd0/agent/llvm-project/build/./bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/races/task-taskgroup-unrelated.c
230 msx64 debian > libarcher.races::task-taskwait-nested.c
Script: -- : 'RUN: at line 13'; /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/races/task-taskwait-nested.c -o /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-taskwait-nested.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1' /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/deflake.bash /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-taskwait-nested.c.tmp 2>&1 | tee /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-taskwait-nested.c.tmp.log | /mnt/disks/ssd0/agent/llvm-project/build/./bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/races/task-taskwait-nested.c
300 msx64 debian > libarcher.races::task-two.c
Script: -- : 'RUN: at line 13'; /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/races/task-two.c -o /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-two.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1' /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/deflake.bash /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-two.c.tmp 2>&1 | tee /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-two.c.tmp.log | /mnt/disks/ssd0/agent/llvm-project/build/./bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/races/task-two.c
View Full Test Results (14 Failed)

Event Timeline

Meinersbur created this revision.Dec 14 2020, 7:41 PM
Meinersbur requested review of this revision.Dec 14 2020, 7:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 14 2020, 7:41 PM
Herald added a subscriber: sstefan1. · View Herald Transcript

Cool, really happy to see this. I tried to follow the logic but had some questions. Most comments are nits though.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
1183

Nit: Assert message.

1191

Nit: auto *Pred at least.

1219

Nit: Maybe do ... while(Changed) but that's just an idea.

1223

Do we really need/want to do such a search here? We could leave them be I guess, or delete them as we make them dead?


Interesting choices for sizes of the set and vector, I'm curious why

1260

Nit: Can we add a TODO saying that an OpenMP sanitizer could use overflow tracking multiplication here and report an error if it does overflow.

1319

How can ContinueBlock in 1297 be non-null? Am I reading this wrong or are we using ContinueBlock once and then ContinuePred for all other loops?

1332

Maybe we want to move this body into a helper, it appears twice, unclear if that helps.

1338

Again I don't see how ContinueBlock is non-null, actually, even 1309 it is always null, isn't it? Something seems to be off, maybe the two are swapped?

Meinersbur marked 5 inline comments as done.
  • Rebase after commit of D92974
  • Address review comments
jdoerfert accepted this revision.Feb 3 2021, 8:13 AM

It's a little hard to determine what this does from the code and unit test. It looks reasonable. I'm fine with including it for now as it won't harm.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
1314

As far as I can tell, IncludeNextSrcBB is always false.

This revision is now accepted and ready to land.Feb 3 2021, 8:13 AM
Meinersbur updated this revision to Diff 321262.Feb 3 2021, 4:46 PM
  • Address @jdoerfert's remark
  • Avoid 'comparison of integer expressions of different signedness' warning
This revision was landed with ongoing or failed builds.Feb 3 2021, 5:12 PM
This revision was automatically updated to reflect the committed changes.