This is an archive of the discontinued LLVM Phabricator instance.

[IRCE] Parse range checks in the form of "LHS - RHS vs Limit"
ClosedPublic

Authored by aleksandr.popov on Jun 29 2023, 5:04 AM.

Details

Summary

Introduced the following range checks forms parsing:

  • IV - Offset vs Limit
  • Offset - IV vs Limit

Range's end boundary is computed as (Offset +/- Limit ).

If it's not possible to prove at compile time that computed upper bound
will not overflow, then scale boundary computation to a wider type to
perform overflow check at runtime.

Runtime overflow will be implemented in the next patch. In the meantime
safe range for such kind of checks isn't computed.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptJun 29 2023, 5:04 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
aleksandr.popov requested review of this revision.Jun 29 2023, 5:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 29 2023, 5:04 AM
aleksandr.popov edited the summary of this revision. (Show Details)Jun 29 2023, 3:40 PM
skatkov added inline comments.Jul 2 2023, 10:31 PM
llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
386

More context here in comment. Why with this restriction failed we cannot process.

405

What exactly overflow you can expect?
Does it makes sense to check, may be we can prove that overflow is not possible and we can avoid scaling?

434

Add some message

1747

Can we add some LLVM_DEBUG output here and add a test which tests whether it is printed and check the values of getBegin(), getEnd() and IndVar?

It would be nice to have tests for this functionality?

aleksandr.popov retitled this revision from [IRCE][NFC] Parse range checks in the form of "LHS - RHS vs Limit" to [IRCE] Parse range checks in the form of "LHS - RHS vs Limit".
aleksandr.popov edited the summary of this revision. (Show Details)

Update according to review comments

aleksandr.popov marked 3 inline comments as done.Jul 6 2023, 8:23 AM
aleksandr.popov added inline comments.
llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
386

I removed the check that initial subtraction will not overflow and added explanation why we don't need that check

405

Good point, done

1747

Done, thanks!

One comment, other looks good...

llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
474

what if you did not scale and Limit == S_INT_MAX? Limit + 1 will overflow then?

aleksandr.popov marked 2 inline comments as done.Jul 7 2023, 2:31 AM
aleksandr.popov added inline comments.
llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
474

Then we will scale Limit + 1, if we can't prove that if doesn't overflow

Add more tests

skatkov accepted this revision.Jul 7 2023, 2:38 AM
This revision is now accepted and ready to land.Jul 7 2023, 2:38 AM
aleksandr.popov added inline comments.Jul 7 2023, 2:41 AM
llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
474

I've added 2 tests:

  • In first one we have IV < N + 2;
  • in the second one we have IV <= N + 2;

In both cases 8-bit N < 126.

In the first example we can prove that (N + 2) will not overflow and we don't scale.
In the second one we are adding 1 to the (N + 2), since predicate is not-strict. Afterwards no-overflow becomes unprovable and we scale.

This may have taken down the openmp amdgpu bot https://lab.llvm.org/buildbot/#/builders/193 - @jhuber6 @jdoerfert @ronlieb we don't seem to get any information from the failing tests about what went wrong, e.g.

******************** TEST 'libomptarget :: amdgcn-amd-amdhsa :: api/assert.c' FAILED ********************
Script:
--
: 'RUN: at line 1';   /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang -fopenmp    -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/test -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/libomptarget -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/libomptarget -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -fopenmp-targets=amdgcn-amd-amdhsa /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/test/api/assert.c -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/libomptarget/test/amdgcn-amd-amdhsa/api/Output/assert.c.tmp && /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/libomptarget/test/amdgcn-amd-amdhsa/api/Output/assert.c.tmp | /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/test/api/assert.c
--
Exit Code: 2
Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang" "-fopenmp" "-I" "/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/test" "-I" "/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src" "-L" "/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/libomptarget" "-L" "/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib" "-L" "/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src" "-Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/libomptarget" "-Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src" "-Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib" "-fopenmp-targets=amdgcn-amd-amdhsa" "/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/test/api/assert.c" "-o" "/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/libomptarget/test/amdgcn-amd-amdhsa/api/Output/assert.c.tmp"
$ "/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/libomptarget/test/amdgcn-amd-amdhsa/api/Output/assert.c.tmp"
note: command had no output on stdout or stderr
error: command failed with exit status: -11
$ "/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck" "/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/test/api/assert.c"
# command stderr:
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/test/api/assert.c
error: command failed with exit status: 2
--
********************

Kicked the bot to see if it fails again.

It looks like next bot failed in the same way.
Could you please provide steps to reproduce the failure?

@Aleksandr Popov, could you please revert patch for a while we figure out what happens...

Thanks for the revert, has taken us back to green. Repro is build openmp and run check-openmp, most of them failed, but I'd guess this was a problem in codegen. Openmp isn't always very easy to build, hopefully someone has some spare cycles to see what's gone wrong.

Thanks for the revert, has taken us back to green. Repro is build openmp and run check-openmp, most of them failed, but I'd guess this was a problem in codegen. Openmp isn't always very easy to build, hopefully someone has some spare cycles to see what's gone wrong.

Seems the libc tests for AMDGPU were fine during that period, so it's probably something specific to OpenMP unfortunately https://lab.llvm.org/staging/#/builders/247/builds/2795.

Repro is build openmp and run check-openmp

I've tried to reproduce the tests, but AMDGPU ones (which were actually failed) were not generated:

...
-- LIBOMPTARGET: Not generating AMDGPU tests, no supported devices detected.

Do you know how to fix that?

Repro is build openmp and run check-openmp

I've tried to reproduce the tests, but AMDGPU ones (which were actually failed) were not generated:

...
-- LIBOMPTARGET: Not generating AMDGPU tests, no supported devices detected.

Do you know how to fix that?

It should (ideally) depend on whether or not you have ROCm and a functional GPU on your system. It basically just calls amdgpu-arch to see if there's any output. This is to prevent OpenMP from running GPU tests on a machine that can't support it. Either make sure that the system is configured at build time, or us LIBOMPTARGET_FORCE_AMDGPU_TESTS=ON to override.

aleksandr.popov added a comment.EditedJul 11 2023, 2:39 AM

It should (ideally) depend on whether or not you have ROCm and a functional GPU on your system. It basically just calls amdgpu-arch to see if there's any output.

Unfortunately amdgpu-arch not found on my machine. I've got amdgpu-install tool but what exactly should I install to execute AMDGPU tests?

BTW, without installed amdgpu all tests failed the same way as they failed in the https://lab.llvm.org/buildbot/#/builders/193

And one more question: my changes relate to InductiveRangeCheckElimination only. How does OpenMP use IRCE which caused the tests to fail?

Hi @jplehr! Could you please help with reproducing AMDGPU tests locally?

nikic added a subscriber: nikic.Jul 11 2023, 3:11 AM

IRCE isn't used in-tree at all, so it's very weird that it affects OpenMP -- or anything at all.

Hi @jplehr! Could you please help with reproducing AMDGPU tests locally?

Sure. I'll check locally and see what happens.

It should (ideally) depend on whether or not you have ROCm and a functional GPU on your system. It basically just calls amdgpu-arch to see if there's any output.

Unfortunately amdgpu-arch not found on my machine. I've got amdgpu-install tool but what exactly should I install to execute AMDGPU tests?

BTW, without installed amdgpu all tests failed the same way as they failed in the https://lab.llvm.org/buildbot/#/builders/193

And one more question: my changes relate to InductiveRangeCheckElimination only. How does OpenMP use IRCE which caused the tests to fail?

amdgpu-arch should be built unconditionally by clang. So generally to build OpenMP we'd recommend CMake like -DLLVM_ENABLE_PROJECTS=clang;lld -DLLVM_ENABLE_RUNTIMES=openmp -DLIBOMPTARGET_FORCE_AMDGPU_TESTS=ON

Chances are that a recent patch I made broke something in libomptarget and this and other patches spuriously trigger the failure. Currently looking into it.

@jhuber6 Does https://reviews.llvm.org/D154971 fix my case? Could I try to reland the patch?

@jhuber6 Does https://reviews.llvm.org/D154971 fix my case? Could I try to reland the patch?

Worth a shot, go for it.

Worth a shot, go for it.

Thanks, let's try