[PowerPC] Refine the checking for emiting TOC restore nops and Tail-Call eligibility.

[sfertile]

Summary

In the Small code-model the linker will allocate multiple TOC bases for modules with TOCs that overflow a 16 byte offset. If this happens then all the issues with cross-module calls apply to calls within the same module but with different TOC bases. Since we don't know at compile time where the linker will decide to split the module we can only treat calls within the same section as 'local'. The medium and large code models are expected to provide a sufficiently large TOC to provide all data addressing needs of a module with a single TOC, and so we only need to check if a callee is local to the same module as the caller.

[gyiu]

lib/Target/PowerPC/PPCISelLowering.cpp

4254 ↗

(On Diff #102695)

Should we just move the code from this function to the existing resideInSameSection() function and rename the function itself? I see in there we're also checking

GlobalAddressSDNode *G = dyn_cast<GlobalAddressSDNode>(Callee);
if (!G)
  return true;

[sfertile]

lib/Target/PowerPC/PPCISelLowering.cpp

4254 ↗

(On Diff #102695)

I think that is a good suggestion. I can't think of any other reason we would want to see if 2 symbols reside in the same section other then if they share a TOC base. I'll update it.

[nemanjai]

lib/Target/PowerPC/PPCISelLowering.cpp

4254 ↗

(On Diff #102695)

I agree. Both calls to this function are really after the same thing. Although I find this name just slightly misleading. Perhaps something like mayHaveDifferentTOCs() or callRequiresTOCRestoreNop().

test/CodeGen/PowerPC/ppc64-blnop.ll

82 ↗

(On Diff #102695)

Well, I think it would be more appropriate to include the call opcode as well - I imagine this one would be bl.

test/CodeGen/PowerPC/ppc64-calls.ll

28 ↗

(On Diff #102695)

This comment is misleading. It almost seems to imply that with the large and medium code model, a weak definition can't be overridden in a different module. You should probably say something like "With the large and medium code models, both definitions will have the same TOC since they won't reside in different DSO's."

Or something along those lines.

[hfinkel]

lib/Target/PowerPC/PPCISelLowering.cpp

4264 ↗

(On Diff #102695)

codemodel should either be "code model" or "CodeModel". My preference is to just write this in English, "small code model", with no extra capitalization.

4265 ↗

(On Diff #102695)

This does not explain why this cannot happen for the other code models. The ABI is a bit ambiguous here, saying "The medium code model is expected to provide a sufficiently large TOC to provide all data addressing needs of a module with a single TOC." Expected, however, does not mean "must". We need to supplement here with some additional explanation of what existing linkers actually do (e.g. that neither ld.bfd nor ld.gold will create multiple TOCs for anything other than the small code model, regardless of what might be theoretically allowed).

4266 ↗

(On Diff #102695)

I don't see exactly how this reference is relevant. There is some explanation of why you can't tail call to functions in other modules, but if that's not explained in the comments in this file, we should do that directly.

[sfertile]

Addressed a number of the review comments.

[sfertile]

lib/Target/PowerPC/PPCISelLowering.cpp

4265 ↗

(On Diff #102695)

I'm going to send an email to Alan Modra to verify my understanding of the behavior for medium and large code models is correct for both gold and ld. I'll update this comment to reflect that once I get an answer.

[sfertile]

Updated comments describing linker behavior relating to TOC overflow.

[inouehrs]

lib/Target/PowerPC/PPCISelLowering.cpp

4252 ↗

(On Diff #105469)

In my understanding, ELFv1 uses small code model (please consult with ABI document). If so we need to check ELFv2 ABI.

[sfertile]

lib/Target/PowerPC/PPCISelLowering.cpp

4252 ↗

(On Diff #105469)

I thought the V1 ABI used small code model for 32 bit and medium code model for 64 bit, but I'll double check.

[hfinkel]

lib/Target/PowerPC/PPCISelLowering.cpp

4273 ↗

(On Diff #105469)

What happened before binutils 2.24? 2.24 is only about 3.5 years old.

[sfertile]

lib/Target/PowerPC/PPCISelLowering.cpp

4252 ↗

(On Diff #105469)

I dug through the 'V1.9 64 bit ELF ABI supplement ' as well as the older '[32-bit] PowerPC Processor Supplement' and neither mention what code model should be used by default. I think I originally used the gcc docs to infer that ppc64 defaults to medium code model:

IBM RS/6000 and PowerPC Options

-mcmodel=medium

Generate PowerPC64 code for the medium model: The TOC and other static data may be up to a total of 4G in size. This is the default for 64-bit Linux.

and then found 'adjustCodeGenOpts' in PPCMCTargetDesc.cpp which confirmed it.

[inouehrs]

lib/Target/PowerPC/PPCISelLowering.cpp

4252 ↗

(On Diff #105469)

Sure. Thanks!

[sfertile]

lib/Target/PowerPC/PPCISelLowering.cpp

4273 ↗

(On Diff #105469)

I don't know. I asked what the behavior of the 2 linkers was if the TOC grows larger then the 32 bit offset, as well as checking that 'module' in the abi text meant entire exe and entire shared-object (rather then just a single object file). I assumed that the behavior change in 2.24 was from generic linker error to specific diagnostic rather then add new toc base --> error. I can follow up though to ensure this is the case.

[sfertile]

rebased

[sfertile]

Simplified the codemodel check since Default and JitDefault code models were removed.

[sfertile]

lib/Target/PowerPC/PPCISelLowering.cpp

4273 ↗

(On Diff #105469)

Before the change in 2.24 if the offset for *_HI and *_HA relocs didn't fit in 32 bits a 4 instruction sequence would be used to build a 64 bit constant. The change added the 32 bit limit and overflow checking to the *_HI and *_HA relocs and added *_HIGH and *_HIGHA relocs to be used if a 64 bit offset may be needed. So even with the old behavior, its still a single TOC base.

[hfinkel]

LGTM

lib/Target/PowerPC/PPCISelLowering.cpp

4273 ↗

(On Diff #105469)

Thanks for checking.

llvm/trunk/lib/Target/PowerPC/PPCISelLowering.cpp

if (SPDiff < FI->getTailCallSPDelta())
     FI->setTailCallSPDelta(SPDiff);
 
   return SPDiff;
 }
 
 static bool isFunctionGlobalAddress(SDValue Callee);
 
 static bool
-resideInSameSection(const Function *Caller, SDValue Callee,
+callsShareTOCBase(const Function *Caller, SDValue Callee,
                     const TargetMachine &TM) {
   // If !G, Callee can be an external symbol.
   GlobalAddressSDNode *G = dyn_cast<GlobalAddressSDNode>(Callee);
   if (!G)
     return false;
 
+  // The medium and large code models are expected to provide a sufficiently
+  // large TOC to provide all data addressing needs of a module with a
+  // single TOC. Since each module will be addressed with a single TOC then we
+  // only need to check that caller and callee don't cross dso boundaries.
+  if (CodeModel::Medium == TM.getCodeModel() ||
+      CodeModel::Large == TM.getCodeModel())
+    return TM.shouldAssumeDSOLocal(*Caller->getParent(), G->getGlobal());
+
+  // Otherwise we need to ensure callee and caller are in the same section,
+  // since the linker may allocate multiple TOCs, and we don't know which
+  // sections will belong to the same TOC base.
+
   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() ||
PPCTargetLowering::IsEligibleForTailCallOptimization_64SVR4(
   if (any_of(Outs, [](const ISD::OutputArg& OA) { return OA.Flags.isByVal(); }))
     return false;
 
   // No TCO/SCO on indirect call because Caller have to restore its TOC
   if (!isFunctionGlobalAddress(Callee) &&
       !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.
+  // If the caller and callee potentially have different TOC bases then we
+  // cannot tail call since we need to restore the TOC pointer after the call.
   // ref: https://bugzilla.mozilla.org/show_bug.cgi?id=973977
-  if (!resideInSameSection(MF.getFunction(), Callee, getTargetMachine()))
+  if (!callsShareTOCBase(MF.getFunction(), Callee, getTargetMachine()))
     return false;
 
   // TCO allows altering callee ABI, so we don't have to check further.
   if (CalleeCC == CallingConv::Fast && TailCallOpt)
     return true;
 
   if (DisableSCO) return false;
 
if (CallOpc == PPCISD::BCTRL) {
       unsigned TOCSaveOffset = Subtarget.getFrameLowering()->getTOCSaveOffset();
       SDValue TOCOff = DAG.getIntPtrConstant(TOCSaveOffset, dl);
       SDValue AddTOC = DAG.getNode(ISD::ADD, dl, MVT::i64, StackPtr, TOCOff);
 
       // 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())) {
+      !callsShareTOCBase(MF.getFunction(), Callee, DAG.getTarget())) {
       // Otherwise insert NOP for non-local calls.
       CallOpc = PPCISD::CALL_NOP;
     }
   }
 
   Chain = DAG.getNode(CallOpc, dl, NodeTys, Ops);
   InFlag = Chain.getValue(1);
 

llvm/trunk/test/CodeGen/PowerPC/ppc64-blnop.ll

 ; 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
+; RUN: llc < %s -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN: -code-model=small -mcpu=pwr8 | FileCheck %s -check-prefix=SCM
 
 %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
 

 
 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
+; CHECK-NOT: nop
+
+; SCM-LABEL: wo_hcaller:
+; SCM:       bl wo_hcallee
+; SCM-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
+; CHECK-NOT: nop
+
+; SCM-LABEL:   wo_pcaller:
+; SCM:         bl wo_pcallee
+; SCM-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
+; CHECK-NOT: nop
+
+; SCM-LABEL: w_pcaller:
+; SCM:       bl w_pcallee
+; SCM-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
+; CHECK-NOT: nop
+
+; SCM-LABEL: w_hcaller:
+; SCM:       bl w_hcallee
+; SCM-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
 }
 

llvm/trunk/test/CodeGen/PowerPC/ppc64-calls.ll

 ; RUN: llc -relocation-model=static -verify-machineinstrs < %s -mcpu=pwr7 | FileCheck %s
+; RUN: llc -relocation-model=static -verify-machineinstrs < %s -code-model=small -mcpu=pwr7 | FileCheck %s -check-prefix=SCM
+
 target datalayout = "E-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v128:128:128-n32:64"
 target triple = "powerpc64-unknown-linux-gnu"
 
 
 define void @foo() nounwind noinline {
   ret void
 }
 
 define weak void @foo_weak() nounwind {
   ret void
 }
 
 ; Calls to local function does not require the TOC restore 'nop'
 define void @test_direct() nounwind readnone {
 ; CHECK-LABEL: test_direct:
   tail call void @foo() nounwind
 ; Because of tail call optimization, it can be 'b' instruction.
 ; CHECK: [[BR:b[l]?]] foo
 ; CHECK-NOT: nop
   ret void
 }
 
-; Calls to weak function requires a TOC restore 'nop' because they
-; may be overridden in a different module.
+; Calls to weak function requires a TOC restore 'nop' with the small codemodel
+; because the definition that gets choosen at link time may come from a
+; different section even though we have seen a weak definition in the same
+; section at compile time.
+; With large and medium codemodels no TOC restore is needed, since we know
+; whichever definition is choosen it resides within the same DSO boundaries and
+; therefore shares the same TOC.
 define void @test_weak() nounwind readnone {
-; CHECK-LABEL: test_weak:
   tail call void @foo_weak() nounwind
-; CHECK: bl foo
-; CHECK-NEXT: nop
+; CHECK-LABEL: test_weak:
+; CHECK: b foo_weak
+; CHECK-NOT: nop
+
+; SCM-LABEL: test_weak:
+; SCM:       bl foo_weak
+; SCM-NEXT:  nop
   ret void
 }
 
 ; Indirect calls requires a full stub creation
 define void @test_indirect(void ()* nocapture %fp) nounwind {
 ; CHECK-LABEL: test_indirect:
   tail call void %fp() nounwind
 ; CHECK: ld [[FP:[0-9]+]], 0(3)

llvm/trunk/test/CodeGen/PowerPC/ppc64-sibcall.ll

 ; RUN: llc < %s -relocation-model=static -O1 -disable-ppc-sco=false -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu | FileCheck %s -check-prefix=CHECK-SCO
 ; RUN: llc < %s -relocation-model=static -O1 -disable-ppc-sco=false -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr8 | FileCheck %s -check-prefix=CHECK-SCO-HASQPX
 ; RUN: llc < %s -relocation-model=static -O1 -disable-ppc-sco=false -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr8 | FileCheck %s -check-prefix=CHECK-SCO-HASQPX
+; RUN: llc < %s -relocation-model=static -O1 -disable-ppc-sco=false -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr8 -code-model=small | FileCheck %s -check-prefix=SCM
 
 ; No combination of "powerpc64le-unknown-linux-gnu" + "CHECK-SCO", because
 ; only Power8 (and later) fully support LE.
 
 %S_56 = type { [13 x i32], i32 }
 %S_64 = type { [15 x i32], i32 }
 %S_32 = type { [7 x i32], i32 }
 

 %class.T = type { [2 x i8] }
 
 define weak_odr hidden void @wo_hcallee(%class.T* %this, i8* %c) { ret void }
 define void @wo_hcaller(%class.T* %this, i8* %c) {
   tail call void @wo_hcallee(%class.T* %this, i8* %c)
   ret void
 
 ; CHECK-SCO-LABEL: wo_hcaller:
-; CHECK-SCO: bl wo_hcallee
+; CHECK-SCO: b wo_hcallee
+
+; SCM-LABEL: wo_hcaller:
+; SCM:       bl wo_hcallee
 }
 
 define weak_odr protected void @wo_pcallee(%class.T* %this, i8* %c) { ret void }
 define void @wo_pcaller(%class.T* %this, i8* %c) {
   tail call void @wo_pcallee(%class.T* %this, i8* %c)
   ret void
 
 ; CHECK-SCO-LABEL: wo_pcaller:
-; CHECK-SCO: bl wo_pcallee
+; CHECK-SCO: b wo_pcallee
+
+; SCM-LABEL: wo_pcaller:
+; SCM:       bl wo_pcallee
 }
 
 define weak_odr void @wo_callee(%class.T* %this, i8* %c) { ret void }
 define void @wo_caller(%class.T* %this, i8* %c) {
   tail call void @wo_callee(%class.T* %this, i8* %c)
   ret void
 
 ; CHECK-SCO-LABEL: wo_caller:
-; CHECK-SCO: bl wo_callee
+; CHECK-SCO: b wo_callee
+
+; SCM-LABEL: wo_caller:
+; SCM:       bl wo_callee
 }
 
 define weak protected void @w_pcallee(i8* %ptr) { ret void }
 define void @w_pcaller(i8* %ptr) {
   tail call void @w_pcallee(i8* %ptr)
   ret void
 
 ; CHECK-SCO-LABEL: w_pcaller:
-; CHECK-SCO: bl w_pcallee
+; CHECK-SCO: b w_pcallee
+
+; SCM-LABEL: w_pcaller:
+; SCM:       bl w_pcallee
 }
 
 define weak hidden void @w_hcallee(i8* %ptr) { ret void }
 define void @w_hcaller(i8* %ptr) {
   tail call void @w_hcallee(i8* %ptr)
   ret void
 
 ; CHECK-SCO-LABEL: w_hcaller:
-; CHECK-SCO: bl w_hcallee
+; CHECK-SCO: b w_hcallee
+
+; SCM-LABEL: w_hcaller:
+; SCM:       bl w_hcallee
 }
 
 define weak void @w_callee(i8* %ptr) { ret void }
 define void @w_caller(i8* %ptr) {
   tail call void @w_callee(i8* %ptr)
   ret void
 
 ; CHECK-SCO-LABEL: w_caller:
-; CHECK-SCO: bl w_callee
+; CHECK-SCO: b w_callee
+
+; SCM-LABEL: w_caller:
+; SCM:       bl w_callee
 }
 
 %struct.byvalTest = type { [8 x i8] }
 @byval = common global %struct.byvalTest zeroinitializer
 
 define void @byval_callee(%struct.byvalTest* byval %ptr) { ret void }
 define void @byval_caller() {
   tail call void @byval_callee(%struct.byvalTest* byval @byval)
   ret void
 
 ; CHECK-SCO-LABEL: bl byval_callee
 ; CHECK-SCO: bl byval_callee
 }