[llvm] r217801 - Fix a lot of confusion around inserting nops on empty functions.

Rafael Espindola rafael.espindola at gmail.com
Mon Sep 15 11:32:58 PDT 2014


Author: rafael
Date: Mon Sep 15 13:32:58 2014
New Revision: 217801

URL: http://llvm.org/viewvc/llvm-project?rev=217801&view=rev
Log:
Fix a lot of confusion around inserting nops on empty functions.

On MachO, and MachO only, we cannot have a truly empty function since that
breaks the linker logic for atomizing the section.

When we are emitting a frame pointer, the presence of an unreachable will
create a cfi instruction pointing past the last instruction. This is perfectly
fine. The FDE information encodes the pc range it applies to. If some tool
cannot handle this, we should explicitly say which bug we are working around
and only work around it when it is actually relevant (not for ELF for example).

Given the unreachable we could omit the .cfi_def_cfa_register, but then
again, we could also omit the entire function prologue if we wanted to.

Removed:
    llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll
    llvm/trunk/test/CodeGen/SPARC/empty-functions.ll
Modified:
    llvm/trunk/include/llvm/Target/TargetInstrInfo.h
    llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
    llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp
    llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp
    llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h
    llvm/trunk/test/CodeGen/X86/empty-functions.ll

Modified: llvm/trunk/include/llvm/Target/TargetInstrInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetInstrInfo.h?rev=217801&r1=217800&r2=217801&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Target/TargetInstrInfo.h (original)
+++ llvm/trunk/include/llvm/Target/TargetInstrInfo.h Mon Sep 15 13:32:58 2014
@@ -847,10 +847,8 @@ public:
                           MachineBasicBlock::iterator MI) const;
 
 
-  /// getNoopForMachoTarget - Return the noop instruction to use for a noop.
-  virtual void getNoopForMachoTarget(MCInst &NopInst) const {
-    // Default to just using 'nop' string.
-  }
+  /// Return the noop instruction to use for a noop.
+  virtual void getNoopForMachoTarget(MCInst &NopInst) const;
 
 
   /// isPredicated - Returns true if the instruction is already predicated.

Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp?rev=217801&r1=217800&r2=217801&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (original)
+++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp Mon Sep 15 13:32:58 2014
@@ -731,12 +731,10 @@ void AsmPrinter::EmitFunctionBody() {
 
   // Print out code for the function.
   bool HasAnyRealCode = false;
-  const MachineInstr *LastMI = nullptr;
   for (auto &MBB : *MF) {
     // Print a label for the basic block.
     EmitBasicBlockStart(MBB);
     for (auto &MI : MBB) {
-      LastMI = &MI;
 
       // Print the assembly for the instruction.
       if (!MI.isPosition() && !MI.isImplicitDef() && !MI.isKill() &&
@@ -797,24 +795,14 @@ void AsmPrinter::EmitFunctionBody() {
     EmitBasicBlockEnd(MBB);
   }
 
-  // If the last instruction was a prolog label, then we have a situation where
-  // we emitted a prolog but no function body. This results in the ending prolog
-  // label equaling the end of function label and an invalid "row" in the
-  // FDE. We need to emit a noop in this situation so that the FDE's rows are
-  // valid.
-  bool RequiresNoop = LastMI && LastMI->isCFIInstruction();
-
   // If the function is empty and the object file uses .subsections_via_symbols,
   // then we need to emit *something* to the function body to prevent the
   // labels from collapsing together.  Just emit a noop.
-  if ((MAI->hasSubsectionsViaSymbols() && !HasAnyRealCode) || RequiresNoop) {
+  if ((MAI->hasSubsectionsViaSymbols() && !HasAnyRealCode)) {
     MCInst Noop;
     TM.getSubtargetImpl()->getInstrInfo()->getNoopForMachoTarget(Noop);
-    if (Noop.getOpcode()) {
-      OutStreamer.AddComment("avoids zero-length function");
-      OutStreamer.EmitInstruction(Noop, getSubtargetInfo());
-    } else  // Target not mc-ized yet.
-      OutStreamer.EmitRawText(StringRef("\tnop\n"));
+    OutStreamer.AddComment("avoids zero-length function");
+    OutStreamer.EmitInstruction(Noop, getSubtargetInfo());
   }
 
   const Function *F = MF->getFunction();

Modified: llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp?rev=217801&r1=217800&r2=217801&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp (original)
+++ llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp Mon Sep 15 13:32:58 2014
@@ -372,6 +372,10 @@ static const TargetRegisterClass *canFol
   return nullptr;
 }
 
+void TargetInstrInfo::getNoopForMachoTarget(MCInst &NopInst) const {
+  llvm_unreachable("Not a MachO target");
+}
+
 bool TargetInstrInfo::
 canFoldMemoryOperand(const MachineInstr *MI,
                      const SmallVectorImpl<unsigned> &Ops) const {

Modified: llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp?rev=217801&r1=217800&r2=217801&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp (original)
+++ llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp Mon Sep 15 13:32:58 2014
@@ -37,11 +37,6 @@ SparcInstrInfo::SparcInstrInfo(SparcSubt
     RI(ST), Subtarget(ST) {
 }
 
-/// getNoopForMachoTarget - Return the noop instruction to use for a noop.
-void SparcInstrInfo::getNoopForMachoTarget(MCInst &NopInst) const {
-  NopInst.setOpcode(SP::NOP);
-}
-
 /// isLoadFromStackSlot - If the specified machine instruction is a direct
 /// load from a stack slot, return the virtual or physical register number of
 /// the destination along with the FrameIndex of the loaded stack slot.  If

Modified: llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h?rev=217801&r1=217800&r2=217801&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h (original)
+++ llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h Mon Sep 15 13:32:58 2014
@@ -93,8 +93,6 @@ public:
                             const TargetRegisterInfo *TRI) const override;
 
   unsigned getGlobalBaseReg(MachineFunction *MF) const;
-
-  void getNoopForMachoTarget(MCInst &NopInst) const override;
 };
 
 }

Removed: llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll?rev=217800&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll (original)
+++ llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll (removed)
@@ -1,13 +0,0 @@
-; RUN: llc < %s -mtriple=powerpc-apple-darwin | FileCheck -check-prefix=CHECK-NO-FP %s
-; RUN: llc < %s -mtriple=powerpc-apple-darwin -disable-fp-elim | FileCheck -check-prefix=CHECK-FP %s
-; RUN: llc < %s -mtriple=powerpc-netbsd -disable-fp-elim | FileCheck -check-prefix=CHECK-FP %s
-
-define void @func() {
-entry:
-  unreachable
-}
-; CHECK-NO-FP:     _func:
-; CHECK-NO-FP:     nop
-
-; CHECK-FP:      {{_?}}func:
-; CHECK-FP: nop {{[;#]}} avoids zero-length function

Removed: llvm/trunk/test/CodeGen/SPARC/empty-functions.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/SPARC/empty-functions.ll?rev=217800&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/SPARC/empty-functions.ll (original)
+++ llvm/trunk/test/CodeGen/SPARC/empty-functions.ll (removed)
@@ -1,8 +0,0 @@
-; RUN: llc < %s -mtriple=sparc-unknown-openbsd -disable-fp-elim | FileCheck -check-prefix=CHECK-FP-LABEL %s
-
-define void @func() {
-entry:
-  unreachable
-}
-; CHECK-FP-LABEL:      {{_?}}func:
-; CHECK-FP-LABEL: nop {{[;!]}} avoids zero-length function

Modified: llvm/trunk/test/CodeGen/X86/empty-functions.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/empty-functions.ll?rev=217801&r1=217800&r2=217801&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/empty-functions.ll (original)
+++ llvm/trunk/test/CodeGen/X86/empty-functions.ll Mon Sep 15 13:32:58 2014
@@ -1,10 +1,14 @@
 ; RUN: llc < %s -mtriple=x86_64-apple-darwin | FileCheck -check-prefix=CHECK-NO-FP %s
 ; RUN: llc < %s -mtriple=x86_64-apple-darwin -disable-fp-elim | FileCheck -check-prefix=CHECK-FP %s
+; RUN: llc < %s -mtriple=x86_64-linux-gnu | FileCheck -check-prefix=LINUX-NO-FP %s
+; RUN: llc < %s -mtriple=x86_64-linux-gnu -disable-fp-elim | FileCheck -check-prefix=LINUX-FP %s
 
 define void @func() {
 entry:
   unreachable
 }
+
+; MachO cannot handle an empty function.
 ; CHECK-NO-FP:     _func:
 ; CHECK-NO-FP-NEXT: .cfi_startproc
 ; CHECK-NO-FP:     nop
@@ -21,5 +25,30 @@ entry:
 ; CHECK-FP-NEXT: movq %rsp, %rbp
 ; CHECK-FP-NEXT: :
 ; CHECK-FP-NEXT: .cfi_def_cfa_register %rbp
-; CHECK-FP-NEXT: nop
 ; CHECK-FP-NEXT: .cfi_endproc
+
+; An empty function is perfectly fine on ELF.
+; LINUX-NO-FP: func:
+; LINUX-NO-FP-NEXT: .cfi_startproc
+; LINUX-NO-FP-NEXT: {{^}}#
+; LINUX-NO-FP-NEXT: {{^}}.L{{.*}}:{{$}}
+; LINUX-NO-FP-NEXT: .size   func, .L{{.*}}-func
+; LINUX-NO-FP-NEXT: .cfi_endproc
+
+; A cfi directive can point to the end of a function. It (and in fact the
+; entire body) could be optimized out because of the unreachable, but we
+; don't do it right now.
+; LINUX-FP: func:
+; LINUX-FP-NEXT: .cfi_startproc
+; LINUX-FP-NEXT: {{^}}#
+; LINUX-FP-NEXT: pushq %rbp
+; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}
+; LINUX-FP-NEXT:  .cfi_def_cfa_offset 16
+; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}
+; LINUX-FP-NEXT: .cfi_offset %rbp, -16
+; LINUX-FP-NEXT: movq        %rsp, %rbp
+; LINUX-FP-NEXT:{{^}}.L{{.*}}:{{$}}
+; LINUX-FP-NEXT: .cfi_def_cfa_register %rbp
+; LINUX-FP-NEXT:{{^}}.L{{.*}}:{{$}}
+; LINUX-FP-NEXT: .size   func, .Ltmp3-func
+; LINUX-FP-NEXT: .cfi_endproc





More information about the llvm-commits mailing list