Bug 59843 - ICE with return of generic vector on aarch64
Summary: ICE with return of generic vector on aarch64
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.8.2
: P3 normal
Target Milestone: ---
Assignee: Alan Lawrence
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2014-01-16 14:54 UTC by Jakub Jelinek
Modified: 2014-11-19 10:12 UTC (History)
4 users (show)

See Also:
Host:
Target: aarch64-linux aarch64-elf
Build:
Known to work:
Known to fail:
Last reconfirmed: 2014-01-16 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2014-01-16 14:54:49 UTC
typedef double V __attribute__ ((vector_size (8)));
V
foo (void)
{
  V r = { 3.5 };
  return r;
}

ICEs at all optimization levels on aarch64-linux, trying to return BLKmode using a PARALLEL isn't going to work.  Seen both on the trunk and 4.8 branch.
Comment 1 Andrew Pinski 2014-01-16 16:38:05 UTC
Confirmed.
Comment 2 Yufeng Zhang 2014-01-16 17:25:04 UTC
Confirmed and assigned to myself.

test.c: In function 'foo':
test.c:6:3: internal compiler error: in emit_move_insn, at expr.c:3610
   return r;
   ^
0x63bbb0 emit_move_insn(rtx_def*, rtx_def*)
        gcc/gcc/expr.c:3610
0x63e1a2 emit_group_load_1
        gcc/gcc/expr.c:1750
0x63e5e9 emit_group_load(rtx_def*, rtx_def*, tree_node*, int)
        gcc/gcc/expr.c:1846
0x587c27 expand_value_return
        gcc/gcc/cfgexpand.c:3036
0x58ade6 expand_return
        gcc/gcc/cfgexpand.c:3114
0x58ade6 expand_gimple_stmt_1
        gcc/gcc/cfgexpand.c:3187
0x58ade6 expand_gimple_stmt
        gcc/gcc/cfgexpand.c:3309
0x58c024 expand_gimple_basic_block
        gcc/gcc/cfgexpand.c:5149
0x590536 gimple_expand_cfg
        gcc/gcc/cfgexpand.c:5715
0x590536 execute
        gcc/gcc/cfgexpand.c:5935
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
Comment 3 Yufeng Zhang 2014-03-20 14:46:29 UTC
This issue is related to the handling of vector of one double in the AArch64 backend.  We don't have a corresponding type defined in ACLE (ARM C Language Extension) the spec and backend is not able to handle it properly this moment.
Comment 4 Yufeng Zhang 2014-03-20 19:44:42 UTC
Remove myself from assignee; I'm not working on it this moment.
Comment 5 ktkachov 2014-07-02 13:40:44 UTC
Alan, I think you've fixed this on trunk as part of PR 60825 (at least we don't ICE on trunk).
Can you check whether we need to backport anything and close this off when appropriate?
Comment 6 Alan Lawrence 2014-07-08 10:33:32 UTC
Author: alalaw01
Date: Tue Jul  8 10:32:57 2014
New Revision: 212355

URL: https://gcc.gnu.org/viewcvs?rev=212355&root=gcc&view=rev
Log:
Backport r211502: PR target/59843 Fix arm_neon.h ZIP/UZP/TRN for bigendian

	2014-06-10  Alan Lawrence  <alan.lawrence@arm.com>

	gcc/:
		* config/aarch64/aarch64-modes.def: Add V1DFmode.
		* config/aarch64/aarch64.c (aarch64_vector_mode_supported_p):
		Support V1DFmode.

	gcc/testsuite/:
	       * gcc.dg/vect/vect-singleton_1.c: New file.



Added:
    branches/gcc-4_9-branch/gcc/testsuite/gcc.dg/vect/vect-singleton_1.c
Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/config/aarch64/aarch64-modes.def
    branches/gcc-4_9-branch/gcc/config/aarch64/aarch64.c
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog
Comment 7 Jakub Jelinek 2014-07-09 10:22:09 UTC
Isn't even this change ABI changing?  I know you ICE when returning such vectors, but doesn't the change from BLKmode to V1DFmode change how such vars are laid out in structures, or passed in as arguments, or when present in aggregates (structs/unions) passed by value etc.?
Comment 8 Alan Lawrence 2014-07-09 15:45:39 UTC
Passing a vector of one double as argument also ICE'd prior to this patch, so no worries there. Likewise passing such a vector in as varargs.

Regarding struct layout:

*IIUC, gcc generally lays out structs in memory according to the sizes of the data members, which hasn't changed.

*The AAPCS64 does not mention gcc vector extensions explicitly, only the architectural concept of short vectors, but if we consider gcc vector extensions as mapping to these short vectors (as they have done) - then there is no dependency on gcc's concept of "mode".
 
*I've verified this with some by-hand testcases, using
typedef double gcc_vector_f64x1 __attribute__ ((__vector_size__ ((8))));
passing and returning by value
	*a small struct of a single gcc_vector_f64x1
	*a small struct of a char and a gcc_vector_f64x1
	*a struct of four gcc_vector_f64x1's (classed as a Homogenous Vector Aggregate)
....all passed in registers, and
	*a large struct (of over 16 bytes, containing char, gcc_vector_f64x1, char, gcc_vector_f64x2)
...written to memory and passing a pointer. Also explicitly passing pointers to structs in memory. In all cases the layout (in registers/memory) is the same regardless of whether we have V1DFmode or the gcc_vector_f64x1 is just assigned BLKmode. (FWIW code generation is usually, tho not in all cases, better with V1DFmode.)

Do you believe the ABI has changed, or are you merely concerned that it may have? Whilst I can never claim my checks have been exhaustive, at this point I don't see reason for concern.
Comment 9 Jakub Jelinek 2014-07-09 15:54:25 UTC
I've been just concerned, many backends derive (perhaps incorrectly) passing conventions from DECL_MODE or TYPE_MODE and so any changes in that are a red flag to me.
If passing as arguments and returning the vectors sized the same as element size always ICEd, then it is not an ABI issue, because any old program using those wouldn't compile.  Are the ICEs on both the caller and callee side though?
I mean, which of these tests ICE?
test1.c:
typedef double V __attribute__ ((vector_size (8)));
V foo (void);
V x;
void bar (void)
{
  x = foo ();
}
test2.c:
typedef double V __attribute__ ((vector_size (8)));
void foo (V);
V x;
void bar (void)
{
  foo (x);
}
test3.c:
typedef double V __attribute__ ((vector_size (8)));
V x;
V foo (void)
{
  return x;
}
test4.c:
typedef double V __attribute__ ((vector_size (8)));
V x;
void foo (V y)
{
  x = y;
}

If not all 4 ICE, then perhaps one could compile with 4.9.0 objects containing just the caller (or just the callee) and try to put it together with 4.9.1 built objects.
Comment 10 Alan Lawrence 2014-07-09 16:16:06 UTC
I can confirm that prior to V1DFmode, all four of those cases gave an ICE (in emit_move_insn, via a variety of routes).
Comment 11 Alan Lawrence 2014-11-18 16:40:01 UTC
As I understand it, This is fixed in both 4.9 and 5.0 branch. Is there any more we want to do or can it now be closed?
Comment 12 Jakub Jelinek 2014-11-18 16:43:23 UTC
If this is fixed even on the trunk (#c6 mentions just 4.9), then it is fine to close this IMHO.
Comment 13 Alan Lawrence 2014-11-19 10:12:09 UTC
Fixed on trunk in r211502 and backported to 4.9.