[llvm-commits] Slot alignment

[Bill Wendling]

On Jul 1, 2008, at 6:38 PM, Mon P Wang wrote:

> Hi,
>
> The following small patch is to fix a problem when we have to  
> EmitStackConvert.  It is possible that two types of the same size  
> may have different alignment (e.g., users may specify a stricter  
> alignment than native or vector types may have stricter alignment  
> than a scalar of the same size).  If one ever tries to do stack  
> convert of this, we create a stack slot with the alignment of the  
> destination and we can generate a load/store with greater alignment  
> than the stack slot that we have created.  The following patch  
> generates a StackSlot with the minimum alignment necessary for both  
> the source and the destination type.  If you have any comments,  
> please let me know.
>
Hi Mon Ping,

A couple of comments.

Index: include/llvm/CodeGen/SelectionDAG.h
===================================================================
--- include/llvm/CodeGen/SelectionDAG.h	(revision 52999)
+++ include/llvm/CodeGen/SelectionDAG.h	(working copy)
@@ -574,9 +574,11 @@
    void dump() const;

    /// CreateStackTemporary - Create a stack temporary, suitable for  
holding the
-  /// specified value type.
+  /// specified value type.  If minAlign is specified, the slot size  
will have
+  /// at least that alignment.
    SDOperand CreateStackTemporary(MVT VT);
-
+  SDOperand CreateStackTemporary(MVT VT, unsigned minAlign);
+
    /// FoldSetCC - Constant fold a setcc to true or false.
    SDOperand FoldSetCC(MVT VT, SDOperand N1,
                        SDOperand N2, ISD::CondCode Cond);
Index: lib/CodeGen/SelectionDAG/SelectionDAG.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/SelectionDAG.cpp	(revision 52999)
+++ lib/CodeGen/SelectionDAG/SelectionDAG.cpp	(working copy)
@@ -1081,15 +1081,20 @@
  /// CreateStackTemporary - Create a stack temporary, suitable for  
holding the
  /// specified value type.
  SDOperand SelectionDAG::CreateStackTemporary(MVT VT) {
+  return CreateStackTemporary(VT, 1);
+}
+
+SDOperand SelectionDAG::CreateStackTemporary(MVT VT, unsigned  
minAlign) {

It's not necessary to create two functions. Just specify a default  
value for minAlign like so:

+  SDOperand CreateStackTemporary(MVT VT, unsigned minAlign = 1);


+  if (StackAlign < minAlign)
+    StackAlign = minAlign;

You could use std::max here.