[PATCH] D82615: [HWASan] [GlobalISel] Add +tagged-globals backend feature for GlobalISel

[Mitch Phillips via Phabricator via llvm-commits]

hctim added inline comments.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp:693
+  if (OpFlags & AArch64II::MO_TAGGED) {
+    auto Tag = MI.getOperand(1);
+    Tag.setTargetFlags(AArch64II::MO_PREL | AArch64II::MO_G3);
----------------
arsenm wrote:
> Isn't this a copy of the original operand, so the further modifications don't do anything? Needs a reference?
I'm admittedly not very familiar with the backend, the goal here is to emit a *new* `movk` instruction w/ an immediate that's relocated with `R_AARCH64_MOVW_PREL_G3`.

I assume that we want to copy the operand so that we can generate the right relocation here?


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp:694
+    auto Tag = MI.getOperand(1);
+    Tag.setTargetFlags(AArch64II::MO_PREL | AArch64II::MO_G3);
+    Tag.setOffset(0x100000000);
----------------
arsenm wrote:
> Assuming this was modifying the original instruction, it needs to notify the change observer. Also you may need to guard against already legalized globals?
w.r.t the change observer, we're looking to add an instruction, not modify the existing. I don't fully understand when the CO needs to be notified, as the other two created instructions in this function (adrp + addlow) don't notify the CO. Can you please help me understand when/whether this is necessary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82615