Minor cleanup: As these are no longer passed to UnrollLoop(), there is no need to modify them in computeUnrollCount(). Make them non-reference parameters.
|540 ms||x64 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
It would be nice to get rid of UseUpperBound as well, but apparently this is still being used by LoopUnrollAndJam in https://github.com/llvm/llvm-project/blob/876de062f94650f9ded56a22b062236f711fcd18/llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp#L177. No test coverage though :(
Ouch, this code is all around terrible isn't it?
I may be missing something, but I think you're changing behavior here. computeUnrollCount appears to indirectly control ULO.Count via this line. Unless I'm misreading this? If I'm not, I'm surprised tests don't cover this.
I think this is just dead code and have dropped it. UnrollLoop() will clamp the unroll count to the max trip count itself, so this is redundant.
Note that TripCount and TripMultiple are also used in the UP.Runtime adjustment below, but computeUnrollCount() only changes TripCount and TripMultiple when fully unrolling, so it shouldn't matter for runtime unrolling.