[SystemZ] Recognize mrecord-mcount in backend

[jonpa]

Summary

Emit the mcount_loc_ section with all fentry__calls.

This is done using just raw text like gcc does it (see gcc@06477d3). Is this acceptable?

A check in SystemZDAGToDAGISel::runOnMachineFunction() just like for mnop-mcount to make sure this is only used together with -mfentry.

[uweigand]

This is done using just raw text like gcc does it (see gcc@06477d3). Is this acceptable?

Well, that doesn't really work, since it means the -mrecord-mcount is then simply ignored when using the built-in assembler (i.e. compiling to object code instead of assembler source).

[jonpa]

In D71629#1788666, @uweigand wrote:

This is done using just raw text like gcc does it (see gcc@06477d3). Is this acceptable?

Well, that doesn't really work, since it means the -mrecord-mcount is then simply ignored when using the built-in assembler (i.e. compiling to object code instead of assembler source).

Ah, of course...

[jonpa]

Patch updated to call EmitLabel, SwitchSection() and EmitIntValue() instead of emitting raw text.

I don't know why gcc is emitting a "1:" label... Is it OK to instead emit .Ltmp0:, .Ltmp1: labels (any name) for each fentry?

Not exactly sure exactly what type of section should be created, but calling getELFSection() this way gives the right output, it seems.

Switching back to the previous section causes a ".text" to be printed instead of ".previous".

See mrecord-mcount-01.ll for the output...

[uweigand]

llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp

568

For consistency, we should really remove the "true" from the "mnop-mcount" attribute as well. (That should be a separate patch.)

577

There's no reason to split the code up this way. You might as well do everything in the block above.

580

This is wrong, you need to emit a reference to the "DotSym" above.

I guess GCC uses the GAS unnamed label feature "1b" -- this is not the hex value 0x1b, but a backward reference to the immediately preceding instance of the "1:" label.

llvm/test/CodeGen/SystemZ/mrecord-mcount-01.ll

13

So instead of ".quad 27" this should read ".quad .Ltmp0".

[uweigand]

llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp

581

It's probably better to use a OutStreamer->PushSection() / OutStreamer->PopSection() pair here.

[jonpa]

Patch updated per review.

Beginning to understand how this is supposed to work now, but still not sure why the text assembly looks ok, while I see with objdump --filetype=obj the new section filled with zeros:

llc llvm/test/CodeGen/SystemZ/mrecord-mcount-01.ll -mtriple=s390x-linux-gnu -mcpu=z10 -o out.o --filetype=obj; objdump -D -z out.o

0000000000000000 <__mcount_loc>:
0:   00 00 00 00             .long   0x00000000
4:   00 00 00 00             .long   0x00000000
8:   00 00 00 00             .long   0x00000000
c:   00 00 00 00             .long   0x00000000

I was expecting to see something like <test1> and <test2> somwhere there... Will the linker fix this somehow (not sure how it could since it's just zeroes...)?

[jonpa]

llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp

577

Ah, ok.

580

Ah, of course...

[uweigand]

In D71629#1789965, @jonpa wrote:

Patch updated per review.

Beginning to understand how this is supposed to work now, but still not sure why the text assembly looks ok, while I see with objdump --filetype=obj the new section filled with zeros:

llc llvm/test/CodeGen/SystemZ/mrecord-mcount-01.ll -mtriple=s390x-linux-gnu -mcpu=z10 -o out.o --filetype=obj; objdump -D -z out.o

0000000000000000 <__mcount_loc>:
0:   00 00 00 00             .long   0x00000000
4:   00 00 00 00             .long   0x00000000
8:   00 00 00 00             .long   0x00000000
c:   00 00 00 00             .long   0x00000000

I was expecting to see something like <test1> and <test2> somwhere there... Will the linker fix this somehow (not sure how it could since it's just zeroes...)?

Yes, this will be done via relocations. You should add -r to the objdump command line, then you'll see them :-)

[uweigand]

Minor cosmetic comment, otherwise this LGTM.

llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp

560

I think it would be nicer to have the EmitLabel after the PopSection, so that the label is optically close to the instruction it labels. But that's just cosmetics.

[jonpa]

Updated per review: EmitLabel after PopSection.

SystemZISelDAGToDAG::runOnMachineFunction() refactored to combine the two checks for fentry-call.

[uweigand]

LGTM with minor tweak. Thanks!

llvm/test/CodeGen/SystemZ/mrecord-mcount-01.ll

32

I believe "mop-mcount" no longer needs "true" here now.

[jonpa]

6be1578

llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "SystemZAsmPrinter.h"
 #include "MCTargetDesc/SystemZInstPrinter.h"
 #include "SystemZConstantPoolValue.h"
 #include "SystemZMCInstLower.h"
 #include "TargetInfo/SystemZTargetInfo.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/CodeGen/MachineModuleInfoImpls.h"
 #include "llvm/CodeGen/TargetLoweringObjectFileImpl.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInstBuilder.h"
+#include "llvm/MC/MCSectionELF.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/Support/TargetRegistry.h"
 
 using namespace llvm;
 
 // Return an RI instruction like MI with opcode Opcode, but with the
 // GR64 register operands turned into GR32s.
 static MCInst lowerRILow(const MachineInstr *MI, unsigned Opcode) {
else {
     OutStreamer.EmitLabel(DotSym);
     return 6;
   }
 }
 
 void SystemZAsmPrinter::LowerFENTRY_CALL(const MachineInstr &MI,
                                          SystemZMCInstLower &Lower) {
   MCContext &Ctx = MF->getContext();
+  if (MF->getFunction().hasFnAttribute("mrecord-mcount")) {
+    MCSymbol *DotSym = OutContext.createTempSymbol();
+    OutStreamer->PushSection();
+    OutStreamer->SwitchSection(
+        Ctx.getELFSection("__mcount_loc", ELF::SHT_PROGBITS, ELF::SHF_ALLOC));
+    OutStreamer->EmitSymbolValue(DotSym, 8);
+    OutStreamer->PopSection();
+    OutStreamer->EmitLabel(DotSym);
+  }
+
   if (MF->getFunction().hasFnAttribute("mnop-mcount")) {
     EmitNop(Ctx, *OutStreamer, 6, getSubtargetInfo());
     return;
   }
 
   MCSymbol *fentry = Ctx.getOrCreateSymbol("__fentry__");
   const MCSymbolRefExpr *Op =
       MCSymbolRefExpr::create(fentry, MCSymbolRefExpr::VK_PLT, Ctx);
   OutStreamer->EmitInstruction(MCInstBuilder(SystemZ::BRASL)
                        .addReg(SystemZ::R0D).addExpr(Op), getSubtargetInfo());
 }
 
 void SystemZAsmPrinter::LowerSTACKMAP(const MachineInstr &MI) {
   const SystemZInstrInfo *TII =
     static_cast<const SystemZInstrInfo *>(MF->getSubtarget().getInstrInfo());
 
   unsigned NumNOPBytes = MI.getOperand(1).getImm();
 
   SM.recordStackMap(MI);
   assert(NumNOPBytes % 2 == 0 && "Invalid number of NOP bytes requested!");
 
   // Scan ahead to trim the shadow.

llvm/lib/Target/SystemZ/SystemZISelDAGToDAG.cpp

class SystemZDAGToDAGISel : public SelectionDAGISel {
   SDValue expandSelectBoolean(SDNode *Node);
 
 public:
   SystemZDAGToDAGISel(SystemZTargetMachine &TM, CodeGenOpt::Level OptLevel)
       : SelectionDAGISel(TM, OptLevel) {}
 
   bool runOnMachineFunction(MachineFunction &MF) override {
     const Function &F = MF.getFunction();
-    if (F.hasFnAttribute("mnop-mcount") &&
-        F.getFnAttribute("fentry-call").getValueAsString() != "true")
+    if (F.getFnAttribute("fentry-call").getValueAsString() != "true") {
+      if (F.hasFnAttribute("mnop-mcount"))
         report_fatal_error("mnop-mcount only supported with fentry-call");
+      if (F.hasFnAttribute("mrecord-mcount"))
+        report_fatal_error("mrecord-mcount only supported with fentry-call");
+    }
 
     Subtarget = &MF.getSubtarget<SystemZSubtarget>();
     return SelectionDAGISel::runOnMachineFunction(MF);
   }
 
   // Override MachineFunctionPass.
   StringRef getPassName() const override {
     return "SystemZ DAG->DAG Pattern Instruction Selection";

llvm/test/CodeGen/SystemZ/mrecord-mcount-01.ll

+; RUN: llc %s -mtriple=s390x-linux-gnu -mcpu=z10 -o - -verify-machineinstrs \
+; RUN:   | FileCheck %s
+
+define void @test1() #0 {
+entry:
+  ret void
+
+; CHECK-LABEL: test1:
+; CHECK: .section __mcount_loc,"a",@progbits
+; CHECK: .quad .Ltmp0
+; CHECK: .text
+; CHECK: .Ltmp0:
+; CHECK: brasl %r0, __fentry__@PLT
+; CHECK: br %r14
+}
+
+define void @test2() #1 {
+entry:
+  ret void
+
+; CHECK-LABEL: test2:
+; CHECK: .section __mcount_loc,"a",@progbits
+; CHECK: .quad .Ltmp1
+; CHECK: .text
+; CHECK: .Ltmp1:
+; CHECK: brcl 0, .Ltmp2
+; CHECK: .Ltmp2:
+; CHECK: br %r14
+}
+
+attributes #0 = { "fentry-call"="true" "mrecord-mcount" }
+attributes #1 = { "fentry-call"="true" "mnop-mcount"="true" "mrecord-mcount" }

llvm/test/CodeGen/SystemZ/mrecord-mcount-02.ll

+; RUN: not llc %s -mtriple=s390x-linux-gnu -o - 2>&1 | FileCheck %s
+;
+; CHECK: LLVM ERROR: mrecord-mcount only supported with fentry-call
+
+define void @test1() #0 {
+entry:
+  ret void
+}
+
+attributes #0 = { "instrument-function-entry-inlined"="mcount" "mrecord-mcount" }