[llvm-commits] [PATCH llvm, clang llvm-gcc] Add TCE targt to LLVM and CLANG support for generating bytecode to TCE architecture.

[Chris Lattner]

On Aug 12, 2009, at 8:26 AM, Mikael Lepistö wrote:
>> Here is patch, which adds TCE target to llvm, llvm-gcc and clang.  
>> Hopefully we can moving llvm specific stuff from TCE to llvm  
>> reposity step by step. I also added small test to testsuite to  
>> check that tce target works as desired. Tests of llvm and clang  
>> looks to run fine after the patches.
>>
>> Patch basically makes it possible to start using clang for  
>> generating bitcode for TCE system.
>>
>> Mikael Lepistö
>> Tampere University of Technology
>> http://tce.cs.tut.fi
>>
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> Fixed one comment in previous patch and moved configure regeneration  
> to separate patch.

Some initial comments:

+++ lib/Target/TCE/TCETargetAsmInfo.cpp	(revision 0)
@@ -0,0 +1,28 @@
+//===-- TCETargetAsmInfo.cpp - TCE asm properties  
---------------------===//

Please make sure these lines are exactly 80 columns wide, there are  
several examples that aren't.

In TCETargetAsmInfo, please only specify things that differ from the  
default TargetAsmInfo implementation.  There is no need to set  
CommentSTring to "#", for example and others might go away too.

+++ lib/Target/TCE/TargetInfo/TCETargetInfo.cpp	(revision 0)
@@ -0,0 +1,22 @@
+//===-- TCETargetInfo.cpp - TCE Target Implementation -----------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open  
Source
+// License. See LICENSE.TXT for details.
+//
+// 
= 
= 
=---------------------------------------------------------------------- 
===//
+
+#include "llvm/Module.h"
+#include "llvm/Target/TargetRegistry.h"
+

I don't think you need the Module include.  Please prune #includes to  
the minimum.


+class TCESubtarget : public TargetSubtarget {
+
+public:
+
+protected:

Please tidy this up.  I don't think either of these are needed.   
TCESubtarget.h shouldn't need to pull in TM.h


TCEAsmPrinter.cpp is nice and small, please prune the #includes though.

TCETargetMachine.cpp:

+#include <iostream>

Never use <iostream>, this and several other #includes can be removed.



High level question: what value does it add to LLVM to add this  
target?  The only test that you have shows it doesn't work at all  
yet.  If this is just a first step, we should probably wait until  
after the 2.6 release.

-Chris