Applied.

-
Devang

On May 7, 2007, at 1:15 AM, Duncan Sands wrote:

Hi Reid,

I'm working on PR1146. It remove parameter attributes from function
types and places them on functions can call/invoke instructions.
Consequently, some changes are needed in llvm-gcc. One of those changes
I'm unsure about. In llvm-types.cpp, in the ConvertArgListToFnType
function, we have this pice of code:

 if (static_chain) {
   // Pass the static chain in a register.
   ParamAttrsVector Attrs;
   ParamAttrsWithIndex PAWI; PAWI.index = 1; PAWI.attrs =
ParamAttr::InReg;
   Attrs.push_back(PAWI);
   PAL = ParamAttrsList::get(Attrs);
 }

Obviously this needs to be deleted as a ParamAttrsList is not needed to
instantiate a FunctionType object.  The question is, where does this
need to be moved to. The static chain has something to do with nested
functions, right? So, do we need to move this "inreg" attribute to the
place where the call is actually made?  Where is that?

I think the inreg attribute should not be applied to the static chain
parameter at all.  The attached patch removes it.  This also fixes a
bug in which a nested function returning a struct was getting the inreg attribute on the struct return parameter, and the sret attribute on the
static chain parameter!

I originally marked the static chain inreg because gcc passes it in a
register (ecx on x86). Inreg is not needed for correctness or even for compatibility (nested functions are always internal, so cannot be called from code compiled by another compiler). And I doubt it is even needed
for efficiency - after all, nested functions end up having the fastcc
calling convention because they are internal, which means all the
parameters will be passed in registers anyway!  Marking it inreg is
just trying to second-guess the code generators, and I don't see why
the static chain should be favored more than any other parameter.  A
final comment: gcc is pretty much obliged to pass the static chain in
a register, because it is not in the list of function parameters (unlike in LLVM), so it has to pass it specially. So the fact that gcc passes it in a register is not necessarily a sign that using a register is a good
idea performance wise.

Ciao,

Duncan.
Index: gcc.llvm/gcc/llvm-types.cpp
===================================================================
--- gcc.llvm.orig/gcc/llvm-types.cpp	2007-05-07 09:01:05.000000000 +0200
+++ gcc.llvm/gcc/llvm-types.cpp	2007-05-07 09:04:10.000000000 +0200
@@ -752,17 +752,7 @@
   for (; Args && TREE_TYPE(Args) != void_type_node; Args = TREE_CHAIN(Args))
     ABIConverter.HandleArgument(TREE_TYPE(Args));
 
-  ParamAttrsList *PAL = 0;
-
-  if (static_chain) {
-    // Pass the static chain in a register.
-    ParamAttrsVector Attrs;
-    ParamAttrsWithIndex PAWI; PAWI.index = 1; PAWI.attrs = ParamAttr::InReg;
-    Attrs.push_back(PAWI);
-    PAL = ParamAttrsList::get(Attrs);
-  }
-
-  return FunctionType::get(RetTy, ArgTys, false, PAL);
+  return FunctionType::get(RetTy, ArgTys, false, 0);
 }
 
 const FunctionType *TypeConverter::ConvertFunctionType(tree type,
@@ -843,12 +833,6 @@
   LLVM_TARGET_INIT_REGPARM(lparam, type);
 #endif // LLVM_TARGET_ENABLE_REGPARM
 
-  if (static_chain) {
-    // Pass the static chain in a register.
-    ParamAttrsWithIndex PAWI; PAWI.index = Idx++; PAWI.attrs = ParamAttr::InReg;
-    Attrs.push_back(PAWI);
-  }
-  
   // The struct return attribute must be associated with the first
   // parameter but that parameter may have other attributes too so we set up
   // the first Attributes value here based on struct return. This only works
// RUN: %llvmgcc %s -S -fnested-functions -o - | not grep {inreg *%agg.result}

struct X { int m, n; };

struct X p(int n) {
  struct X c(int m) {
    struct X x;
    x.m = m;
    x.n = n;
    return x;
  }
  return c(n);
}
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

Reply via email to