[llvm-commits] Stack and global alignment enhancement patch

[Chris Lattner]

On Jan 17, 2007, at 12:47 PM, Scott Michel wrote:

> Per Chris' suggestion to submit enhancement patches to llvm- 
> commits,...
>
> - Adds stack and global alignment options to TargetData specification
>   strings.
>
> - Adds minimum stack and global alignment for aggregates.

Hi Scott,

I think I miscommunicated a little bit :(

You have:

+  // Stack type alignments.
+  unsigned char BoolStackAlignment;    // Defaults to BoolAlignment
+  unsigned char ByteStackAlignment;    // Defaults to ByteAlignment
+  unsigned char ShortStackAlignment;   // Defaults to ShortAlignment
+  unsigned char IntStackAlignment;     // Defaults to IntAlignment
+  unsigned char LongStackAlignment;    // Defaults to LongAlignment
+  unsigned char FloatStackAlignment;   // Defaults to FloatAlignment
+  unsigned char DoubleStackAlignment;  // Defaults to DoubleAlignment
+  unsigned char PointerStackAlignment; // Defaults to PointerAlignment
+  unsigned char AggMinStackAlignment;  // Defaults to 4 bytes
+
+  // Global alignments
+  unsigned char BoolGlobalAlignment;   // Defaults to BoolAlignment
+  unsigned char ByteGlobalAlignment;   // Defaults to ByteAlignment
+  unsigned char ShortGlobalAlignment;  // Defaults to ShortAlignment
+  unsigned char IntGlobalAlignment;    // Defaults to IntAlignment
+  unsigned char LongGlobalAlignment;   // Defaults to LongAlignment
+  unsigned char FloatGlobalAlignment;  // Defaults to FloatAlignment
+  unsigned char DoubleGlobalAlignment; // Defaults to DoubleAlignment
+  unsigned char PointerGlobalAlignment;// Defaults to PointerAlignment
+  unsigned char AggMinGlobalAlignment; // Defaults to 4 bytes

I don't think there is any need for this generality.  It seems that a  
target has two sorts of alignments: that imposed by the ABI, and that  
which is really wants things to be aligned at.  The target itself  
doesn't care *where* something is, but if it has the choice, it  
sometimes wants to use a bigger alignment than necessary.

As such, I don't think there is any reason to have separate variables  
for stack and global alignment.  These are both just the, "we can use  
any alignment, so lets use whatever is preferred for the target".   
Why not have one set of XXXPrefAlignment instance variables which can  
be used for both globals and stack objects?

If you'd like to rename the existing IntAlignment values to  
something, I'd suggest IntABIAlignment instead of IntSAAlignment.

Aside from that, your patch looks good.  Updating the X86 backend and  
hack removal is great!

-Chris