Bug 69504 - XMM register variable ICE with vector extensions
Summary: XMM register variable ICE with vector extensions
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 6.0
: P3 normal
Target Milestone: 7.0
Assignee: Marek Polacek
URL:
Keywords: accepts-invalid, ice-on-invalid-code
Depends on:
Blocks:
 
Reported: 2016-01-26 22:40 UTC by Adam Warner
Modified: 2016-05-24 16:15 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-01-27 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Warner 2016-01-26 22:40:19 UTC
xmm_register_variable_ICE.c:

#include <stdint.h>
#include <stdio.h>

typedef uint8_t u8x16_t __attribute__ ((vector_size (16)));

int main(void) {
  register u8x16_t u8x16 asm ("xmm0");
  for (int i=0; i<16; ++i) printf("%d\n", u8x16[i]);
  return 0;
}


$ gcc-5 --version
gcc-5 (Debian 5.3.1-7) 5.3.1 20160121
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ gcc-5 -O3 xmm_register_variable_ICE.c 
xmm_register_variable_ICE.c: In function ‘main’:
xmm_register_variable_ICE.c:6:5: internal compiler error: in expand_expr_addr_expr_1, at expr.c:7736
 int main(void) {
     ^
0x669f89 expand_expr_addr_expr_1
	../../src/gcc/expr.c:7736
0xd85886 expand_expr_addr_expr
	../../src/gcc/expr.c:7850
0xd85886 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
	../../src/gcc/expr.c:10724
0xd894e0 expand_expr_real(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
	../../src/gcc/expr.c:8018
0xd894e0 expand_expr
	../../src/gcc/expr.h:254
0xd894e0 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier)
	../../src/gcc/expr.c:8235
0xce38f7 expand_gimple_stmt_1
	../../src/gcc/cfgexpand.c:3462
0xce38f7 expand_gimple_stmt
	../../src/gcc/cfgexpand.c:3522
0xcdcb38 expand_gimple_basic_block
	../../src/gcc/cfgexpand.c:5534
0xcdcb38 execute
	../../src/gcc/cfgexpand.c:6152


Also ICEs with latest Debian snapshot:
gcc (Debian 20160105-1) 6.0.0 20160105 (experimental) [trunk revision 232093]
Comment 1 Richard Biener 2016-01-27 08:53:06 UTC
This is because you use the vector extension of indexing a vector with [] but
with variable indexes the FE represents this in a way that takes the address
of the vector variable which isn't possible in this case - it's a register.

    printf ((const char * restrict) "%d\n", (int) *((unsigned char *) &u8x16 + (sizetype) i));

so I fear the solution here is to reject the code with an error or force
the frontend to special-case register vars by going through a temporary,
that is, use ({ u8x16_t tem = u8x16; tem[i]; })

Frontend issue.  If you do sth like

void foo (u8x16_t *);
int main(void) {
    register u8x16_t u8x16 asm ("xmm0");
    foo (&u8x16);
}

the FEs will reject it with

error: address of register variable ‘u8x16’ requested

so I think rejecting this code is a reasonable thing.  Using intrinsics if you
use processor specific registers is a reasonable thing to expect from you here.
Comment 2 Marek Polacek 2016-01-27 09:02:38 UTC
gcc34 / 44 / 45: 
v.c: In function `main':
v.c:8: error: subscripted value is neither array nor pointer

gcc4.6+ ICE.

g++ 4.6 and 4.7 give
v.c: In function ‘int main()’:
v.c:8:50: error: invalid types ‘u8x16_t {aka __vector(16) unsigned char}[int]’ for array subscript

And g++ 4.8 and newer ICE.

So looks like a regression and it seems we want to do what the FEs were aleready doing in the past.
Comment 3 Richard Biener 2016-01-27 09:11:05 UTC
I think at some point the FEs created array-refs here.  I think I suggested
that elsewhere during last stage1 but nobody implemented that ... (now it's
already again quite late).

Thus for vector[i] create

  ARRAY_REF<VIEW_CONVERT <array-type-for-vector-type> (vector), i>

avoiding the need to take the address of vector.

Thus instead of

  bool non_lvalue
    = convert_vector_to_pointer_for_subscript (loc, &array, index);

convert the vector to an array type via a VIEW_CONVERT.  That should work
for both r and lvalues.  Of course it may trigger interesting new paths
should this survive until RTL expansion ;)

The other option is in convert_vector_to_pointer_for_subscript, force
the !lvalue_p path for DECL_HARD_REGISTER *vecp.
Comment 4 Marek Polacek 2016-01-27 09:16:50 UTC
Ok, I'll try that out, but as you say, in the next stage1.
Comment 5 Marek Polacek 2016-04-18 12:09:11 UTC
Better testcase:

typedef int U __attribute__ ((vector_size (16)));

int
foo (int i)
{
  register U u asm ("xmm0");
  return u[i];
}
Comment 6 Richard Biener 2016-05-24 07:56:27 UTC
Author: rguenth
Date: Tue May 24 07:55:56 2016
New Revision: 236630

URL: https://gcc.gnu.org/viewcvs?rev=236630&root=gcc&view=rev
Log:
2016-05-24  Richard Biener  <rguenther@suse.de>

	PR middle-end/70434
	PR c/69504
	c-family/
	* c-common.h (convert_vector_to_pointer_for_subscript): Rename to ...
	(convert_vector_to_array_for_subscript): ... this.
	* c-common.c (convert_vector_to_pointer_for_subscript): Use a
	VIEW_CONVERT_EXPR to an array type.  Rename to ...
	(convert_vector_to_array_for_subscript): ... this.

	cp/
	* expr.c (mark_exp_read): Handle VIEW_CONVERT_EXPR.
	* constexpr.c (cxx_eval_array_reference): Handle indexed
	vectors.
	* typeck.c (cp_build_array_ref): Adjust.

	c/
	* c-typeck.c (build_array_ref): Do not complain about indexing
	non-lvalue vectors.  Adjust for function name change.

	* tree-ssa.c (non_rewritable_mem_ref_base): Make sure to mark
	bases which are accessed with non-invariant indices.
	* gimple-fold.c (maybe_canonicalize_mem_ref_addr): Re-write
	constant index ARRAY_REFs of vectors into BIT_FIELD_REFs.

	* c-c++-common/vector-subscript-4.c: New testcase.
	* c-c++-common/vector-subscript-5.c: Likewise.

Added:
    trunk/gcc/testsuite/c-c++-common/vector-subscript-4.c
    trunk/gcc/testsuite/c-c++-common/vector-subscript-5.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c
    trunk/gcc/c-family/c-common.h
    trunk/gcc/c/ChangeLog
    trunk/gcc/c/c-typeck.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/constexpr.c
    trunk/gcc/cp/expr.c
    trunk/gcc/cp/typeck.c
    trunk/gcc/gimple-fold.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa.c
Comment 7 Richard Biener 2016-05-24 08:56:11 UTC
Fixed on trunk.
Comment 8 Jakub Jelinek 2016-05-24 16:15:07 UTC
Author: jakub
Date: Tue May 24 16:14:34 2016
New Revision: 236647

URL: https://gcc.gnu.org/viewcvs?rev=236647&root=gcc&view=rev
Log:
	PR middle-end/70434
	PR c/69504
	* c-c++-common/vector-subscript-5.c (foo): Move ; out of the ifdef.

Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/c-c++-common/vector-subscript-5.c