[PATCH] D129936: [JITLink][COFF][x86_64] Reimplement ADDR32NB/REL32.

Wed Jul 20 02:38:38 PDT 2022

sgraenitz added a comment.

Nice, thanks for the updates. Are you planning to replace the `COFF_external_func` test or is it fully covered by the new `COFF_addr32nb_reloc` tests?

================
Comment at: llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp:24
+  return T;
+}
+
----------------
Yes, I think it's a lot better to have it explicit like this. Can we still add a short note that summarizes our knowledge about missing COFF object format settings? Thanks!

================
Comment at: llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp:325
+    if (SymbolSet.empty())
+      continue;
     jitlink::Block *B = getGraphBlock(SecIndex);
----------------
How does your patch affect symbol sets so that they might be empty now?

================
Comment at: llvm/test/ExecutionEngine/JITLink/X86/COFF_addr32nb_reloc_neg_addend.test:9
+#
+# jitlink-check: *{4}(pdata) = func - __ImageBase - 4
+--- !COFF
----------------
Can we name a good example here that involves negative addend?

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

https://reviews.llvm.org/D129936