[llvm] r289928 - Revert r289638: [PowerPC] Fix logic dealing with nop after calls (and tail-call eligibility)
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 15 23:31:21 PST 2016
Author: chandlerc
Date: Fri Dec 16 01:31:20 2016
New Revision: 289928
URL: http://llvm.org/viewvc/llvm-project?rev=289928&view=rev
Log:
Revert r289638: [PowerPC] Fix logic dealing with nop after calls (and tail-call eligibility)
This patch appears to result in trampolines in vtables being miscompiled
when they in turn tail call a method.
I've posted some preliminary details about the failure on the thread for
this commit and talked to Hal. He was comfortable going ahead and
reverting until we sort out what is wrong.
Removed:
llvm/trunk/test/CodeGen/PowerPC/ppc64-blnop.ll
Modified:
llvm/trunk/lib/Target/PowerPC/PPCISelLowering.cpp
llvm/trunk/test/CodeGen/PowerPC/ppc64-sibcall.ll
Modified: llvm/trunk/lib/Target/PowerPC/PPCISelLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCISelLowering.cpp?rev=289928&r1=289927&r2=289928&view=diff
==============================================================================
--- llvm/trunk/lib/Target/PowerPC/PPCISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/PowerPC/PPCISelLowering.cpp Fri Dec 16 01:31:20 2016
@@ -3981,32 +3981,40 @@ static int CalculateTailCallSPDiff(Selec
static bool isFunctionGlobalAddress(SDValue Callee);
static bool
-resideInSameSection(const Function *Caller, SDValue Callee,
- const TargetMachine &TM) {
+resideInSameModule(SDValue Callee, Reloc::Model RelMod) {
// If !G, Callee can be an external symbol.
GlobalAddressSDNode *G = dyn_cast<GlobalAddressSDNode>(Callee);
- if (!G)
- return false;
+ if (!G) return false;
const GlobalValue *GV = G->getGlobal();
- if (!GV->isStrongDefinitionForLinker())
- return false;
- // Any explicitly-specified sections and section prefixes must also match.
- // Also, if we're using -ffunction-sections, then each function is always in
- // a different section (the same is true for COMDAT functions).
- if (TM.getFunctionSections() || GV->hasComdat() || Caller->hasComdat() ||
- GV->getSection() != Caller->getSection())
+ if (GV->isDeclaration()) return false;
+
+ switch(GV->getLinkage()) {
+ default: llvm_unreachable("unknow linkage type");
+ case GlobalValue::AvailableExternallyLinkage:
+ case GlobalValue::ExternalWeakLinkage:
return false;
- if (const auto *F = dyn_cast<Function>(GV)) {
- if (F->getSectionPrefix() != Caller->getSectionPrefix())
+
+ // Callee with weak linkage is allowed if it has hidden or protected
+ // visibility
+ case GlobalValue::LinkOnceAnyLinkage:
+ case GlobalValue::LinkOnceODRLinkage: // e.g. c++ inline functions
+ case GlobalValue::WeakAnyLinkage:
+ case GlobalValue::WeakODRLinkage: // e.g. c++ template instantiation
+ if (GV->hasDefaultVisibility())
return false;
+
+ case GlobalValue::ExternalLinkage:
+ case GlobalValue::InternalLinkage:
+ case GlobalValue::PrivateLinkage:
+ break;
}
- // If the callee might be interposed, then we can't assume the ultimate call
- // target will be in the same section.
- if (GV->isInterposable() &&
- !TM.shouldAssumeDSOLocal(*Caller->getParent(), GV))
+ // With '-fPIC', calling default visiblity function need insert 'nop' after
+ // function call, no matter that function resides in same module or not, so
+ // we treat it as in different module.
+ if (RelMod == Reloc::PIC_ && GV->hasDefaultVisibility())
return false;
return true;
@@ -4122,11 +4130,11 @@ PPCTargetLowering::IsEligibleForTailCall
!isa<ExternalSymbolSDNode>(Callee))
return false;
- // Check if Callee resides in the same section, because for now, PPC64 SVR4
- // ABI (ELFv1/ELFv2) doesn't allow tail calls to a symbol resides in another
- // section.
+ // Check if Callee resides in the same module, because for now, PPC64 SVR4 ABI
+ // (ELFv1/ELFv2) doesn't allow tail calls to a symbol resides in another
+ // module.
// ref: https://bugzilla.mozilla.org/show_bug.cgi?id=973977
- if (!resideInSameSection(MF.getFunction(), Callee, getTargetMachine()))
+ if (!resideInSameModule(Callee, getTargetMachine().getRelocationModel()))
return false;
// TCO allows altering callee ABI, so we don't have to check further.
@@ -4584,6 +4592,14 @@ PrepareCall(SelectionDAG &DAG, SDValue &
return CallOpc;
}
+static
+bool isLocalCall(const SDValue &Callee)
+{
+ if (GlobalAddressSDNode *G = dyn_cast<GlobalAddressSDNode>(Callee))
+ return G->getGlobal()->isStrongDefinitionForLinker();
+ return false;
+}
+
SDValue PPCTargetLowering::LowerCallResult(
SDValue Chain, SDValue InFlag, CallingConv::ID CallConv, bool isVarArg,
const SmallVectorImpl<ISD::InputArg> &Ins, const SDLoc &dl,
@@ -4685,7 +4701,6 @@ SDValue PPCTargetLowering::FinishCall(
// stack frame. If caller and callee belong to the same module (and have the
// same TOC), the NOP will remain unchanged.
- MachineFunction &MF = DAG.getMachineFunction();
if (!isTailCall && Subtarget.isSVR4ABI()&& Subtarget.isPPC64() &&
!isPatchPoint) {
if (CallOpc == PPCISD::BCTRL) {
@@ -4709,11 +4724,11 @@ SDValue PPCTargetLowering::FinishCall(
// The address needs to go after the chain input but before the flag (or
// any other variadic arguments).
Ops.insert(std::next(Ops.begin()), AddTOC);
- } else if (CallOpc == PPCISD::CALL &&
- !resideInSameSection(MF.getFunction(), Callee, DAG.getTarget())) {
+ } else if ((CallOpc == PPCISD::CALL) &&
+ (!isLocalCall(Callee) ||
+ DAG.getTarget().getRelocationModel() == Reloc::PIC_))
// Otherwise insert NOP for non-local calls.
CallOpc = PPCISD::CALL_NOP;
- }
}
Chain = DAG.getNode(CallOpc, dl, NodeTys, Ops);
Removed: llvm/trunk/test/CodeGen/PowerPC/ppc64-blnop.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/PowerPC/ppc64-blnop.ll?rev=289927&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/PowerPC/ppc64-blnop.ll (original)
+++ llvm/trunk/test/CodeGen/PowerPC/ppc64-blnop.ll (removed)
@@ -1,129 +0,0 @@
-; RUN: llc < %s -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu | FileCheck %s
-; RUN: llc < %s -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr8 | FileCheck %s
-; RUN: llc < %s -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr8 | FileCheck %s
-; RUN: llc < %s -relocation-model=pic -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu | FileCheck %s
-; RUN: llc < %s -function-sections -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu | FileCheck %s -check-prefix=CHECK-FS
-; RUN: llc < %s -relocation-model=pic -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu | FileCheck %s
-; RUN: llc < %s -function-sections -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu | FileCheck %s -check-prefix=CHECK-FS
-
-%class.T = type { [2 x i8] }
-
-define void @e_callee(%class.T* %this, i8* %c) { ret void }
-define void @e_caller(%class.T* %this, i8* %c) {
- call void @e_callee(%class.T* %this, i8* %c)
- ret void
-
-; CHECK-LABEL: e_caller:
-; CHECK: bl e_callee
-; CHECK-NOT: nop
-
-; CHECK-FS-LABEL: e_caller:
-; CHECK-FS: bl e_callee
-; CHECK-FS-NEXT: nop
-}
-
-define void @e_scallee(%class.T* %this, i8* %c) section "different" { ret void }
-define void @e_scaller(%class.T* %this, i8* %c) {
- call void @e_scallee(%class.T* %this, i8* %c)
- ret void
-
-; CHECK-LABEL: e_scaller:
-; CHECK: bl e_scallee
-; CHECK-NEXT: nop
-}
-
-define void @e_s2callee(%class.T* %this, i8* %c) { ret void }
-define void @e_s2caller(%class.T* %this, i8* %c) section "different" {
- call void @e_s2callee(%class.T* %this, i8* %c)
- ret void
-
-; CHECK-LABEL: e_s2caller:
-; CHECK: bl e_s2callee
-; CHECK-NEXT: nop
-}
-
-$cd1 = comdat any
-$cd2 = comdat any
-
-define void @e_ccallee(%class.T* %this, i8* %c) comdat($cd1) { ret void }
-define void @e_ccaller(%class.T* %this, i8* %c) comdat($cd2) {
- call void @e_ccallee(%class.T* %this, i8* %c)
- ret void
-
-; CHECK-LABEL: e_ccaller:
-; CHECK: bl e_ccallee
-; CHECK-NEXT: nop
-}
-
-$cd = comdat any
-
-define void @e_c1callee(%class.T* %this, i8* %c) comdat($cd) { ret void }
-define void @e_c1caller(%class.T* %this, i8* %c) comdat($cd) {
- call void @e_c1callee(%class.T* %this, i8* %c)
- ret void
-
-; CHECK-LABEL: e_c1caller:
-; CHECK: bl e_c1callee
-; CHECK-NEXT: nop
-}
-
-define weak_odr hidden void @wo_hcallee(%class.T* %this, i8* %c) { ret void }
-define void @wo_hcaller(%class.T* %this, i8* %c) {
- call void @wo_hcallee(%class.T* %this, i8* %c)
- ret void
-
-; CHECK-LABEL: wo_hcaller:
-; CHECK: bl wo_hcallee
-; CHECK-NEXT: nop
-}
-
-define weak_odr protected void @wo_pcallee(%class.T* %this, i8* %c) { ret void }
-define void @wo_pcaller(%class.T* %this, i8* %c) {
- call void @wo_pcallee(%class.T* %this, i8* %c)
- ret void
-
-; CHECK-LABEL: wo_pcaller:
-; CHECK: bl wo_pcallee
-; CHECK-NEXT: nop
-}
-
-define weak_odr void @wo_callee(%class.T* %this, i8* %c) { ret void }
-define void @wo_caller(%class.T* %this, i8* %c) {
- call void @wo_callee(%class.T* %this, i8* %c)
- ret void
-
-; CHECK-LABEL: wo_caller:
-; CHECK: bl wo_callee
-; CHECK-NEXT: nop
-}
-
-define weak protected void @w_pcallee(i8* %ptr) { ret void }
-define void @w_pcaller(i8* %ptr) {
- call void @w_pcallee(i8* %ptr)
- ret void
-
-; CHECK-LABEL: w_pcaller:
-; CHECK: bl w_pcallee
-; CHECK-NEXT: nop
-}
-
-define weak hidden void @w_hcallee(i8* %ptr) { ret void }
-define void @w_hcaller(i8* %ptr) {
- call void @w_hcallee(i8* %ptr)
- ret void
-
-; CHECK-LABEL: w_hcaller:
-; CHECK: bl w_hcallee
-; CHECK-NEXT: nop
-}
-
-define weak void @w_callee(i8* %ptr) { ret void }
-define void @w_caller(i8* %ptr) {
- call void @w_callee(i8* %ptr)
- ret void
-
-; CHECK-LABEL: w_caller:
-; CHECK: bl w_callee
-; CHECK-NEXT: nop
-}
-
Modified: llvm/trunk/test/CodeGen/PowerPC/ppc64-sibcall.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/PowerPC/ppc64-sibcall.ll?rev=289928&r1=289927&r2=289928&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/PowerPC/ppc64-sibcall.ll (original)
+++ llvm/trunk/test/CodeGen/PowerPC/ppc64-sibcall.ll Fri Dec 16 01:31:20 2016
@@ -142,7 +142,7 @@ define void @wo_hcaller(%class.T* %this,
ret void
; CHECK-SCO-LABEL: wo_hcaller:
-; CHECK-SCO: bl wo_hcallee
+; CHECK-SCO: b wo_hcallee
}
define weak_odr protected void @wo_pcallee(%class.T* %this, i8* %c) { ret void }
@@ -151,7 +151,7 @@ define void @wo_pcaller(%class.T* %this,
ret void
; CHECK-SCO-LABEL: wo_pcaller:
-; CHECK-SCO: bl wo_pcallee
+; CHECK-SCO: b wo_pcallee
}
define weak_odr void @wo_callee(%class.T* %this, i8* %c) { ret void }
@@ -169,7 +169,7 @@ define void @w_pcaller(i8* %ptr) {
ret void
; CHECK-SCO-LABEL: w_pcaller:
-; CHECK-SCO: bl w_pcallee
+; CHECK-SCO: b w_pcallee
}
define weak hidden void @w_hcallee(i8* %ptr) { ret void }
@@ -178,7 +178,7 @@ define void @w_hcaller(i8* %ptr) {
ret void
; CHECK-SCO-LABEL: w_hcaller:
-; CHECK-SCO: bl w_hcallee
+; CHECK-SCO: b w_hcallee
}
define weak void @w_callee(i8* %ptr) { ret void }
More information about the llvm-commits
mailing list