[cfe-dev] stack-less model on small devices (patch)

[Alireza.Moshtaghi at microchip.com]

Thank you Chris,
I'll apply the changes and resubmit a new patch.

Regards.
Ali

-----Original Message-----
From: Chris Lattner [mailto:clattner at apple.com] 
Sent: Friday, May 02, 2008 10:42 AM
To: Alireza Moshtaghi - C13012
Cc: cfe-dev at cs.uiuc.edu
Subject: Re: [cfe-dev] stack-less model on small devices (patch)

On Apr 30, 2008, at 11:31 AM, Alireza.Moshtaghi at microchip.com wrote:
> I created the patch for my target specific modifications and sent it  
> to
> cfe-commits. Since this is my first time to send a patch I don't  
> know if
> I have submitted my changes to the right place or not and of course  
> what
> is the turnaround time.

You did exactly the right thing.  I've been bogged down with other  
things lately and haven't had much time to stay on top of clang, this  
will hopefully be fixed next week, I apologize for the delay.

> I also have attached it to this email just in case.
> Please let me know if I have to do it differently.

The patch looks great.  Some specific comments:

    /// getPointerWidth - Return the width of pointers on this target,  
for the
    /// specified address space. FIXME: implement correctly.
-  uint64_t getPointerWidth(unsigned AddrSpace) const { return 32; }
-  uint64_t getPointerAlign(unsigned AddrSpace) const { return 32; }
+  virtual uint64_t getPointerWidth(unsigned AddrSpace) const { return  
32; }
+  virtual uint64_t getPointerAlign(unsigned AddrSpace) const { return  
32; }

    /// getIntWidth/Align - Return the size of 'signed int' and  
'unsigned int' for
    /// this target, in bits.
-  unsigned getIntWidth() const { return 32; } // FIXME
-  unsigned getIntAlign() const { return 32; } // FIXME
+  virtual unsigned getIntWidth() const { return 32; } // FIXME
+  virtual unsigned getIntAlign() const { return 32; } // FIXME

Instead of making these virtual, please add instance variables for  
these like double and wchar are handled.  You can also remove the  
FIXMEs.  Thanks for doing this.

+++ lib/Basic/Targets.cpp	(working copy)
@@ -863,6 +863,28 @@
+class PIC16TargetInfo : public TargetInfo{
+public:
+  virtual const char *getVAListDeclaration() const { return "";}
+  virtual const char *getClobbers() const {return "";}
+  virtual const char *getTargetPrefix() const {return "";}
+  virtual void getGCCRegNames(const char * const *&Names, unsigned  
&NumNames) const {}
+  virtual bool validateAsmConstraint(char c,  
TargetInfo::ConstraintInfo &info) const {return true;}
+  virtual void getGCCRegAliases(const GCCRegAlias *&Aliases, unsigned  
&NumAliases) const {}
+};
+}

Please make sure the code fits in 80 columns.


+++ lib/CodeGen/CGDecl.cpp	(working copy)
@@ -15,6 +15,7 @@

+    if (strncmp (this->Target.getTargetTriple(), "pic16-", 6) == 0) {
+      const llvm::Type *LTy = CGM.getTypes().ConvertTypeForMem(Ty);

The preferred way to do a target check like this is to add some new  
property to TargetInfo with an accessor like  
"Target.useGlobalsForAutomaticVariables()" or something like that.   
PIC16 can return true, all other targets return false.

Can you just use the code path for static variables to handle the LLVM  
IR emission?  That would avoid duplicating the code.

-Chris