[cfe-commits] [PATCH] Adding TCE target to Clang

Mikael Lepistö lepisto.mikael at gmail.com
Tue Aug 18 00:34:01 PDT 2009


Thanks for the review.

On 17.8.2009, at 22:05, Eli Friedman wrote:

> 2009/8/17 Mikael Lepistö <mikael.lepisto at tut.fi>:
>> Hi,
>>
>> I added the comment as requested in llvm-commits list. Patch adds  
>> support to
>> Clang for compiling C code with TCE compatible type sizes and  
>> alignments.
>> TCE further uses the bitcode to generate target program and processor
>> description. TCE is publicly (currently with MIT licence) available  
>> ASP
>> design environment.
>
> Detailed comments on the patch:
>
> +  class TCETargetInfo : public TargetInfo{
> +  public:
> +    TCETargetInfo(const std::string& triple) : TargetInfo(triple) {
> +      TLSSupported = false;
> +      IntWidth = 32;
> +      LongWidth = LongLongWidth = 32;
>
> Making "long long" 32 bits is not compliant with C99; that said, it's
> your choice.  (I'm double-checking because it looks like you mostly
> copied the PIC16 target, which is an unusual target.)
>
> +      FloatFormat = &llvm::APFloat::IEEEsingle;
> +      DoubleFormat = &llvm::APFloat::IEEEsingle;
>
> Making "double" an IEEE single is not compliant with C99, but again,
> your choice.

Sad but true. Hopefully we can add at least 64bit long long support  
soon.

> +    virtual uint64_t getPointerWidthV(unsigned AddrSpace) const  
> { return 32; }
> +    virtual uint64_t getPointerAlignV(unsigned AddrSpace) const  
> { return 32; }
>
> These are identical to the default implementation.

Removed.

> +    virtual void getTargetDefines(const LangOptions &Opts,
> +                                  std::vector<char> &Defines) const {
> +      Define(Defines, "tce");
>
> This should check something like "if (Opts.GNUMode)", like DefineStd
> does.  Alternatively, you could just use DefineStd here, although that
> has slightly different behavior.

Fixed.

>
> +    virtual const char *getVAListDeclaration() const { return "";}
>
> You don't support using stdarg.h?

Yes, we support that, fixed.

> +    virtual bool useGlobalsForAutomaticVariables() const {return  
> true;}
>
> Do you know what this does?  If so, it's fine, but if not you should  
> remove it.

Fixed, we like' em in stack :)

-Mikael

-------------- next part --------------
A non-text attachment was scrubbed...
Name: TCE-clang-target-with-comment-rc1.patch
Type: application/octet-stream
Size: 3107 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20090818/2af2c605/attachment.obj>
-------------- next part --------------




More information about the cfe-commits mailing list