Steve Ellcey <sell...@marvell.com> writes: > This patch fixes a bug with SIMD functions on Aarch64. I found it > while trying to run SPEC with ToT GCC and a glibc that defines vector > math functions for aarch64. When a function is declared with the simd > attribute GCC creates vector clones of that function with the return > and argument types changed to vector types. On Aarch64 the vector > clones are also marked with the aarch64_vector_pcs attribute to signify > that they use an alternate calling convention. Due to a bug in GCC the > non-vector version of the function being cloned was also being marked > with this attribute. > > Because simd_clone_adjust and expand_simd_clones are calling > targetm.simd_clone.adjust (which attached the aarch64_vector_pcs > attribute to the function type) before calling > simd_clone_adjust_return_type (which created a new distinct type tree > for the cloned function) the attribute got attached to both the > 'normal' scalar version of the SIMD function and any vector versions of > the function. The attribute should only be on the vector versions. > > My fix is to call simd_clone_adjust_return_type and create the new type > before calling targetm.simd_clone.adjust which adds the attribute. The > only other platform that this patch could affect is x86 because that is > the only other platform to use targetm.simd_clone.adjust. I did a > bootstrap and gcc test run on x86 (as well as Aarch64) and got no > regressions. > > OK to checkin? > > Steve Ellcey > sell...@marvell.com > > > 2019-07-17 Steve Ellcey <sell...@marvell.com> > > * omp-simd-clone.c (simd_clone_adjust): Call targetm.simd_clone.adjust > after calling simd_clone_adjust_return_type. > (expand_simd_clones): Ditto.
It should be pretty easy to add a test for this, now that we use .variant_pcs to mark symbols with the attribute. > diff --git a/gcc/omp-simd-clone.c b/gcc/omp-simd-clone.c > index caa8da3cba5..6a6b439d146 100644 > --- a/gcc/omp-simd-clone.c > +++ b/gcc/omp-simd-clone.c > @@ -1164,9 +1164,8 @@ simd_clone_adjust (struct cgraph_node *node) > { > push_cfun (DECL_STRUCT_FUNCTION (node->decl)); > > - targetm.simd_clone.adjust (node); > - > tree retval = simd_clone_adjust_return_type (node); > + targetm.simd_clone.adjust (node); > ipa_parm_adjustment_vec adjustments > = simd_clone_adjust_argument_types (node); > > @@ -1737,8 +1736,8 @@ expand_simd_clones (struct cgraph_node *node) > simd_clone_adjust (n); > else > { > - targetm.simd_clone.adjust (n); > simd_clone_adjust_return_type (n); > + targetm.simd_clone.adjust (n); > simd_clone_adjust_argument_types (n); > } > } I don't think this is enough, since simd_clone_adjust_return_type does nothing for functions that return void (e.g. sincos). I think instead aarch64_simd_clone_adjust should do something like: TREE_TYPE (node->decl) = build_distinct_type_copy (TREE_TYPE (node->decl)); But maybe that has consequences that I've not thought about... Thanks, Richard