[llvm] Add --offoading option to llvm-readobj (PR #143342)
James Henderson via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 12 01:56:06 PDT 2025
================
@@ -0,0 +1,42 @@
+## Test that --offloading with a fatbin works correctly
+# REQUIRES: target={{x86_64-.*-linux.*}}
+# REQUIRES: amdgpu-registered-target
----------------
jh7370 wrote:
> It is target specific. Only targets where a hip_fatbin offload section is generated. So AMDGPU targets.
Maybe I'm misunderstanding something, but I'm not convinced this should restrict under what configurations the test should run. yaml2obj can generate the section data regardless of what targets have been enabled in LLVM. Equally, llvm-readobj can read ELF object data without needing targets to be enabled, since the code lives in the `Object` library. Thus e.g. an x86 host could easily support this test without needing any particular configuration of LLVM such as the AMDGPU target being enabled. This means a `REQUIRES` target directive is inappropriate. This would be different if e.g. llvm-mc was being used to generate the input object, since llvm-mc requires targets to be enabled.
> There is an issue on Windows where yam2obj doesn't properly generate the object file from this test.
I don't think `target` is the right thing to be using for this as this should be based on the system host, which may well be Windows targeting Linux. Specifically, you want something like `UNSUPPORTED: system-windows`. This will also allow the test to run on e.g. Mac systems. I'd even consider making it `XFAIL` if the intent is that it should work, but something unexpected is causing it to not work. That way, when the issue is fixed, the test will start XPASSing and the annotation can be removed, whereas with UNSUPPORTED, it might get forgotten about.
The annotation needs an accompanying comment too.
> Missing full stop at end of comment.
This nit hasn't been addressed.
https://github.com/llvm/llvm-project/pull/143342
More information about the llvm-commits
mailing list