[PATCH] D129052: [Object] Add ELF section type for offloading objects

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

We probably need a test case showing that the assembler understands "@llvm_offloading" when reading it (and produces an SHT_LLVM_OFFLOADING section).



================
Comment at: llvm/docs/Extensions.rst:456
+This section stores the binary data used to perform offloading device linking
+and execution, creating a fat-binary. This section is emitted during compilation
+of offloading languages such as OpenMP or CUDA. If the data is intended to be
----------------
I'm not all too familiar with this area, but should "fat-binary" be hyphenated? I'd have thought it would have been two separate words (since the adjective "fat" isn't tied to the noun "binary"). OTOH, if that's how it's always referred to, I guess it's fine.


================
Comment at: llvm/docs/Extensions.rst:461
+
+The binary data stored at this section conforms to a custom binary format used
+for storing offloading metadata. This format is effectively a string table
----------------



================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:504-506
+  if (hasPrefix(Name, ".llvm.offloading"))
+    return ELF::SHT_LLVM_OFFLOADING;
+
----------------
jhuber6 wrote:
> I'm not sure about just checking the name here. The way these are generated by clang is just dumping the contents of a file to a constant string global in LLVM-IR like so:
> ```
> @llvm.embedded.object = private constant [0 x i8] zeroinitializer, section ".llvm.offloading", align 8
> ```
> I could probably add some metadata node to indicate this instead like this, but I wasn't sure if it was worth introducing yet more attributes for something that's somewhat niche like this.
> ```
> @llvm.embedded.object = private constant [0 x i8] zeroinitializer, section ".llvm.offloading", align 8 !offloading
> ```
> 
> Also, I may want to distinguish between "offloading" data intended for linking or execution. The former should get the `SHF_EXCLUDE` flag while the latter should not. It would be nice to be able to pass this to the backend somehow. Let me know what you think.
Unfortunately, I don't really know CodeGen personally, so I can't advise - someone else needs to comment here.

Unrelated to that discussion, but is this line tested? (It might be, but I'd somewhat expect to see a test case which checks the section type of an IR-produced section which doesn't go via asm or something like that - (this may all be nonsense due to lack of familiarity).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129052/new/

https://reviews.llvm.org/D129052