[SystemZ][XRay] Implement XRay instrumentation for SystemZ

[redstar]

Expands pseudo instructions PATCHABLE_FUNCTION_ENTER and PATCHABLE_RET into a small instruction sequence which calls into the XRay library.

[llvmbot]

@llvm/pr-subscribers-backend-systemz

Author: Kai Nacke (redstar)

Changes

Expands pseudo instructions PATCHABLE_FUNCTION_ENTER and PATCHABLE_RET into a small instruction sequence which calls into the XRay library.

---

Full diff: https://github.com/llvm/llvm-project/pull/113253.diff

6 Files Affected:

• (modified) llvm/lib/CodeGen/XRayInstrumentation.cpp (+2-1)
• (modified) llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp (+88)
• (modified) llvm/lib/Target/SystemZ/SystemZAsmPrinter.h (+12)
• (modified) llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp (+6)
• (modified) llvm/lib/Target/SystemZ/SystemZSubtarget.h (+2)
• (added) llvm/test/CodeGen/SystemZ/xray-01.ll (+39)

diff --git a/llvm/lib/CodeGen/XRayInstrumentation.cpp b/llvm/lib/CodeGen/XRayInstrumentation.cpp index d7cc5d5c2b41dd..8f718d884cd067 100644 --- a/llvm/lib/CodeGen/XRayInstrumentation.cpp +++ b/llvm/lib/CodeGen/XRayInstrumentation.cpp @@ -241,7 +241,8 @@ bool XRayInstrumentation::runOnMachineFunction(MachineFunction &MF) { prependRetWithPatchableExit(MF, TII, op); break; } - case Triple::ArchType::ppc64le: { + case Triple::ArchType::ppc64le: + case Triple::ArchType::systemz: { // PPC has conditional returns. Turn them into branch and plain returns. InstrumentationOptions op; op.HandleTailcall = false; diff --git a/llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp b/llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp index ed400e9eceb9ca..dbb21d30edaf0c 100644 --- a/llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp +++ b/llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp @@ -14,6 +14,7 @@ #include "SystemZAsmPrinter.h" #include "MCTargetDesc/SystemZInstPrinter.h" #include "MCTargetDesc/SystemZMCExpr.h" +#include "MCTargetDesc/SystemZMCTargetDesc.h" #include "SystemZConstantPoolValue.h" #include "SystemZMCInstLower.h" #include "TargetInfo/SystemZTargetInfo.h" @@ -661,6 +662,23 @@ void SystemZAsmPrinter::emitInstruction(const MachineInstr *MI) { LowerPATCHPOINT(*MI, Lower); return; + case TargetOpcode::PATCHABLE_FUNCTION_ENTER: + LowerPATCHABLE_FUNCTION_ENTER(*MI, Lower); + return ; + + case TargetOpcode::PATCHABLE_RET: + LowerPATCHABLE_RET(*MI, Lower); + return ; + + case TargetOpcode::PATCHABLE_FUNCTION_EXIT: + llvm_unreachable("PATCHABLE_FUNCTION_EXIT should never be emitted"); + + case TargetOpcode::PATCHABLE_TAIL_CALL: + // TODO: Define a trampoline `__xray_FunctionTailExit` and differentiate a + // normal function exit from a tail exit. + llvm_unreachable("Tail call is handled in the normal case. See comments " + "around this assert."); + case SystemZ::EXRL_Pseudo: { unsigned TargetInsOpc = MI->getOperand(0).getImm(); Register LenMinus1Reg = MI->getOperand(1).getReg(); @@ -843,6 +861,76 @@ void SystemZAsmPrinter::LowerPATCHPOINT(const MachineInstr &MI, getSubtargetInfo()); } +void SystemZAsmPrinter::LowerPATCHABLE_FUNCTION_ENTER( + const MachineInstr &MI, SystemZMCInstLower &Lower) { + // .begin: + // j .end # -> stmg %r2, %r15, 16(%r15) + // nop + // llilf %2, FuncID + // brasl %r14, __xray_FunctionEntry@GOT + // .end: + // + // Update compiler-rt/lib/xray/xray_s390x.cpp accordingly when number + // of instructions change. + MCSymbol *BeginOfSled = OutContext.createTempSymbol("xray_sled_", true); + MCSymbol *EndOfSled = OutContext.createTempSymbol(); + OutStreamer->emitLabel(BeginOfSled); + EmitToStreamer(*OutStreamer, + MCInstBuilder(SystemZ::J) + .addExpr(MCSymbolRefExpr::create(EndOfSled, OutContext))); + EmitNop(OutContext, *OutStreamer, 2, getSubtargetInfo()); + EmitToStreamer(*OutStreamer, + MCInstBuilder(SystemZ::LLILF).addReg(SystemZ::R2D).addImm(0)); + EmitToStreamer(*OutStreamer, + MCInstBuilder(SystemZ::BRASL) + .addReg(SystemZ::R14D) + .addExpr(MCSymbolRefExpr::create( + OutContext.getOrCreateSymbol("__xray_FunctionEntry"), + MCSymbolRefExpr::VK_PLT, OutContext))); + OutStreamer->emitLabel(EndOfSled); + recordSled(BeginOfSled, MI, SledKind::FUNCTION_ENTER, 2); +} + +void SystemZAsmPrinter::LowerPATCHABLE_RET(const MachineInstr &MI, + SystemZMCInstLower &Lower) { + unsigned OpCode = MI.getOperand(0).getImm(); + MCSymbol *FallthroughLabel = nullptr; + if (OpCode == SystemZ::CondReturn) { + FallthroughLabel = OutContext.createTempSymbol(); + int64_t Cond0 = MI.getOperand(1).getImm(); + int64_t Cond1 = MI.getOperand(2).getImm(); + EmitToStreamer(*OutStreamer, MCInstBuilder(SystemZ::BRC) + .addImm(Cond0) + .addImm(Cond1 ^ Cond0) + .addExpr(MCSymbolRefExpr::create( + FallthroughLabel, OutContext))); + } + // .begin: + // br %r14 # -> stmg %r2, %r15, 24(%r15) + // nop + // nop + // llilf %2,FuncID + // j __xray_FunctionEntry@GOT + // + // Update compiler-rt/lib/xray/xray_s390x.cpp accordingly when number + // of instructions change. + MCSymbol *BeginOfSled = OutContext.createTempSymbol("xray_sled_", true); + OutStreamer->emitLabel(BeginOfSled); + EmitToStreamer(*OutStreamer, + MCInstBuilder(SystemZ::BR).addReg(SystemZ::R14D)); + EmitNop(OutContext, *OutStreamer, 4, getSubtargetInfo()); + EmitToStreamer(*OutStreamer, + MCInstBuilder(SystemZ::LLILF).addReg(SystemZ::R2D).addImm(0)); + EmitToStreamer(*OutStreamer, + MCInstBuilder(SystemZ::J) + .addExpr(MCSymbolRefExpr::create( + OutContext.getOrCreateSymbol("__xray_FunctionExit"), + MCSymbolRefExpr::VK_PLT, OutContext))); + if (FallthroughLabel) + OutStreamer->emitLabel(FallthroughLabel); + recordSled(BeginOfSled, MI, SledKind::FUNCTION_EXIT, 2); +} + // The *alignment* of 128-bit vector types is different between the software // and hardware vector ABIs. If the there is an externally visible use of a // vector type in the module it should be annotated with an attribute. diff --git a/llvm/lib/Target/SystemZ/SystemZAsmPrinter.h b/llvm/lib/Target/SystemZ/SystemZAsmPrinter.h index 303cce1a1b6581..2696702b44551d 100644 --- a/llvm/lib/Target/SystemZ/SystemZAsmPrinter.h +++ b/llvm/lib/Target/SystemZ/SystemZAsmPrinter.h @@ -111,6 +111,15 @@ class LLVM_LIBRARY_VISIBILITY SystemZAsmPrinter : public AsmPrinter { bool PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo, const char *ExtraCode, raw_ostream &OS) override; + bool runOnMachineFunction(MachineFunction &MF) override { + AsmPrinter::runOnMachineFunction(MF); + + // Emit the XRay table for this function. + emitXRayTable(); + + return false; + } + bool doInitialization(Module &M) override { SM.reset(); return AsmPrinter::doInitialization(M); @@ -124,6 +133,9 @@ class LLVM_LIBRARY_VISIBILITY SystemZAsmPrinter : public AsmPrinter { void LowerFENTRY_CALL(const MachineInstr &MI, SystemZMCInstLower &MCIL); void LowerSTACKMAP(const MachineInstr &MI); void LowerPATCHPOINT(const MachineInstr &MI, SystemZMCInstLower &Lower); + void LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr &MI, + SystemZMCInstLower &Lower); + void LowerPATCHABLE_RET(const MachineInstr &MI, SystemZMCInstLower &Lower); void emitAttributes(Module &M); }; } // end namespace llvm diff --git a/llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp b/llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp index 91db858f5cdaf6..d3de2c9cda7269 100644 --- a/llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp +++ b/llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp @@ -30,6 +30,7 @@ #include "llvm/CodeGen/SlotIndexes.h" #include "llvm/CodeGen/StackMaps.h" #include "llvm/CodeGen/TargetInstrInfo.h" +#include "llvm/CodeGen/TargetOpcodes.h" #include "llvm/CodeGen/TargetSubtargetInfo.h" #include "llvm/CodeGen/VirtRegMap.h" #include "llvm/MC/MCInstrDesc.h" @@ -1792,6 +1793,11 @@ unsigned SystemZInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const { return MI.getOperand(1).getImm(); else if (MI.getOpcode() == SystemZ::FENTRY_CALL) return 6; + if (MI.getOpcode() == TargetOpcode::PATCHABLE_FUNCTION_ENTER) + return 18; + if (MI.getOpcode() == TargetOpcode::PATCHABLE_RET) + return 18 + + (MI.getOperand(0).getImm() == SystemZ::CondReturn ? 4 : 0); return MI.getDesc().getSize(); } diff --git a/llvm/lib/Target/SystemZ/SystemZSubtarget.h b/llvm/lib/Target/SystemZ/SystemZSubtarget.h index 5fa7c8f194ebf3..761bc525b59d35 100644 --- a/llvm/lib/Target/SystemZ/SystemZSubtarget.h +++ b/llvm/lib/Target/SystemZ/SystemZSubtarget.h @@ -106,6 +106,8 @@ class SystemZSubtarget : public SystemZGenSubtargetInfo { bool GETTER() const { return ATTRIBUTE; } #include "SystemZGenSubtargetInfo.inc" + bool isXRaySupported() const override { return true; } + bool isAddressedViaADA(const GlobalValue *GV) const; // Return true if GV can be accessed using LARL for reloc model RM diff --git a/llvm/test/CodeGen/SystemZ/xray-01.ll b/llvm/test/CodeGen/SystemZ/xray-01.ll new file mode 100644 index 00000000000000..7a364df2066763 --- /dev/null +++ b/llvm/test/CodeGen/SystemZ/xray-01.ll @@ -0,0 +1,39 @@ +; RUN: llc < %s -mtriple=s390x-linux-gnu | FileCheck %s + +define signext i32 @foo() "function-instrument"="xray-always" { +; CHECK-LABEL: .Lxray_sled_0: +; CHECK: j	.Ltmp[[#l:]] +; CHECK: bcr	0, %r0 +; CHECK: llilf	%r2, 0 +; CHECK: brasl	%r14, __xray_FunctionEntry@PLT +; CHECK: .Ltmp[[#l]]: + ret i32 0 +; CHECK-LABEL: .Lxray_sled_1: +; CHECK: br	%r14 +; CHECK: bc	0, 0 +; CHECK: llilf	%r2, 0 +; CHECK: j	__xray_FunctionExit@PLT +} + +; CHECK:	.section	xray_instr_map,"ao",@progbits,foo +; CHECK: .Lxray_sleds_start0: +; CHECK: [[TMP1:.Ltmp[0-9]+]]: +; CHECK:	.quad	.Lxray_sled_0-[[TMP1]] +; CHECK:	.quad	.Lfunc_begin0-([[TMP1]]+8) +; CHECK:	.byte	0x00 +; CHECK:	.byte	0x01 +; CHECK:	.byte	0x02 +; CHECK: .space	13 +; CHECK: [[TMP2:.Ltmp[0-9]+]]: +; CHECK:	.quad	.Lxray_sled_1-[[TMP2]] +; CHECK:	.quad	.Lfunc_begin0-([[TMP2]]+8) +; CHECK:	.byte	0x01 +; CHECK:	.byte	0x01 +; CHECK:	.byte	0x02 +; CHECK:	.space	13 +; CHECK: .Lxray_sleds_end0: +; CHECK:	.section	xray_fn_idx,"ao",@progbits,foo +; CHECK:	.p2align	4 +; CHECK: .Lxray_fn_idx0: +; CHECK:	.quad	.Lxray_sleds_start0-.Lxray_fn_idx0 +; CHECK:	.quad	2 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[MaskRay]

Perhaps just xray.ll

When we need other tests, they are likely suffixed, e.g. xray-custom-log.ll

[redstar]

@uweigand Just to make sure:

 bool HasVectorFeature = TM.getMCSubtargetInfo()->hasFeature(SystemZ::FeatureVector); 

is the correct way to determine if the user compiles with the vector extension enabled, isn't it?

[uweigand]

@uweigand Just to make sure:

 bool HasVectorFeature = TM.getMCSubtargetInfo()->hasFeature(SystemZ::FeatureVector); 

is the correct way to determine if the user compiles with the vector extension enabled, isn't it?

Yes, this should be correct. I'm now wondering whether this handles -msoft-float correctly - for the main Subtarget, we disable the vector feature if soft-float is used, but I'm not sure if that also happens for the MCSubtarget. Might be interesting to verify.

[redstar]

Yes, this should be correct. I'm now wondering whether this handles -msoft-float correctly - for the main Subtarget, we disable the vector feature if soft-float is used, but I'm not sure if that also happens for the MCSubtarget. Might be interesting to verify.

That was a good point. It turns out that I had to check the soft-float feature in addition to the vector feature to get the correct result. I updated the test accordingly. Can you have another look, please?

[uweigand]

LGTM