[AArch64] Mark condition flags as clobbered when calling __chkstk

[mstorsjo]

Summary

This is similar to SVN r311061 for ARM.

[efriedma]

Missing implicit-def of r16/r17?

On a sort-of-related note, I'm confused about the difference between small/large code model; doesn't the AArch64 linker support range-extension thunks? (This is related because range-extension thunks can clobber r16/r17.)

[mstorsjo]

In D53878#1280785, @efriedma wrote:

Missing implicit-def of r16/r17?

For both cases, or just the first one? (The other one has got .addReg(AArch64::X16, RegState::Kill) already. Would that be .addReg(AArch64::X16, RegState::Implicit | RegState::Define) for both X16 and X17 there? I can't say I really understand what all these flags mean.

On a sort-of-related note, I'm confused about the difference between small/large code model; doesn't the AArch64 linker support range-extension thunks? (This is related because range-extension thunks can clobber r16/r17.)

I guess AArch64 COFF linkers in general could support range extension thunks, although I don't know if link.exe does - lld doesn't at least. The __chkstk function itself also clobbers both X16 and X17, so they definitely should be considered clobbered here. (Not that I know what really would assume these registers to have any sensible value here, as this is emitted in the prologue of a function.)

[efriedma]

For both cases; the "blr x16" both uses and clobbers x16, so it needs two operands to represent that. .addReg(AArch64::X16, RegState::Implicit | RegState::Define | RegState::Dead) is correct (just like you used for NZCV).

"RegState::Define" means it's a definition, rather than a use. RegState::Implicit means that it's not one of the explicit inputs or outputs listed in the instruction description. RegState::Dead and RegState::Kill are optimization hints; a dead definition is never used, and a "kill" use is the last use of a definition.

(Not that I know what really would assume these registers to have any sensible value here, as this is emitted in the prologue of a function.)

In theory, if unwind tables are disabled, some code after the prologue could get rescheduled into the prologue.

I guess AArch64 COFF linkers in general could support range extension thunks, although I don't know if link.exe does - lld doesn't at least.

I would be surprised if link.exe doesn't support it, given it supports the equivalent for 32-bit ARM.

[mstorsjo]

In D53878#1281069, @efriedma wrote:

For both cases; the "blr x16" both uses and clobbers x16, so it needs two operands to represent that. .addReg(AArch64::X16, RegState::Implicit | RegState::Define | RegState::Dead) is correct (just like you used for NZCV).

"RegState::Define" means it's a definition, rather than a use. RegState::Implicit means that it's not one of the explicit inputs or outputs listed in the instruction description. RegState::Dead and RegState::Kill are optimization hints; a dead definition is never used, and a "kill" use is the last use of a definition.

Ok, thanks!

(Not that I know what really would assume these registers to have any sensible value here, as this is emitted in the prologue of a function.)

In theory, if unwind tables are disabled, some code after the prologue could get rescheduled into the prologue.

Oh, ok.

[mstorsjo]

Marking x16/x17 as clobbered as well, extended the test to both variants.

[efriedma]

LGTM

lib/Target/AArch64/AArch64FrameLowering.cpp

if (windowsRequiresStackProbe(MF, NumBytes)) {
     switch (MF.getTarget().getCodeModel()) {
     case CodeModel::Tiny:
     case CodeModel::Small:
     case CodeModel::Medium:
     case CodeModel::Kernel:
       BuildMI(MBB, MBBI, DL, TII->get(AArch64::BL))
           .addExternalSymbol("__chkstk")
           .addReg(AArch64::X15, RegState::Implicit)
+          .addReg(AArch64::NZCV, RegState::Implicit | RegState::Define | RegState::Dead)
           .setMIFlags(MachineInstr::FrameSetup);
       break;
     case CodeModel::Large:
       BuildMI(MBB, MBBI, DL, TII->get(AArch64::MOVaddrEXT))
           .addReg(AArch64::X16, RegState::Define)
           .addExternalSymbol("__chkstk")
           .addExternalSymbol("__chkstk")
           .setMIFlags(MachineInstr::FrameSetup);
 
       BuildMI(MBB, MBBI, DL, TII->get(AArch64::BLR))
           .addReg(AArch64::X16, RegState::Kill)
           .addReg(AArch64::X15, RegState::Implicit | RegState::Define)
+          .addReg(AArch64::NZCV, RegState::Implicit | RegState::Define | RegState::Dead)
           .setMIFlags(MachineInstr::FrameSetup);
       break;
     }
 
     BuildMI(MBB, MBBI, DL, TII->get(AArch64::SUBXrx64), AArch64::SP)
         .addReg(AArch64::SP, RegState::Kill)
         .addReg(AArch64::X15, RegState::Kill)
         .addImm(AArch64_AM::getArithExtendImm(AArch64_AM::UXTX, 4))

test/CodeGen/AArch64/chkstk.ll

 ; RUN: llc -mtriple=aarch64-windows -verify-machineinstrs %s -o - \
 ; RUN:  | FileCheck -check-prefix CHECK-DEFAULT-CODE-MODEL %s
+; RUN: llc -mtriple=aarch64-windows -print-machineinstrs=prologepilog %s -o - 2>&1 \
+; RUN:  | FileCheck -check-prefix CHECK-REGSTATE %s
 
 ; RUN: llc -mtriple=aarch64-windows -verify-machineinstrs -code-model=large %s -o - \
 ; RUN:  | FileCheck -check-prefix CHECK-LARGE-CODE-MODEL %s
 
 define void @check_watermark() {
 entry:
   %buffer = alloca [4096 x i8], align 1
   ret void
 }
 
 ; CHECK-DEFAULT-CODE-MODEL: check_watermark:
 ; CHECK-DEFAULT-CODE-MODEL-DAG: stp x29, x30, [sp
 ; CHECK-DEFAULT-CODE-MODEL-DAG: orr x15, xzr, #0x100
 ; CHECK-DEFAULT-CODE-MODEL:     bl __chkstk
 ; CHECK-DEFAULT-CODE-MODEL:     sub sp, sp, x15, lsl #4
 
 ; CHECK-LARGE-CODE-MODEL: check_watermark:
 ; CHECK-LARGE-CODE-MODEL-DAG: stp x29, x30, [sp
 ; CHECK-LARGE-CODE-MODEL-DAG: orr x15, xzr, #0x100
 ; CHECK-LARGE-CODE-MODEL-DAG: adrp x16, __chkstk
 ; CHECK-LARGE-CODE-MODEL-DAG: add x16, x16, __chkstk
 ; CHECK-LARGE-CODE-MODEL:     blr x16
 ; CHECK-LARGE-CODE-MODEL:     sub sp, sp, x15, lsl #4
+
+; CHECK-REGSTATE: frame-setup BL &__chkstk, implicit-def $lr, implicit $sp, implicit $x15, implicit-def dead $nzcv