[flang-commits] [flang] [Flang][OpenMP][Driver][Test] Fix the omp-driver-offload-test commands (PR #66926)

Andrzej WarzyƄski via flang-commits flang-commits at lists.llvm.org
Sun Sep 24 03:23:43 PDT 2023


banach-space wrote:

I don't want to come across as picky, but changing `not %flang` to `%flang` is significant  - it basically means that in one case the end-user will see their compiler failing and in the other succeeding. And I still don't quite follow why in one case the compiler fails and in the other it works fine. 

> This was because if certain environment variables are set for AMD devices some of the commands succeed happily and the not check causes the test to fail.

IIUC, this test does not depend on any env variables and it should succeed regardless of the env variables? If it does depend on env variables then that should be documented. Is it? Similar note on various libraries being present or not.

As per PR title:
> [Flang][OpenMP][Driver][Test] Fix the omp-driver-offload-test commands

This test works for me fine and from my perspective it is being "refined" rather than "fixed". Perhaps there's an issue that affects some folks and this change resolves that, but from your description it sounds like a "downstream" issue? Are there any public buildbots affected by this? 

 "fix" hints to me that there's a bug and in such cases I feel that I should understand what the issue is before I approve a patch. In this case I don't (*), but am happy to rely on your expertise and the expertise of other reviewers. The whole discussion seems to assume a good deal of background on OpenMP offloading for AMD GPUs, which I don't have.


(*) With the exception of the `-o` changes.

https://github.com/llvm/llvm-project/pull/66926


More information about the flang-commits mailing list