[Bug target/108910] [12/13 Regression] Further ICE in aarch64_layout_arg
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108910 --- Comment #14 from CVS Commits --- The trunk branch has been updated by Richard Sandiford : https://gcc.gnu.org/g:66946624b96b762985de56444d726a0ebd4e0df5 commit r13-7167-g66946624b96b762985de56444d726a0ebd4e0df5 Author: Richard Sandiford Date: Thu Apr 13 16:57:57 2023 +0100 aarch64: Don't trust TYPE_ALIGN for pointers [PR108910] The aarch64 PCS rules ignore user alignment for scalars and vectors and use the "natural" alignment of the type. GCC tried to calculate that natural alignment using: TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) But as discussed in the PR, it's possible that the main variant of a pointer type is an overaligned type (although that's usually accidental). This isn't known to be a problem for other types, so this patch changes the bare minimum. It might be that we need to ignore TYPE_ALIGN in other cases too. gcc/ PR target/108910 * config/aarch64/aarch64.cc (aarch64_function_arg_alignment): Do not trust TYPE_ALIGN for pointer types; use POINTER_SIZE instead. gcc/testsuite/ PR target/108910 * gcc.dg/torture/pr108910.c: New test.
[Bug target/108910] [12/13 Regression] Further ICE in aarch64_layout_arg
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108910 Jakub Jelinek changed: What|Removed |Added CC||jason at gcc dot gnu.org --- Comment #13 from Jakub Jelinek --- Jason, any thoughts on why we for build_type_attribute_qual_variant call build_distinct_type_copy rather than build_variant_type_copy, which is e.g. what all spots in c-family/c-attribs.cc that need a separate type use? Or could we use build_variant_type_copy say for a subset of types (perhaps integral/pointer/reference/vector/complex types or something similar) and use distinct copies of the rest (function/method types, aggregates)?
[Bug target/108910] [12/13 Regression] Further ICE in aarch64_layout_arg
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108910
--- Comment #12 from CVS Commits ---
The releases/gcc-12 branch has been updated by Jakub Jelinek
:
https://gcc.gnu.org/g:467a789b0c5e50676bb7e65f861b9d79e0c9fe4c
commit r12-9278-g467a789b0c5e50676bb7e65f861b9d79e0c9fe4c
Author: Jakub Jelinek
Date: Wed Mar 1 09:54:52 2023 +0100
lto: Fix up lto_fixup_prevailing_type [PR108910]
Without LTO, TYPE_POINTER_TO/TYPE_REFERENCE_TO chains are only maintained
inside of build_{pointer,reference}_type_for_mode and those routines
ensure that the pointer/reference type added to the chain is really
without any user attributes (unless something would modify the types
in place, but that would be wrong).
Now, LTO adds stuff to these chains in lto_fixup_prevailing_type but
doesn't guarantee that. The testcase in the PR (which I'm not including
for testsuite because when (I hope) the aarch64 backend bug will be fixed,
the testcase would work either way) shows a case where user has
TYPE_USER_ALIGN type with very high alignment, as there aren't enough
pointers to float in the code left that one becomes the prevailing one
(because types with attributes are created with build_distinct_type_copy
and thus their own TYPE_MAIN_VARIANTs), lto_fixup_prevailing_type puts
it into the TYPE_POINTER_TO chain of float and later on during expansion
of __builtin_cexpif expander uses build_pointer_type (float_type_node)
to emit a sincosf call and instead of getting a normal pointer type gets
this non-standard one.
The following patch fixes that by not adding into those chains
types with TYPE_ATTRIBUTES, and for REFERENCE_TYPEs not even with
TYPE_REF_IS_RVALUE - while the C++ FE adds those into those chains,
it always ensures such a type goes immediately after the corresponding
non-TYPE_REF_IS_RVALUE REFERENCE_TYPE with the same
mode/TYPE_REF_CAN_ALIAS_ALL, so LTO would need to ensure that too, but
TYPE_REF_IS_RVALUE types are looked that way only in the C++ FE.
2023-03-01 Jakub Jelinek
PR target/108910
* lto-common.cc (lto_fixup_prevailing_type): Don't add t to
TYPE_POINTER_TO or TYPE_REFERENCE_TO chain if it has
TYPE_ATTRIBUTES or is TYPE_REF_IS_RVALUE.
(cherry picked from commit 9b4f7004a77b10bc403875f56c94f73ef86562d8)
[Bug target/108910] [12/13 Regression] Further ICE in aarch64_layout_arg
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108910
--- Comment #11 from Jakub Jelinek ---
To be exact, it is more complicated than that.
Some types with extra types are created using build_variant_type_copy and so
their TYPE_MAIN_VARIANT is without the attributes.
Example is handle_aligned_attribute -> common_handle_aligned_attribute ->
build_variant_type_copy.
Others are created as build_distinct_type_copy and don't have TYPE_MAIN_VARIANT
without it, e.g. build_type_attribute_variant ->
build_type_attribute_qual_variant -> build_distinct_type_copy.
Compare e.g. the #c0 testcase vs.
extern void foo (float, float *, float *);
void
bar (void *p)
{
typedef float *FP;
typedef FP FP2 __attribute__((aligned (64)));
FP2 q = __builtin_assume_aligned (p, 64);
foo (0.0f, q, q);
}
which doesn't ICE.
#0 build_distinct_type_copy (type=) at
../../gcc/tree.cc:5693
#1 0x00c143bb in build_type_attribute_qual_variant
(otype=, attribute=,
quals=0) at ../../gcc/attribs.cc:1298
#2 0x00c156f0 in build_type_attribute_variant (ttype=, attribute=) at
../../gcc/attribs.cc:1591
#3 0x00c13465 in decl_attributes (node=0x7fffd1f8,
attributes=, flags=1, last_decl=) at
../../gcc/attribs.cc:964
#4 0x00c32a69 in grokdeclarator (declarator=0x3716e40,
declspecs=0x3716cc0, decl_context=NORMAL, initialized=true, width=0x0,
decl_attrs=0x7fffd3d0,
expr=0x7fffd3f0, expr_const_operands=0x7fffd1ef,
deprecated_state=DEPRECATED_NORMAL) at ../../gcc/c/c-decl.cc:6967
#5 0x00c2d270 in start_decl (declarator=0x3716e70,
declspecs=0x3716cc0, initialized=true, attributes=, do_push=true,
lastloc=0x0) at ../../gcc/c/c-decl.cc:5364
on #c0 while this one:
#1 0x0191494b in build_variant_type_copy (type=) at ../../gcc/tree.cc:5727
#2 0x00dbaeba in common_handle_aligned_attribute (node=0x7fffd270,
name=, args=,
flags=0,
no_add_attrs=0x7fffd28f, warn_if_not_aligned_p=false) at
../../gcc/c-family/c-attribs.cc:2386
#3 0x00dbb633 in handle_aligned_attribute (node=0x7fffd270,
name=, args=,
flags=0,
no_add_attrs=0x7fffd28f) at ../../gcc/c-family/c-attribs.cc:2499
#4 0x00c12dcd in decl_attributes (node=0x7fffd3f8,
attributes=, flags=0, last_decl=) at
../../gcc/attribs.cc:878
#5 0x00c2d1bc in c_decl_attributes (node=0x7fffd3f8,
attributes=, flags=0) at ../../gcc/c/c-decl.cc:5323
#6 0x00c2d699 in start_decl (declarator=0x3716f60,
declspecs=0x3716ea0, initialized=false, attributes=,
do_push=true, lastloc=0x7fffd584)
at ../../gcc/c/c-decl.cc:5460
[Bug target/108910] [12/13 Regression] Further ICE in aarch64_layout_arg
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108910 --- Comment #10 from Jakub Jelinek --- (In reply to Christophe Lyon from comment #9) > So is it expected that the alignment of TYPE_MAIN_VARIANT(type) is 512? Yes, it is. aligned attribute on non-aggregate types create a distinct type, and TYPE_MAIN_VARIANT of a distinct type is the non-qualified variant of such a distinct type. For structures/unions, I believe we disallow creation of less/more aligned variants of the original types, one needs to supply the alignment when defining the structure/union type.
[Bug target/108910] [12/13 Regression] Further ICE in aarch64_layout_arg
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108910 Christophe Lyon changed: What|Removed |Added CC||rearnsha at gcc dot gnu.org --- Comment #9 from Christophe Lyon --- Looking at #c6 again, the behavior of aarch64_function_arg_alignment does not seem to match the comment preceding it. IIUC, it should ignore user alignment. So is it expected that the alignment of TYPE_MAIN_VARIANT(type) is 512? (ie. is the bug in aarch64_function_arg_alignment or earlier, when we build "type"?)
[Bug target/108910] [12/13 Regression] Further ICE in aarch64_layout_arg
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108910 --- Comment #8 from Jakub Jelinek --- The above commit fixed just the #c4 testcase, not the #c0 one.
[Bug target/108910] [12/13 Regression] Further ICE in aarch64_layout_arg
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108910
--- Comment #7 from CVS Commits ---
The master branch has been updated by Jakub Jelinek :
https://gcc.gnu.org/g:9b4f7004a77b10bc403875f56c94f73ef86562d8
commit r13-6385-g9b4f7004a77b10bc403875f56c94f73ef86562d8
Author: Jakub Jelinek
Date: Wed Mar 1 09:54:52 2023 +0100
lto: Fix up lto_fixup_prevailing_type [PR108910]
Without LTO, TYPE_POINTER_TO/TYPE_REFERENCE_TO chains are only maintained
inside of build_{pointer,reference}_type_for_mode and those routines
ensure that the pointer/reference type added to the chain is really
without any user attributes (unless something would modify the types
in place, but that would be wrong).
Now, LTO adds stuff to these chains in lto_fixup_prevailing_type but
doesn't guarantee that. The testcase in the PR (which I'm not including
for testsuite because when (I hope) the aarch64 backend bug will be fixed,
the testcase would work either way) shows a case where user has
TYPE_USER_ALIGN type with very high alignment, as there aren't enough
pointers to float in the code left that one becomes the prevailing one
(because types with attributes are created with build_distinct_type_copy
and thus their own TYPE_MAIN_VARIANTs), lto_fixup_prevailing_type puts
it into the TYPE_POINTER_TO chain of float and later on during expansion
of __builtin_cexpif expander uses build_pointer_type (float_type_node)
to emit a sincosf call and instead of getting a normal pointer type gets
this non-standard one.
The following patch fixes that by not adding into those chains
types with TYPE_ATTRIBUTES, and for REFERENCE_TYPEs not even with
TYPE_REF_IS_RVALUE - while the C++ FE adds those into those chains,
it always ensures such a type goes immediately after the corresponding
non-TYPE_REF_IS_RVALUE REFERENCE_TYPE with the same
mode/TYPE_REF_CAN_ALIAS_ALL, so LTO would need to ensure that too, but
TYPE_REF_IS_RVALUE types are looked that way only in the C++ FE.
2023-03-01 Jakub Jelinek
PR target/108910
* lto-common.cc (lto_fixup_prevailing_type): Don't add t to
TYPE_POINTER_TO or TYPE_REFERENCE_TO chain if it has
TYPE_ATTRIBUTES or is TYPE_REF_IS_RVALUE.
[Bug target/108910] [12/13 Regression] Further ICE in aarch64_layout_arg
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108910 Christophe Lyon changed: What|Removed |Added CC||rsandifo at gcc dot gnu.org --- Comment #6 from Christophe Lyon --- I can see that aarch64_function_arg_alignment has: if (!AGGREGATE_TYPE_P (type)) return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)); which returns 512 with the testcase from comment #0 type is (TYPE_MAIN_VARIANT(type) is the same): unit-size align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x776222a0 precision:32 pointer_to_this > sizes-gimplified public unsigned DI size constant 64> unit-size constant 8> user align:512 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x77622888 attributes value >>>
[Bug target/108910] [12/13 Regression] Further ICE in aarch64_layout_arg
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108910 --- Comment #5 from Jakub Jelinek --- Created attachment 54544 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54544=edit gcc13-pr108910-lto.patch Untested fix for the lto side of this bug. No testcase included, as when the target bug is fixed, the lto testcase will not fail regardless of whether this fix is in or not.
[Bug target/108910] [12/13 Regression] Further ICE in aarch64_layout_arg
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108910
--- Comment #4 from Jakub Jelinek ---
There is another bug, in darktable actually such overaligned pointer isn't
passed, but it is cvise reduced into:
$ cat color_picker.c
void
bar (void)
{
float *__attribute__((aligned(64))) x;
}
$ cat color_vocabulary.c
int a, b, c;
void
foo (void)
{
c = __builtin_cosf (2.0f * 3.14159265358979324f * a);
b = __builtin_sinf (2.0f * 3.14159265358979324f * a);
}
$ ./xgcc -B ./ -c -flto=auto -ffat-lto-objects -g -O3 -ffast-math
-fno-finite-math-only -fPIC -fexpensive-optimizations color_picker.c
$ ./xgcc -B ./ -c -flto=auto -ffat-lto-objects -g -O3 -ffast-math
-fno-finite-math-only -fPIC -fexpensive-optimizations color_vocabulary.c
$ ./xgcc -B ./ -flto=auto -ffat-lto-objects -o color_picker -r color_picker.o
color_vocabulary.o
during RTL pass: expand
color_vocabulary.c: In function ‘foo’:
color_vocabulary.c:4:1: internal compiler error: in aarch64_layout_arg, at
config/aarch64/aarch64.cc:7688
4 | foo (void)
| ^
0x118d493 aarch64_layout_arg
../../gcc/config/aarch64/aarch64.cc:7688
0x118dd7a aarch64_function_arg
../../gcc/config/aarch64/aarch64.cc:7868
0x92080b initialize_argument_information
../../gcc/calls.cc:1499
0x925273 expand_call(tree_node*, rtx_def*, int)
../../gcc/calls.cc:2973
0x90a2da expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)
../../gcc/builtins.cc:8452
0xa5ecfe expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
../../gcc/expr.cc:11872
0x8f638d expand_builtin_cexpi
../../gcc/builtins.cc:2685
0x9077ee expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)
../../gcc/builtins.cc:7476
0xa5ecfe expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
../../gcc/expr.cc:11872
0xa694b5 store_expr(tree_node*, rtx_def*, int, bool, bool)
../../gcc/expr.cc:6333
0xa6bbf0 expand_assignment(tree_node*, tree_node*, bool)
../../gcc/expr.cc:6051
0x93866a expand_call_stmt
../../gcc/cfgexpand.cc:2829
0x93866a expand_gimple_stmt_1
../../gcc/cfgexpand.cc:3880
0x93866a expand_gimple_stmt
../../gcc/cfgexpand.cc:4044
0x93e0c6 expand_gimple_basic_block
../../gcc/cfgexpand.cc:6096
0x93fa67 execute
../../gcc/cfgexpand.cc:6831
Please submit a full bug report, with preprocessed source (by using
-freport-bug).
Please include the complete backtrace with any bug report.
See
[Bug target/108910] [12/13 Regression] Further ICE in aarch64_layout_arg
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108910
--- Comment #3 from Jakub Jelinek ---
As I've tried to explain in the past, C/C++ considers float * and float
*__attribute__((aligned (64))) types to be compatible, similarly to int and int
__attribute__((aligned (64))), so in calling convention one should ignore
alignment on scalar types of passed arguments at least, because they can be
pretty much random.
Though
extern void foo (long, long, long);
void baz (void *);
void
bar (void *p)
{
long __attribute__((aligned (64))) q = 1;
asm volatile ("" : "+r" (q));
foo (0.0L, q, q);
}
doesn't ICE.
[Bug target/108910] [12/13 Regression] Further ICE in aarch64_layout_arg
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108910 Andrew Pinski changed: What|Removed |Added Target Milestone|13.0|12.3 Summary|[13 Regression] Further ICE |[12/13 Regression] Further |in aarch64_layout_arg |ICE in aarch64_layout_arg --- Comment #2 from Andrew Pinski --- > ICEs on aarch64-linux with -O2 likely since r13-5124 Which has been backported to the GCC 12 branch too.
