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

Mikael Lepistö mikael.lepisto at tut.fi
Tue Aug 18 04:12:00 PDT 2009


Oops, I tried to send this from wrong address. Sorry if this comes  
twice to the list.

> 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/7f433efb/attachment.obj>
-------------- next part --------------
>
>



More information about the cfe-commits mailing list