The following testcase used to work in 4.1.1 but no longer does: #define vector __attribute__((vector_size(16) )) template<int INDEX> vector signed int MyFunction(vector float value) {} template<int> void MyFunction(float ){} int main() { vector float myVector; float myFloat; MyFunction<1>(myVector); MyFunction<1>(myFloat); } ---- CUT ---- We get: vector14.C: In function ‘int main()’: vector14.C:12: error: no matching function for call to ‘MyFunction(float __vector__&)’ vector14.C:13: error: call of overloaded ‘MyFunction(float&)’ is ambiguous vector14.C:3: note: candidates are: int MyFunction(float) [with int INDEX = 1] vector14.C:6: note: void MyFunction(float) [with int <anonymous> = 1]
This is caused by the delaying of vector_size attribute inside templates.
This blocks me from even thinking about updating the PS3 toolchain to 4.3.0.
This is related to PR 27433 and I bet fixing this one will also fix that one.
It worked with "4.3.0 20070623" and "4.2.0 20061019".
Confirmed.
So what is happening is we are not applying the attribute while doing overload resolution so we reject the template function then.
That being said, I think fn_type_unification should apply the attributes but I don't know/understand the template part of the C++ front-end.
I think this shows that "vector_size" attribute can't be a late template attribute whenever processing_template_decl, it can be only a late template attribute if the decl is actually type or value dependent. So I believe we need to remove the vector_size special case from is_late_template_attribute, but then either something like the http://gcc.gnu.org/ml/gcc-patches/2008-01/msg01020.html (second patch) hack to reconstruct the types, or use a langhook in handle_vector_size_attribute for reconstruct_complex_type (defaulting to tree.c reconstruct_complex_type everywhere but in C++).
Testing a patch with a langhook.
See http://gcc.gnu.org/ml/gcc-patches/2008-04/msg00177.html for details. I guess before moving further along with this, attributes for parameter packs should be decided upon. Do we want to allow them at all? What actually will they mean, when they in fact change the type? #define vector __attribute__((__vector_size__ (16))) template <typename... T> void foo (int x, vector T... y) { } void bar (vector long a, vector double b, vector long c, vector double d) { foo<long, double> (5, a, b, c, d); foo<long, double, long, double> (6, a, b, c, d); foo<long, double> (7, 17, 18.0, 19L, 20.0); } ATM g++ will grok just the last foo call (and incorrectly, as the foo function will expect to get int and 4 vectors as arguments, while the caller passes 5 scalars).
Actually, to clarify #c10, attributes on parameter packs just make things harder on the compiler side, but even in C++98 the same issue is present: #define vector __attribute__((__vector_size__ (16))) template <typename T> void foo (int x, vector T y) { } void bar (vector long a, vector double b) { foo<long> (5, a); foo (5, b); } If we apply late template attributes in fn_type_unification, then the foo<long> (5, a); call will work as expected, but should the second call deduce template parameter double or vector double? If the latter (which would be weird), then it will fail, because vector_size attribute can't be applied to a VECTOR_TYPE. Are there any attributes other than vector_size which affect the decls similarly? If not, I'd say that the C++ FE should hardcode some knowledge about this attribute, e.g. know that it applies to the type, so if processing_template_decl move them from DECL_ATTRIBUTES to corresponding type's TYPE_ATTRIBUTES (either the parameter type such that it would be in TYPE_ARG_TYPES too, or for FUNCTION_TYPE/METHOD_TYPE stick it into return type's TYPE_ATTRIBUTES). And in type_unification_real take it into account.
Subject: Re: [4.3/4.4 Regression] vector_size attribute lost in function arguments for templates jakub at gcc dot gnu dot org wrote: > Actually, to clarify #c10, attributes on parameter packs just make things > harder on the compiler side, but even in C++98 the same issue is present: > #define vector __attribute__((__vector_size__ (16))) > > template <typename T> void foo (int x, vector T y) { } > void bar (vector long a, vector double b) > { > foo<long> (5, a); > foo (5, b); > } This functionality seems desirable, but cannot be considered a regression. > Are there any attributes other than vector_size which affect the decls > similarly? I don't think so. > If not, I'd say that the C++ FE should hardcode some knowledge about this > attribute, e.g. know that it applies to the type, so if > processing_template_decl > move them from DECL_ATTRIBUTES to corresponding type's TYPE_ATTRIBUTES (either > the parameter type such that it would be in TYPE_ARG_TYPES too, or for > FUNCTION_TYPE/METHOD_TYPE stick it into return type's TYPE_ATTRIBUTES). > And in type_unification_real take it into account. This makes sense to me. Though I think that if we just push the attribute down to the type that it actually modifies, we don't have to think about TYPE_ARG_TYPES. That should also avoid the need for reconstruct_complex_type. Jason
We also need to make sure the target specific attribute __altivec__ (powerPC) and __spu_vector__ (SPU) works correctly just as vector_size works.
Hmm, for some reason __spu_vector__ works correctly for my testcase but not __altivec__; I have not looked into why though.
I've tried the first step - putting the attributes that require type to TYPE_ATTRIBUTES rather than DECL_ATTRIBUTES, see below. Unfortunately tsubst doesn't call apply_late_template_attributes in that case (the only place which calls it on TYPE_ATTRIBUTES is instantiate_class_template on the fn type). So I'm giving up on this. The patch was: --- decl2.c.jj 2008-03-25 23:31:25.000000000 +0100 +++ decl2.c 2008-04-07 12:40:19.000000000 +0200 @@ -1041,12 +1041,13 @@ is_late_template_attribute (tree attr, t at instantiation time. */ static tree -splice_template_attributes (tree *attr_p, tree decl) +splice_template_attributes (tree *attr_p, tree *type_attr_p, tree decl) { tree *p = attr_p; tree late_attrs = NULL_TREE; tree *q = &late_attrs; + *type_attr_p = NULL_TREE; if (!p) return NULL_TREE; @@ -1054,11 +1055,28 @@ splice_template_attributes (tree *attr_p { if (is_late_template_attribute (*p, decl)) { - ATTR_IS_DEPENDENT (*p) = 1; - *q = *p; + const struct attribute_spec *spec + = lookup_attribute_spec (TREE_PURPOSE (*p)); + + /* Put attributes that require type into TYPE_ATTRIBUTES, + rather than DECL_ATTRIBUTES. */ + if (DECL_P (decl) + && spec + && spec->type_required + && !CLASS_TYPE_P (TREE_TYPE (decl))) + { + *type_attr_p = *p; + type_attr_p = &TREE_CHAIN (*type_attr_p); + *type_attr_p = NULL_TREE; + } + else + { + ATTR_IS_DEPENDENT (*p) = 1; + *q = *p; + q = &TREE_CHAIN (*q); + *q = NULL_TREE; + } *p = TREE_CHAIN (*p); - q = &TREE_CHAIN (*q); - *q = NULL_TREE; } else p = &TREE_CHAIN (*p); @@ -1071,12 +1089,17 @@ splice_template_attributes (tree *attr_p DECL_P. */ static void -save_template_attributes (tree *attr_p, tree *decl_p) +save_template_attributes (tree *attr_p, tree *decl_p, int flags) { - tree late_attrs = splice_template_attributes (attr_p, *decl_p); + tree type_attrs; + tree late_attrs = splice_template_attributes (attr_p, &type_attrs, *decl_p); tree *q; tree old_attrs = NULL_TREE; + if (type_attrs) + cplus_decl_attributes (&TREE_TYPE (*decl_p), type_attrs, + flags & ~(int) ATTR_FLAG_TYPE_IN_PLACE); + if (!late_attrs) return; @@ -1135,7 +1158,7 @@ cplus_decl_attributes (tree *decl, tree if (check_for_bare_parameter_packs (attributes)) return; - save_template_attributes (&attributes, decl); + save_template_attributes (&attributes, decl, flags); if (attributes == NULL_TREE) return; }
Especially for 4.3.x getting rid of the "HACK. GROSS. This is absolutely disgusting." might not be something desirable for a stable branch. We've lived with this gross absolutely disgusting hack for more than 7 years, and the: http://gcc.gnu.org/ml/gcc-patches/2008-04/msg00630.html patch doesn't make the hack IMHO any more disgusting than it was till now, but fixes an important regression.
Subject: Bug 35758 Author: jakub Date: Thu Apr 24 16:29:40 2008 New Revision: 134639 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=134639 Log: PR c++/35758 * c-common.c (handle_vector_size_attribute): Call lang_hooks.types.reconstruct_complex_type instead of reconstruct_complex_type. * config/rs6000/rs6000.c (rs6000_handle_altivec_attribute): Likewise. * config/spu/spu.c (spu_handle_vector_attribute): Likewise. * langhooks.h (struct lang_hooks_for_types): Add reconstruct_complex_type hook. * langhooks-def.h (LANG_HOOKS_RECONSTRUCT_COMPLEX_TYPE): Define. (LANG_HOOKS_FOR_TYPES_INITIALIZER): Add it. * cp-tree.h (cp_reconstruct_complex_type): New prototype. * cp-objcp-common.h (LANG_HOOKS_RECONSTRUCT_COMPLEX_TYPE): Define. * decl2.c (is_late_template_attribute): Only make vector_size late tmpl attribute if argument is type or value dependent. (cp_reconstruct_complex_type): New function. * g++.dg/ext/vector14.C: New test. Added: trunk/gcc/testsuite/g++.dg/ext/vector14.C Modified: trunk/gcc/ChangeLog trunk/gcc/c-common.c trunk/gcc/config/rs6000/rs6000.c trunk/gcc/config/spu/spu.c trunk/gcc/cp/ChangeLog trunk/gcc/cp/cp-objcp-common.h trunk/gcc/cp/cp-tree.h trunk/gcc/cp/decl2.c trunk/gcc/langhooks-def.h trunk/gcc/langhooks.h trunk/gcc/testsuite/ChangeLog
Subject: Bug 35758 Author: jakub Date: Thu Apr 24 16:31:59 2008 New Revision: 134640 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=134640 Log: PR c++/35758 * c-common.c (handle_vector_size_attribute): Call lang_hooks.types.reconstruct_complex_type instead of reconstruct_complex_type. * config/rs6000/rs6000.c (rs6000_handle_altivec_attribute): Likewise. * config/spu/spu.c (spu_handle_vector_attribute): Likewise. * langhooks.h (struct lang_hooks_for_types): Add reconstruct_complex_type hook. * langhooks-def.h (LANG_HOOKS_RECONSTRUCT_COMPLEX_TYPE): Define. (LANG_HOOKS_FOR_TYPES_INITIALIZER): Add it. * cp-tree.h (cp_reconstruct_complex_type): New prototype. * cp-objcp-common.h (LANG_HOOKS_RECONSTRUCT_COMPLEX_TYPE): Define. * decl2.c (is_late_template_attribute): Only make vector_size late tmpl attribute if argument is type or value dependent. (cp_reconstruct_complex_type): New function. * g++.dg/ext/vector14.C: New test. Added: branches/gcc-4_3-branch/gcc/testsuite/g++.dg/ext/vector14.C Modified: branches/gcc-4_3-branch/gcc/ChangeLog branches/gcc-4_3-branch/gcc/c-common.c branches/gcc-4_3-branch/gcc/config/rs6000/rs6000.c branches/gcc-4_3-branch/gcc/config/spu/spu.c branches/gcc-4_3-branch/gcc/cp/ChangeLog branches/gcc-4_3-branch/gcc/cp/cp-objcp-common.h branches/gcc-4_3-branch/gcc/cp/cp-tree.h branches/gcc-4_3-branch/gcc/cp/decl2.c branches/gcc-4_3-branch/gcc/langhooks-def.h branches/gcc-4_3-branch/gcc/langhooks.h branches/gcc-4_3-branch/gcc/testsuite/ChangeLog
With the changes checked in, I believe there is no regression anymore, but using vector_size, altivec or spu_vector attributes on template parameters will still do bad and unexpected things.
4.3.1 is being released, adjusting target milestone.
Comment says "no regression", removing milestone.
Downgrading priority/severity since this is no longer a regression.
The test case compiles successfully with 6.0 (albeit with warnings). Since the originally reported problem was fixed in r134639 and since there are other bugs tracking problems with dependent arguments to attributes (bug 58109 and bug 69022), should this bug be RESOLVED as FIXED? If there is an outstanding problem that isn't tracked anywhere else, can we distill it into a small test case and open a separate bug for it? (I'd be happy to do it but I'm not sure what the outstanding problem is that this bug still tracks.) Setting Status to WAITING until a decision is made.
(In reply to Martin Sebor from comment #23) > The test case compiles successfully with 6.0 (albeit with warnings). Since > the originally reported problem was fixed in r134639 and since there are > other bugs tracking problems with dependent arguments to attributes (bug > 58109 and bug 69022), should this bug be RESOLVED as FIXED? > I'd say so at least. > If there is an outstanding problem that isn't tracked anywhere else, can we > distill it into a small test case and open a separate bug for it? (I'd be > happy to do it but I'm not sure what the outstanding problem is that this > bug still tracks.) > > Setting Status to WAITING until a decision is made.