[PATCH] D64172: Basic codegen for MTE stack tagging.

[Oliver Stannard (Linaro) via Phabricator via llvm-commits]

ostannard added a comment.

What about the tests for large stack frames?



================
Comment at: llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp:368
+  }
+  MachineBasicBlock::iterator I =
+      BuildMI(MBB, MBBI, DL, TII->get(AArch64::MOVi64imm), SizeReg)
----------------
eugenis wrote:
> ostannard wrote:
> > Could we make the size a register input to these pseudo-instructions, so that this gets done by normal code-generation?
> That would require moving the previous conditional block (Size % (16 * 2) != 0) to SDAG because this pass will no longer now if the size if 32-byte aligned or not. If I do that, I run into a different problem: regalloc refuses to generate
> 
> ```
> STG Xn, [Xn], #offset
> ```
> 
> even if Xn is dead after this, because STGPostIndex has @earlyclobber on the writeback register.
> 
> Is this correct? I don't see anything in the spec where the same register can not be used for both, and the pseudo-code suggests that registers are read first and updated later.
> 
> This adds an extra register copy in all of the settag.ll test cases, ex.
> 
> ```
> 	mov	x8, x0
> 	stg	x0, [x8], #16
> 	mov	w9, #256
> .LBB0_1:                                // %entry
>                                         // =>This Inner Loop Header: Depth=1
> 	st2g	x8, [x8], #32
> 	sub	x9, x9, #32             // =32
> 	cbnz	x9, .LBB0_1
> ```
> 
> (the second st2g is emitted in expand-pseudos)
> 
I think earlier versions of the architecture did not allow Xt and Xn to be the same in STG with writeback, but that restriction has been removed, so removing the `@earlyclobber` is correct.


================
Comment at: llvm/lib/Target/AArch64/AArch64StackTagging.cpp:1
+//===- AArch64StackTagging.cpp - Stack tagging in IR --===//
+//
----------------
It looks like you accidentally merged D64173 into this patch,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64172