[cfe-commits] PATCH: Produce static local vars for stack-less devices

[Alireza.Moshtaghi at microchip.com]

This patch is replacement for the previous patch that I sent. 

It incorporates feedback from Chris (see below)

 

Thanks.

 

Alireza Moshtaghi

Senior Software Engineer

Development Systems, Microchip Technology

 

------------------------------------------------------------------------
-------------------------

 

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

 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20080507/41687bca/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: static.local.patch.svn.50716
Type: application/octet-stream
Size: 10120 bytes
Desc: static.local.patch.svn.50716
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20080507/41687bca/attachment.obj>