[PATCH] D68822: [WIP][BPF] Support external globals

[Andrii Nakryiko via Phabricator via llvm-commits]

anakryiko added inline comments.


================
Comment at: llvm/lib/Target/BPF/BTF.h:183-184
   VAR_GLOBAL_ALLOCATED = 1, ///< Linkage: ExternalLinkage
-  VAR_GLOBAL_TENTATIVE = 2, ///< Linkage: CommonLinkage
-  VAR_GLOBAL_EXTERNAL = 3,  ///< Linkage: ExternalLinkage
+  VAR_GLOBAL_EXTERNAL = 2,  ///< Linkage: ExternalLinkage
+  VAR_GLOBAL_TENTATIVE = 3, ///< Linkage: CommonLinkage
 };
----------------
what's the difference between EXTERNAL and TENTATIVE?


================
Comment at: llvm/lib/Target/BPF/BTFDebug.cpp:1064
+    // Whether DataSec is readonly or not can be found from the
+    // corresponding ELF section flags.
 
----------------
I remember there were problems figuring out datasec size when emitting BTF. Is this still the case? Does the same problem prevent readonly flag in DATASEC or you are trying to not put redundant information into BTF that can be derived from ELF?


================
Comment at: llvm/lib/Target/BPF/BTFDebug.cpp:1112
 
+    if (SecName.empty())
+      continue;
----------------
we'll just ignore externs with GlobalValue::CommonLinkage, right? Should we just assign a special section name to them instead? feels bad to only have a partial list of externs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68822