[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:
> hctim wrote:
> > 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?
> You're copying the operand, and then you don't do anything with it after modifying it; you don't add Tag to the new instruction. In any case, having a freestanding MachineOperand value is a bad idea.
> 
> You're also creating a new generic virtual register for the selected instruction, so it needs a register class. You should create one with the final register class (I'm guessing AArch64::GPR64RegClass), and then set the type with MRI.setType (the above code does the same thing in the opposite order, by creating the vreg with a type and setting the class later)
Ah, yes - it's vestigal and can be removed, thanks!

I've also classed the vreg, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82615