[PATCH] D47566: AMDHSA: Code object v3 updates

[Tony Tye via Phabricator via llvm-commits]

t-tye added inline comments.


================
Comment at: lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.cpp:403-409
+  Streamer.EmitValue(MCBinaryExpr::createSub(
+      MCSymbolRefExpr::create(
+          KernelCodeSymbol, MCSymbolRefExpr::VK_AMDGPU_REL64, Context),
+      MCSymbolRefExpr::create(
+          KernelDescriptorSymbol, MCSymbolRefExpr::VK_None, Context),
+      Context),
+      sizeof(KernelDescriptor.kernel_code_entry_byte_offset));
----------------
nhaehnle wrote:
> Isn't the REL64 here redundant? The way I see it, we should either have an //absolute// reference to `(start of kernel code) - (start of kernel descriptor)`, or a //relative// reference to `(start of kernel code) - offsetof(kernel_descriptor_t, kernel_code_entry_byte_offset)`.
> 
> I'm not deep enough in MC to say for certain which of these is really preferable.
The field requires an offset to the entry point so an absolute relocation cannot be used. The Rel64 relocation record is what describes what is needed. Since the kernel descriptor and entry point are now in different sections, a static relocation record is needed so that it can be fixed up when the relocatable code object is linked to make a shared object.

Previously the kernel descriptor was put in the same section as the code, and the offset was "hard-wired" when it was generated.


https://reviews.llvm.org/D47566