[llvm-commits] [llvm] r63377 - in /llvm/trunk: include/llvm/CodeGen/AsmPrinter.h include/llvm/Target/TargetAsmInfo.h lib/CodeGen/AsmPrinter/AsmPrinter.cpp lib/Target/PIC16/PIC16AsmPrinter.cpp lib/Target/PIC16/PIC16AsmPrinter.h lib/Target/PIC16/PIC16TargetAsmInfo.cpp lib/Target/PIC16/PIC16TargetAsmInfo.h

[Chris Lattner]

On Jan 29, 2009, at 8:25 PM, Sanjiv Gupta wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=63377&view=rev
> Log:
> Enable emitting of constant values in non-default address space as  
> well. The APIs emitting constants now take an additional parameter  
> signifying the address space in which to emit. The APIs like  
> getData8BitsDirective() etc are made virtual enabling targets to be  
> able to define appropirate directivers for various sizes and address  
> spaces.

Hi Sanjiv,

Thank you for working on this.

> +    // Data directive accessors
> +    //
> +    virtual const char *getData8bitsDirective(unsigned AddrSpace =  
> 0) const {
> +      return Data8bitsDirective;
> +    }

Unfortunately, this significantly pessimizes the common case where  
AddrSpace = 0 by making these trivial functions virtual.  How about  
something like this?:

     // virtual, also protected.
     virtual const char *getData8bitsDirectiveV(unsigned AddrSpace)  
const;

     // nonvirtual, public.
     const char *getData16bitsDirective(unsigned AddrSpace = 0) const {
       return AddrSpace == 0 ? Data16bitsDirective :  
getData8bitsDirectiveV(AddrSpace);
     }

I think this should be a pretty straight-forward change.  What do you  
think?

-Chris