[llvm-commits] [PATCH, RFC] Medium code model support for 64-bit PowerPC

[Will Schmidt]

On Thu, 2012-11-15 at 10:51 -0600, Bill Schmidt wrote:
> 
>            From: 
> Bill Schmidt
> <wschmidt at linux.vnet.ibm.com>
>              To: 
> llvm-commits at cs.uiuc.edu
>         Subject: 
> [llvm-commits]
> [PATCH,  RFC]
> Medium code model
> support for
> 64-bit PowerPC
>            Date: 
>  Thu, 15 Nov 2012
>    10:51:49 -0600
> 
> 
>                                                                 Hello,


Hi!    ;-) 

Awesome writeup, just a couple questions and comments throughout. 


> Note that for LDtocL, we generate a new form of LD called LDrs.  This

I initially had a question on the LDrs , but your comment in-code
answered it.   (... LDrs, which is a form of LD with the offset
specified by a SymbolLo ...).



> Index: lib/Target/PowerPC/PPCInstr64Bit.td
> ===================================================================

> @@ -625,6 +629,12 @@ let canFoldAsLoad = 1, PPC970_Unit = 2 in {
>  def LD   : DSForm_1<58, 0, (outs G8RC:$rD), (ins memrix:$src),
>                      "ld $rD, $src", LdStLD,
>                      [(set G8RC:$rD, (load ixaddr:$src))]>, isPPC64;
> +def LDrs : DSForm_1<58, 0, (outs G8RC:$rD), (ins memrs:$src),
> +                    "ld $rD, $src", LdStLD,
> +                    []>, isPPC64;
> +// The following three definitions are selected for small code model
> only.
> +// Otherwise, we need to create two instructions to form a 32-bit
> offset,
> +// so we have a custom matcher for TOC_ENTRY in
> PPCDAGToDAGIsel::Select().

^^ re this comment, i don't see any changes to the following
definitions, so this is just an informational for existing code? 



> +/// lookUpOrCreateTOCEntry -- Given a symbol, look up whether a TOC
> entry
> +/// exists for it.  If not, create one.  Then return a symbol that
> references
> +/// the TOC entry.
> +MCSymbol *PPCAsmPrinter::lookUpOrCreateTOCEntry(MCSymbol *Sym) {
> +
> +  MCSymbol *&TOCEntry = TOC[Sym];
> +
> +  // To avoid name clash check if the name already exists.
> +  while (TOCEntry == 0) {
> +    if (OutContext.LookupSymbol(Twine(MAI->getPrivateGlobalPrefix())
> +
> +                                "C" + Twine(TOCLabelID++)) == 0) {
> +      TOCEntry = GetTempSymbol("C", TOCLabelID);
> +    }
> +  }
> +
> +  return TOCEntry;
> +}

Nice break-out.  :-)

> +
> +    // Change the opcode to LDrs, which is a form of LD with the
> offset
> +    // specified by a SymbolLo.  If the global address is external,

^ It was useful to me to also have LDrs described in the patch
description per my earlier comment.


> Index: lib/Target/PowerPC/PPCISelDAGToDAG.cpp
> ===================================================================
> --- lib/Target/PowerPC/PPCISelDAGToDAG.cpp      (revision 168008)
> +++ lib/Target/PowerPC/PPCISelDAGToDAG.cpp      (working copy)
> @@ -25,6 +25,7 @@
>  #include "llvm/Constants.h"
>  #include "llvm/Function.h"
>  #include "llvm/GlobalValue.h"
> +#include "llvm/GlobalVariable.h"
>  #include "llvm/Intrinsics.h"
>  #include "llvm/Support/Debug.h"
>  #include "llvm/Support/MathExtras.h"
> @@ -1268,7 +1269,49 @@ SDNode *PPCDAGToDAGISel::Select(SDNode *N) {
>                                             Chain), 0);
>      return CurDAG->SelectNodeTo(N, Reg, MVT::Other, Chain);
>    }
> +  case PPCISD::TOC_ENTRY: {
> +    assert (PPCSubTarget.isPPC64() && "Only supported for 64-bit
> ABI");


^ Should that be "64-bit Elf Linux ABI" ? 


Thanks, 
-Will