OSDN Git Service

Revert r289638: [PowerPC] Fix logic dealing with nop after calls (and tail-call eligi...
authorChandler Carruth <chandlerc@gmail.com>
Fri, 16 Dec 2016 07:31:20 +0000 (07:31 +0000)
committerChandler Carruth <chandlerc@gmail.com>
Fri, 16 Dec 2016 07:31:20 +0000 (07:31 +0000)
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.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@289928 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Target/PowerPC/PPCISelLowering.cpp
test/CodeGen/PowerPC/ppc64-blnop.ll [deleted file]
test/CodeGen/PowerPC/ppc64-sibcall.ll

index 15765bc..aa3ffde 100644 (file)
@@ -3981,32 +3981,40 @@ static int CalculateTailCallSPDiff(SelectionDAG& DAG, bool isTailCall,
 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::IsEligibleForTailCallOptimization_64SVR4(
       !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 &Callee, SDValue &InFlag, SDValue &Chain,
   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);
diff --git a/test/CodeGen/PowerPC/ppc64-blnop.ll b/test/CodeGen/PowerPC/ppc64-blnop.ll
deleted file mode 100644 (file)
index 384edf3..0000000
+++ /dev/null
@@ -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
-}
-
index 59e5456..418b782 100644 (file)
@@ -142,7 +142,7 @@ define void @wo_hcaller(%class.T* %this, i8* %c) {
   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, i8* %c) {
   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 }