<div dir="ltr">Thanks for doing this Rafael!<div><br></div><div>-eric</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 15, 2014 at 11:32 AM, Rafael Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rafael<br>
Date: Mon Sep 15 13:32:58 2014<br>
New Revision: 217801<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=217801&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=217801&view=rev</a><br>
Log:<br>
Fix a lot of confusion around inserting nops on empty functions.<br>
<br>
On MachO, and MachO only, we cannot have a truly empty function since that<br>
breaks the linker logic for atomizing the section.<br>
<br>
When we are emitting a frame pointer, the presence of an unreachable will<br>
create a cfi instruction pointing past the last instruction. This is perfectly<br>
fine. The FDE information encodes the pc range it applies to. If some tool<br>
cannot handle this, we should explicitly say which bug we are working around<br>
and only work around it when it is actually relevant (not for ELF for example).<br>
<br>
Given the unreachable we could omit the .cfi_def_cfa_register, but then<br>
again, we could also omit the entire function prologue if we wanted to.<br>
<br>
Removed:<br>
llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll<br>
llvm/trunk/test/CodeGen/SPARC/empty-functions.ll<br>
Modified:<br>
llvm/trunk/include/llvm/Target/TargetInstrInfo.h<br>
llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp<br>
llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp<br>
llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp<br>
llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h<br>
llvm/trunk/test/CodeGen/X86/empty-functions.ll<br>
<br>
Modified: llvm/trunk/include/llvm/Target/TargetInstrInfo.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetInstrInfo.h?rev=217801&r1=217800&r2=217801&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetInstrInfo.h?rev=217801&r1=217800&r2=217801&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Target/TargetInstrInfo.h (original)<br>
+++ llvm/trunk/include/llvm/Target/TargetInstrInfo.h Mon Sep 15 13:32:58 2014<br>
@@ -847,10 +847,8 @@ public:<br>
MachineBasicBlock::iterator MI) const;<br>
<br>
<br>
- /// getNoopForMachoTarget - Return the noop instruction to use for a noop.<br>
- virtual void getNoopForMachoTarget(MCInst &NopInst) const {<br>
- // Default to just using 'nop' string.<br>
- }<br>
+ /// Return the noop instruction to use for a noop.<br>
+ virtual void getNoopForMachoTarget(MCInst &NopInst) const;<br>
<br>
<br>
/// isPredicated - Returns true if the instruction is already predicated.<br>
<br>
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp?rev=217801&r1=217800&r2=217801&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp?rev=217801&r1=217800&r2=217801&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp Mon Sep 15 13:32:58 2014<br>
@@ -731,12 +731,10 @@ void AsmPrinter::EmitFunctionBody() {<br>
<br>
// Print out code for the function.<br>
bool HasAnyRealCode = false;<br>
- const MachineInstr *LastMI = nullptr;<br>
for (auto &MBB : *MF) {<br>
// Print a label for the basic block.<br>
EmitBasicBlockStart(MBB);<br>
for (auto &MI : MBB) {<br>
- LastMI = &MI;<br>
<br>
// Print the assembly for the instruction.<br>
if (!MI.isPosition() && !MI.isImplicitDef() && !MI.isKill() &&<br>
@@ -797,24 +795,14 @@ void AsmPrinter::EmitFunctionBody() {<br>
EmitBasicBlockEnd(MBB);<br>
}<br>
<br>
- // If the last instruction was a prolog label, then we have a situation where<br>
- // we emitted a prolog but no function body. This results in the ending prolog<br>
- // label equaling the end of function label and an invalid "row" in the<br>
- // FDE. We need to emit a noop in this situation so that the FDE's rows are<br>
- // valid.<br>
- bool RequiresNoop = LastMI && LastMI->isCFIInstruction();<br>
-<br>
// If the function is empty and the object file uses .subsections_via_symbols,<br>
// then we need to emit *something* to the function body to prevent the<br>
// labels from collapsing together. Just emit a noop.<br>
- if ((MAI->hasSubsectionsViaSymbols() && !HasAnyRealCode) || RequiresNoop) {<br>
+ if ((MAI->hasSubsectionsViaSymbols() && !HasAnyRealCode)) {<br>
MCInst Noop;<br>
TM.getSubtargetImpl()->getInstrInfo()->getNoopForMachoTarget(Noop);<br>
- if (Noop.getOpcode()) {<br>
- OutStreamer.AddComment("avoids zero-length function");<br>
- OutStreamer.EmitInstruction(Noop, getSubtargetInfo());<br>
- } else // Target not mc-ized yet.<br>
- OutStreamer.EmitRawText(StringRef("\tnop\n"));<br>
+ OutStreamer.AddComment("avoids zero-length function");<br>
+ OutStreamer.EmitInstruction(Noop, getSubtargetInfo());<br>
}<br>
<br>
const Function *F = MF->getFunction();<br>
<br>
Modified: llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp?rev=217801&r1=217800&r2=217801&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp?rev=217801&r1=217800&r2=217801&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp Mon Sep 15 13:32:58 2014<br>
@@ -372,6 +372,10 @@ static const TargetRegisterClass *canFol<br>
return nullptr;<br>
}<br>
<br>
+void TargetInstrInfo::getNoopForMachoTarget(MCInst &NopInst) const {<br>
+ llvm_unreachable("Not a MachO target");<br>
+}<br>
+<br>
bool TargetInstrInfo::<br>
canFoldMemoryOperand(const MachineInstr *MI,<br>
const SmallVectorImpl<unsigned> &Ops) const {<br>
<br>
Modified: llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp?rev=217801&r1=217800&r2=217801&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp?rev=217801&r1=217800&r2=217801&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp (original)<br>
+++ llvm/trunk/lib/Target/Sparc/SparcInstrInfo.cpp Mon Sep 15 13:32:58 2014<br>
@@ -37,11 +37,6 @@ SparcInstrInfo::SparcInstrInfo(SparcSubt<br>
RI(ST), Subtarget(ST) {<br>
}<br>
<br>
-/// getNoopForMachoTarget - Return the noop instruction to use for a noop.<br>
-void SparcInstrInfo::getNoopForMachoTarget(MCInst &NopInst) const {<br>
- NopInst.setOpcode(SP::NOP);<br>
-}<br>
-<br>
/// isLoadFromStackSlot - If the specified machine instruction is a direct<br>
/// load from a stack slot, return the virtual or physical register number of<br>
/// the destination along with the FrameIndex of the loaded stack slot. If<br>
<br>
Modified: llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h?rev=217801&r1=217800&r2=217801&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h?rev=217801&r1=217800&r2=217801&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h (original)<br>
+++ llvm/trunk/lib/Target/Sparc/SparcInstrInfo.h Mon Sep 15 13:32:58 2014<br>
@@ -93,8 +93,6 @@ public:<br>
const TargetRegisterInfo *TRI) const override;<br>
<br>
unsigned getGlobalBaseReg(MachineFunction *MF) const;<br>
-<br>
- void getNoopForMachoTarget(MCInst &NopInst) const override;<br>
};<br>
<br>
}<br>
<br>
Removed: llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll?rev=217800&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll?rev=217800&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll (original)<br>
+++ llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll (removed)<br>
@@ -1,13 +0,0 @@<br>
-; RUN: llc < %s -mtriple=powerpc-apple-darwin | FileCheck -check-prefix=CHECK-NO-FP %s<br>
-; RUN: llc < %s -mtriple=powerpc-apple-darwin -disable-fp-elim | FileCheck -check-prefix=CHECK-FP %s<br>
-; RUN: llc < %s -mtriple=powerpc-netbsd -disable-fp-elim | FileCheck -check-prefix=CHECK-FP %s<br>
-<br>
-define void @func() {<br>
-entry:<br>
- unreachable<br>
-}<br>
-; CHECK-NO-FP: _func:<br>
-; CHECK-NO-FP: nop<br>
-<br>
-; CHECK-FP: {{_?}}func:<br>
-; CHECK-FP: nop {{[;#]}} avoids zero-length function<br>
<br>
Removed: llvm/trunk/test/CodeGen/SPARC/empty-functions.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/SPARC/empty-functions.ll?rev=217800&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/SPARC/empty-functions.ll?rev=217800&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/CodeGen/SPARC/empty-functions.ll (original)<br>
+++ llvm/trunk/test/CodeGen/SPARC/empty-functions.ll (removed)<br>
@@ -1,8 +0,0 @@<br>
-; RUN: llc < %s -mtriple=sparc-unknown-openbsd -disable-fp-elim | FileCheck -check-prefix=CHECK-FP-LABEL %s<br>
-<br>
-define void @func() {<br>
-entry:<br>
- unreachable<br>
-}<br>
-; CHECK-FP-LABEL: {{_?}}func:<br>
-; CHECK-FP-LABEL: nop {{[;!]}} avoids zero-length function<br>
<br>
Modified: llvm/trunk/test/CodeGen/X86/empty-functions.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/empty-functions.ll?rev=217801&r1=217800&r2=217801&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/empty-functions.ll?rev=217801&r1=217800&r2=217801&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/CodeGen/X86/empty-functions.ll (original)<br>
+++ llvm/trunk/test/CodeGen/X86/empty-functions.ll Mon Sep 15 13:32:58 2014<br>
@@ -1,10 +1,14 @@<br>
; RUN: llc < %s -mtriple=x86_64-apple-darwin | FileCheck -check-prefix=CHECK-NO-FP %s<br>
; RUN: llc < %s -mtriple=x86_64-apple-darwin -disable-fp-elim | FileCheck -check-prefix=CHECK-FP %s<br>
+; RUN: llc < %s -mtriple=x86_64-linux-gnu | FileCheck -check-prefix=LINUX-NO-FP %s<br>
+; RUN: llc < %s -mtriple=x86_64-linux-gnu -disable-fp-elim | FileCheck -check-prefix=LINUX-FP %s<br>
<br>
define void @func() {<br>
entry:<br>
unreachable<br>
}<br>
+<br>
+; MachO cannot handle an empty function.<br>
; CHECK-NO-FP: _func:<br>
; CHECK-NO-FP-NEXT: .cfi_startproc<br>
; CHECK-NO-FP: nop<br>
@@ -21,5 +25,30 @@ entry:<br>
; CHECK-FP-NEXT: movq %rsp, %rbp<br>
; CHECK-FP-NEXT: :<br>
; CHECK-FP-NEXT: .cfi_def_cfa_register %rbp<br>
-; CHECK-FP-NEXT: nop<br>
; CHECK-FP-NEXT: .cfi_endproc<br>
+<br>
+; An empty function is perfectly fine on ELF.<br>
+; LINUX-NO-FP: func:<br>
+; LINUX-NO-FP-NEXT: .cfi_startproc<br>
+; LINUX-NO-FP-NEXT: {{^}}#<br>
+; LINUX-NO-FP-NEXT: {{^}}.L{{.*}}:{{$}}<br>
+; LINUX-NO-FP-NEXT: .size func, .L{{.*}}-func<br>
+; LINUX-NO-FP-NEXT: .cfi_endproc<br>
+<br>
+; A cfi directive can point to the end of a function. It (and in fact the<br>
+; entire body) could be optimized out because of the unreachable, but we<br>
+; don't do it right now.<br>
+; LINUX-FP: func:<br>
+; LINUX-FP-NEXT: .cfi_startproc<br>
+; LINUX-FP-NEXT: {{^}}#<br>
+; LINUX-FP-NEXT: pushq %rbp<br>
+; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}<br>
+; LINUX-FP-NEXT: .cfi_def_cfa_offset 16<br>
+; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}<br>
+; LINUX-FP-NEXT: .cfi_offset %rbp, -16<br>
+; LINUX-FP-NEXT: movq %rsp, %rbp<br>
+; LINUX-FP-NEXT:{{^}}.L{{.*}}:{{$}}<br>
+; LINUX-FP-NEXT: .cfi_def_cfa_register %rbp<br>
+; LINUX-FP-NEXT:{{^}}.L{{.*}}:{{$}}<br>
+; LINUX-FP-NEXT: .size func, .Ltmp3-func<br>
+; LINUX-FP-NEXT: .cfi_endproc<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>