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
