Page MenuHomePhabricator

Basic codegen for MTE stack tagging.
ClosedPublic

Authored by eugenis on Jul 3 2019, 4:42 PM.

Details

Summary

Implement IR intrinsics for stack tagging. Generated code is very
unoptimized for now.

Two special intrinsics, llvm.aarch64.irg.sp and llvm.aarch64.tagp are
used to implement a tagged stack frame pointer in a virtual register.

Diff Detail

Repository
rL LLVM

Event Timeline

eugenis created this revision.Jul 3 2019, 4:42 PM

I think this could do with some more tests covering different stack layouts. In particular, we should check that we do the correct thing for functions which use frame or base pointers, and when large stack frames cause the immediates in STG and ADDG instructions go out of range.

llvm/include/llvm/CodeGen/SelectionDAGTargetInfo.h
151 ↗(On Diff #207922)

Could Op1 and Op2 be given more descriptive names? It looks like they are always the pointer and length of the memory region.

llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp
368 ↗(On Diff #207922)

Could we make the size a register input to these pseudo-instructions, so that this gets done by normal code-generation?

eugenis marked 2 inline comments as done.Jul 8 2019, 2:51 PM
eugenis added inline comments.
llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp
368 ↗(On Diff #207922)

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)

eugenis updated this revision to Diff 208517.Jul 8 2019, 3:10 PM
eugenis marked an inline comment as not done.

address review comments & remove @earlyclobber from STGPreIndex / STGPostIndex

eugenis marked an inline comment as done.Jul 8 2019, 3:11 PM

What about the tests for large stack frames?

llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp
368 ↗(On Diff #207922)

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.

llvm/lib/Target/AArch64/AArch64StackTagging.cpp
1 ↗(On Diff #208517)

It looks like you accidentally merged D64173 into this patch,

eugenis updated this revision to Diff 209393.Jul 11 2019, 6:21 PM

Made aarch64.irg.sp IntrInaccessibleMemOnly cause it has side effects.
Simplified SDAG a little by matching intrinsic directly w/o going through an SDag node for IRGstack.
Added a bunch of tests.

eugenis marked an inline comment as done.Jul 11 2019, 6:24 PM

PTAL. I've added tests for various frame layouts. Stg offset overflow is covered by existing tests (settag.ll).

llvm/include/llvm/IR/IntrinsicsAArch64.td
708 ↗(On Diff #209393)

Ideally this should be [IntrNoMem, IntrHasSideEffects],
but it depends on the discussion in https://reviews.llvm.org/D64414.
Keeping this the same as int_aarch64_irg for now.

ostannard accepted this revision.Jul 16 2019, 1:59 AM

LGTM, thanks

This revision is now accepted and ready to land.Jul 16 2019, 1:59 AM
This revision was automatically updated to reflect the committed changes.