Page MenuHomePhabricator

[LV] Lazy creation of runtime checks
Needs ReviewPublic

Authored by ebrevnov on Sep 8 2021, 9:00 AM.

Details

Reviewers
fhahn
Summary

This change simplifies usage of GeneratedRuntimeChecks abstraction by the end user. With this change there is no need to care about matrialization of the checks since they will be lazily created when required. Also this facilitates future development.

Diff Detail

Unit TestsFailed

TimeTest
1,030 msx64 debian > libomp.lock::omp_init_lock.c
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -I /var/lib/buildkite-agent/builds/llvm-project/openmp/runtime/test -I /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/runtime/src -L /var/lib/buildkite-agent/builds/llvm-project/build/lib -I /var/lib/buildkite-agent/builds/llvm-project/openmp/runtime/test/ompt /var/lib/buildkite-agent/builds/llvm-project/openmp/runtime/test/lock/omp_init_lock.c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/runtime/test/lock/Output/omp_init_lock.c.tmp -lm -latomic && /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/runtime/test/lock/Output/omp_init_lock.c.tmp
720 msx64 windows > lit.lit::test-output-micro.py
Script: -- : 'RUN: at line 1'; env -u FILECHECK_OPTS "C:\Python39\python.exe" C:/ws/w8/llvm-project/premerge-checks/llvm\utils\lit\lit.py -j1 --order=lexical -v C:/ws/w8/llvm-project/premerge-checks/build/utils/lit/tests\Inputs/test-data-micro --output C:\ws\w8\llvm-project\premerge-checks\build\utils\lit\tests\Output\test-output-micro.py.tmp.results.out

Event Timeline

ebrevnov created this revision.Sep 8 2021, 9:00 AM
ebrevnov requested review of this revision.Sep 8 2021, 9:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 8 2021, 9:00 AM
ebrevnov updated this revision to Diff 372186.Sep 13 2021, 2:23 AM
ebrevnov edited the summary of this revision. (Show Details)

Update

ebrevnov retitled this revision from [LV][WIP] Lazy creation of runtime checks to [LV] Lazy creation of runtime checks.Sep 13 2021, 2:23 AM

Also this facilitates future development.

Could you provide a few more details on this, i.e. what the drawbacks of explicit initialization are? Personally I find the code easier to follow if things happen explicitly. Are you anticipating the need to generate the runtime checks at different points?

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
10052

Do this need to be moved?

10288

does this need to be moved?

Also this facilitates future development.

Could you provide a few more details on this, i.e. what the drawbacks of explicit initialization are?

Honestly, I find current API a bit confusing. I see the following drawbacks in priority:

  1. Current API is a potential source of future errors since it is easy to misplace a call to Create() or miss it completely. There is no easy way to detect the mistake.
  2. Negatively affects end user experience if the existing API.
  3. Requires code changes which could be avoided otherwise.

In the end, why should we care about explicit initialization if everything can be done automatically behind the scene?

Personally I find the code easier to follow if things happen explicitly. Are you anticipating the need to generate the runtime checks at different points?

In D75981, you already introducing two new calls to Create() and move one existing call to new place. All these changes could be avoided with lazy approach.

ebrevnov added inline comments.Sep 27 2021, 12:00 AM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
10052

Not strictly necessary for this patch. Last patch in the series makes LoopVectorizationCostModel to take RTChecks.

10288

Same as above.