[PATCH] D133392: [MTE] Add AArch64GlobalsTagging Pass

[Mitch Phillips via Phabricator via cfe-commits]

hctim added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalMerge.cpp:657
+    // Tagged global variables shouldn't be merged, as they are assigned unique
+    // memory tags at runtime.
+    if (GV.isTagged())
----------------
eugenis wrote:
> This comment is not convincing. The change description is right that some globals can be merged, copy that here, or extend the comment a little to explain why and under what assumptions.
Thanks, extended the comment.


================
Comment at: llvm/lib/MC/ELFObjectWriter.cpp:1344
+  // when relocating `end` symbols, and this can only be determined by the
+  // attributes of the symbol itself.
+  if (Sym->isMemtag())
----------------
eugenis wrote:
> Isn't a bigger reason that we need to know the address tag to put on the pointer, and that requires a symbol, not a section reference?
removed this comment here, as this was moved into the parent patch https://reviews.llvm.org/D131863 and the comment was updated there. this comment references the old st_other-based implementation, whereas now it's needed for SHT_AARCH64_MEMTAG_GLOBALS_STATIC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133392