public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [AARCH64] Add missing entries in iterator vwcore
@ 2015-10-01 20:41 Kugan
  2015-10-05 10:33 ` James Greenhalgh
  0 siblings, 1 reply; 4+ messages in thread
From: Kugan @ 2015-10-01 20:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: James Greenhalgh, Marcus Shawcroft

[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]

Hi,

In "aarch64_get_lane<mode>" operand 0 is VEL, so  for %<vwcore>0,
iterator vwcore should (?) support all the modes in VEL.

Ran into following error with a local patch for an existing test case.
However it can also be reproduced with the attached test case.

fnction ‘fn1’:
t.c:25:1: internal compiler error: output_operand: invalid %-code
 }
 ^
0x8198fb output_operand_lossage(char const*, ...)
	../../base/gcc/final.c:3417
0x81a45b output_asm_insn(char const*, rtx_def**)
	../../base/gcc/final.c:3782
0x81b9d3 output_asm_insn(char const*, rtx_def**)
	../../base/gcc/final.c:2364
0x81b9d3 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
	../../base/gcc/final.c:3029
0x81be2b final(rtx_insn*, _IO_FILE*, int)
	../../base/gcc/final.c:2058
0x81c6e7 rest_of_handle_final
	../../base/gcc/final.c:4449
0x81c6e7 execute
	../../base/gcc/final.c:4524


Attached patch fixes this. Bootstrapped and regression tested for
aarch64-none-linux-gnu with no new regression. Is this OK for trunk?

Thanks,
Kugan

gcc/ChangeLog:

2015-10-02  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* config/aarch64/iterators.md: Add missing core element mode for
	 mode.

gcc/testsuite/ChangeLog:

2015-10-02  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* gcc.target/aarch64/foo.c: New test.


[-- Attachment #2: p.txt --]
[-- Type: text/plain, Size: 1319 bytes --]

diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index 38c5a24..e49abd5 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -537,8 +537,11 @@
 			   (V4HI "w") (V8HI "w")
 			   (V2SI "w") (V4SI "w")
 			   (DI   "x") (V2DI "x")
+			   (V4HF "w") (V8HF "w")
 			   (V2SF "w") (V4SF "w")
-			   (V2DF "x")])
+			   (V2DF "x") (SI   "x")
+			   (HI   "x") (QI   "x")])
+
 
 ;; Double vector types for ALLX.
 (define_mode_attr Vallxd [(QI "8b") (HI "4h") (SI "2s")])
diff --git a/gcc/testsuite/gcc.target/aarch64/foo.c b/gcc/testsuite/gcc.target/aarch64/foo.c
index e69de29..77f161e 100644
--- a/gcc/testsuite/gcc.target/aarch64/foo.c
+++ b/gcc/testsuite/gcc.target/aarch64/foo.c
@@ -0,0 +1,25 @@
+
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+void fn2 ();
+
+typedef __Float16x4_t float16x4_t;
+__fp16 result_float16x4[1];
+float16x4_t exec_vst1_lane_vector_float16x4, exec_vst1_lane___trans_tmp_1;
+
+void fn1 ()
+{
+  exec_vst1_lane_vector_float16x4 = exec_vst1_lane___trans_tmp_1;
+  __fp16 *__a = result_float16x4;
+  float16x4_t __b = exec_vst1_lane___trans_tmp_1;
+  int __lane = 0;
+  *__a = ({ __b[__lane]; });
+  union {
+      short i;
+      __fp16 f;
+  } tmp_res;
+  tmp_res.f = result_float16x4[0];
+  if (tmp_res.i)
+    fn2();
+}

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [AARCH64] Add missing entries in iterator vwcore
  2015-10-01 20:41 [AARCH64] Add missing entries in iterator vwcore Kugan
@ 2015-10-05 10:33 ` James Greenhalgh
  2015-10-06  1:40   ` Kugan
  0 siblings, 1 reply; 4+ messages in thread
From: James Greenhalgh @ 2015-10-05 10:33 UTC (permalink / raw)
  To: Kugan; +Cc: gcc-patches, Marcus Shawcroft

On Thu, Oct 01, 2015 at 09:41:20PM +0100, Kugan wrote:
> Hi,
> 
> In "aarch64_get_lane<mode>" operand 0 is VEL, so  for %<vwcore>0,
> iterator vwcore should (?) support all the modes in VEL.
> 
> Ran into following error with a local patch for an existing test case.
> However it can also be reproduced with the attached test case.
> 
> fnction ???fn1???:
> t.c:25:1: internal compiler error: output_operand: invalid %-code
>  }
>  ^
> 0x8198fb output_operand_lossage(char const*, ...)
> 	../../base/gcc/final.c:3417
> 0x81a45b output_asm_insn(char const*, rtx_def**)
> 	../../base/gcc/final.c:3782
> 0x81b9d3 output_asm_insn(char const*, rtx_def**)
> 	../../base/gcc/final.c:2364
> 0x81b9d3 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
> 	../../base/gcc/final.c:3029
> 0x81be2b final(rtx_insn*, _IO_FILE*, int)
> 	../../base/gcc/final.c:2058
> 0x81c6e7 rest_of_handle_final
> 	../../base/gcc/final.c:4449
> 0x81c6e7 execute
> 	../../base/gcc/final.c:4524
> 
> 
> Attached patch fixes this. Bootstrapped and regression tested for
> aarch64-none-linux-gnu with no new regression. Is this OK for trunk?
>
> gcc/ChangeLog:
> 
> 2015-10-02  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	* config/aarch64/iterators.md: Add missing core element mode for
> 	 mode.
> 
> gcc/testsuite/ChangeLog:
> 
> 2015-10-02  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	* gcc.target/aarch64/foo.c: New test.
> 

"foo.c" is not OK, please give this testcase a meaningful name.

> diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
> index 38c5a24..e49abd5 100644
> --- a/gcc/config/aarch64/iterators.md
> +++ b/gcc/config/aarch64/iterators.md
> @@ -537,8 +537,11 @@
>  			   (V4HI "w") (V8HI "w")
>  			   (V2SI "w") (V4SI "w")
>  			   (DI   "x") (V2DI "x")
> +			   (V4HF "w") (V8HF "w")
>  			   (V2SF "w") (V4SF "w")
> -			   (V2DF "x")])
> +			   (V2DF "x") (SI   "x")
> +			   (HI   "x") (QI   "x")])

I don't understand the reasoning here, Surely we want "w" for SI,HI,QI
modes? Though are you sure we need them to fix your bug? I'd have expected
the hunk for V4HF and V8HF to be enough.

>  
>  ;; Double vector types for ALLX.
>  (define_mode_attr Vallxd [(QI "8b") (HI "4h") (SI "2s")])
> diff --git a/gcc/testsuite/gcc.target/aarch64/foo.c b/gcc/testsuite/gcc.target/aarch64/foo.c
> index e69de29..77f161e 100644
> --- a/gcc/testsuite/gcc.target/aarch64/foo.c
> +++ b/gcc/testsuite/gcc.target/aarch64/foo.c

Again, please give this test a meaningful name.

Thanks,
James

> @@ -0,0 +1,25 @@
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +void fn2 ();
> +
> +typedef __Float16x4_t float16x4_t;
> +__fp16 result_float16x4[1];
> +float16x4_t exec_vst1_lane_vector_float16x4, exec_vst1_lane___trans_tmp_1;
> +
> +void fn1 ()
> +{
> +  exec_vst1_lane_vector_float16x4 = exec_vst1_lane___trans_tmp_1;
> +  __fp16 *__a = result_float16x4;
> +  float16x4_t __b = exec_vst1_lane___trans_tmp_1;
> +  int __lane = 0;
> +  *__a = ({ __b[__lane]; });
> +  union {
> +      short i;
> +      __fp16 f;
> +  } tmp_res;
> +  tmp_res.f = result_float16x4[0];
> +  if (tmp_res.i)
> +    fn2();
> +}

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [AARCH64] Add missing entries in iterator vwcore
  2015-10-05 10:33 ` James Greenhalgh
@ 2015-10-06  1:40   ` Kugan
  2015-10-06  8:15     ` James Greenhalgh
  0 siblings, 1 reply; 4+ messages in thread
From: Kugan @ 2015-10-06  1:40 UTC (permalink / raw)
  To: James Greenhalgh; +Cc: gcc-patches, Marcus Shawcroft

[-- Attachment #1: Type: text/plain, Size: 3835 bytes --]



On 05/10/15 21:33, James Greenhalgh wrote:
> On Thu, Oct 01, 2015 at 09:41:20PM +0100, Kugan wrote:
>> Hi,
>>
>> In "aarch64_get_lane<mode>" operand 0 is VEL, so  for %<vwcore>0,
>> iterator vwcore should (?) support all the modes in VEL.
>>
>> Ran into following error with a local patch for an existing test case.
>> However it can also be reproduced with the attached test case.
>>
>> fnction ???fn1???:
>> t.c:25:1: internal compiler error: output_operand: invalid %-code
>>  }
>>  ^
>> 0x8198fb output_operand_lossage(char const*, ...)
>> 	../../base/gcc/final.c:3417
>> 0x81a45b output_asm_insn(char const*, rtx_def**)
>> 	../../base/gcc/final.c:3782
>> 0x81b9d3 output_asm_insn(char const*, rtx_def**)
>> 	../../base/gcc/final.c:2364
>> 0x81b9d3 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
>> 	../../base/gcc/final.c:3029
>> 0x81be2b final(rtx_insn*, _IO_FILE*, int)
>> 	../../base/gcc/final.c:2058
>> 0x81c6e7 rest_of_handle_final
>> 	../../base/gcc/final.c:4449
>> 0x81c6e7 execute
>> 	../../base/gcc/final.c:4524
>>
>>
>> Attached patch fixes this. Bootstrapped and regression tested for
>> aarch64-none-linux-gnu with no new regression. Is this OK for trunk?
>>
>> gcc/ChangeLog:
>>
>> 2015-10-02  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>> 	* config/aarch64/iterators.md: Add missing core element mode for
>> 	 mode.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2015-10-02  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>> 	* gcc.target/aarch64/foo.c: New test.
>>
> 
> "foo.c" is not OK, please give this testcase a meaningful name.
> 
>> diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
>> index 38c5a24..e49abd5 100644
>> --- a/gcc/config/aarch64/iterators.md
>> +++ b/gcc/config/aarch64/iterators.md
>> @@ -537,8 +537,11 @@
>>  			   (V4HI "w") (V8HI "w")
>>  			   (V2SI "w") (V4SI "w")
>>  			   (DI   "x") (V2DI "x")
>> +			   (V4HF "w") (V8HF "w")
>>  			   (V2SF "w") (V4SF "w")
>> -			   (V2DF "x")])
>> +			   (V2DF "x") (SI   "x")
>> +			   (HI   "x") (QI   "x")])
> 
> I don't understand the reasoning here, Surely we want "w" for SI,HI,QI
> modes? Though are you sure we need them to fix your bug? I'd have expected
> the hunk for V4HF and V8HF to be enough.

Yes, the hunk for V4HF and V8HF is enough and that is what I started
with. Then I was thinking maybe we should cover all the modes in VEL.
Please find the attached that has just V4HF and V8HF.



> 
>>  
>>  ;; Double vector types for ALLX.
>>  (define_mode_attr Vallxd [(QI "8b") (HI "4h") (SI "2s")])
>> diff --git a/gcc/testsuite/gcc.target/aarch64/foo.c b/gcc/testsuite/gcc.target/aarch64/foo.c
>> index e69de29..77f161e 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/foo.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/foo.c
> 
> Again, please give this test a meaningful name.

Renamed the test case.

Is this OK now?

Thanks,
Kugan



gcc/ChangeLog:

2015-10-06  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* config/aarch64/iterators.md: Add missing core element mode for
	 mode.

gcc/testsuite/ChangeLog:

2015-10-06  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* gcc.target/aarch64/vcore_ice_test.c: New test.



> 
> Thanks,
> James
> 
>> @@ -0,0 +1,25 @@
>> +
>> +/* { dg-do compile } */
>> +/* { dg-options "-O3" } */
>> +
>> +void fn2 ();
>> +
>> +typedef __Float16x4_t float16x4_t;
>> +__fp16 result_float16x4[1];
>> +float16x4_t exec_vst1_lane_vector_float16x4, exec_vst1_lane___trans_tmp_1;
>> +
>> +void fn1 ()
>> +{
>> +  exec_vst1_lane_vector_float16x4 = exec_vst1_lane___trans_tmp_1;
>> +  __fp16 *__a = result_float16x4;
>> +  float16x4_t __b = exec_vst1_lane___trans_tmp_1;
>> +  int __lane = 0;
>> +  *__a = ({ __b[__lane]; });
>> +  union {
>> +      short i;
>> +      __fp16 f;
>> +  } tmp_res;
>> +  tmp_res.f = result_float16x4[0];
>> +  if (tmp_res.i)
>> +    fn2();
>> +}
> 

[-- Attachment #2: p.txt --]
[-- Type: text/plain, Size: 1207 bytes --]

diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index 38c5a24..90e8533 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -537,6 +537,7 @@
 			   (V4HI "w") (V8HI "w")
 			   (V2SI "w") (V4SI "w")
 			   (DI   "x") (V2DI "x")
+			   (V4HF "w") (V8HF "w")
 			   (V2SF "w") (V4SF "w")
 			   (V2DF "x")])
 
diff --git a/gcc/testsuite/gcc.target/aarch64/vcore_ice_test.c b/gcc/testsuite/gcc.target/aarch64/vcore_ice_test.c
index e69de29..77f161e 100644
--- a/gcc/testsuite/gcc.target/aarch64/vcore_ice_test.c
+++ b/gcc/testsuite/gcc.target/aarch64/vcore_ice_test.c
@@ -0,0 +1,25 @@
+
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+void fn2 ();
+
+typedef __Float16x4_t float16x4_t;
+__fp16 result_float16x4[1];
+float16x4_t exec_vst1_lane_vector_float16x4, exec_vst1_lane___trans_tmp_1;
+
+void fn1 ()
+{
+  exec_vst1_lane_vector_float16x4 = exec_vst1_lane___trans_tmp_1;
+  __fp16 *__a = result_float16x4;
+  float16x4_t __b = exec_vst1_lane___trans_tmp_1;
+  int __lane = 0;
+  *__a = ({ __b[__lane]; });
+  union {
+      short i;
+      __fp16 f;
+  } tmp_res;
+  tmp_res.f = result_float16x4[0];
+  if (tmp_res.i)
+    fn2();
+}

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [AARCH64] Add missing entries in iterator vwcore
  2015-10-06  1:40   ` Kugan
@ 2015-10-06  8:15     ` James Greenhalgh
  0 siblings, 0 replies; 4+ messages in thread
From: James Greenhalgh @ 2015-10-06  8:15 UTC (permalink / raw)
  To: Kugan; +Cc: gcc-patches, Marcus Shawcroft

On Tue, Oct 06, 2015 at 02:40:26AM +0100, Kugan wrote:
> 
> 
> On 05/10/15 21:33, James Greenhalgh wrote:
> > On Thu, Oct 01, 2015 at 09:41:20PM +0100, Kugan wrote:
> >> Hi,
> >>
> >> In "aarch64_get_lane<mode>" operand 0 is VEL, so  for %<vwcore>0,
> >> iterator vwcore should (?) support all the modes in VEL.
> >>
> >> Ran into following error with a local patch for an existing test case.
> >> However it can also be reproduced with the attached test case.
> >>
> >> fnction ???fn1???:
> >> t.c:25:1: internal compiler error: output_operand: invalid %-code
> >>  }
> >>  ^
> >> 0x8198fb output_operand_lossage(char const*, ...)
> >> 	../../base/gcc/final.c:3417
> >> 0x81a45b output_asm_insn(char const*, rtx_def**)
> >> 	../../base/gcc/final.c:3782
> >> 0x81b9d3 output_asm_insn(char const*, rtx_def**)
> >> 	../../base/gcc/final.c:2364
> >> 0x81b9d3 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
> >> 	../../base/gcc/final.c:3029
> >> 0x81be2b final(rtx_insn*, _IO_FILE*, int)
> >> 	../../base/gcc/final.c:2058
> >> 0x81c6e7 rest_of_handle_final
> >> 	../../base/gcc/final.c:4449
> >> 0x81c6e7 execute
> >> 	../../base/gcc/final.c:4524
> >>
> >>
> >> Attached patch fixes this. Bootstrapped and regression tested for
> >> aarch64-none-linux-gnu with no new regression. Is this OK for trunk?
> >>
> >> gcc/ChangeLog:
> >>
> >> 2015-10-02  Kugan Vivekanandarajah  <kuganv@linaro.org>
> >>
> >> 	* config/aarch64/iterators.md: Add missing core element mode for
> >> 	 mode.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >> 2015-10-02  Kugan Vivekanandarajah  <kuganv@linaro.org>
> >>
> >> 	* gcc.target/aarch64/foo.c: New test.
> >>
> > 
> > "foo.c" is not OK, please give this testcase a meaningful name.
> > 
> Renamed the test case.
> 
> Is this OK now?

It still doesn't quite look right. For one, the attribute causing the ICE
is "vwcore" not "vcore".

How about calling the test gcc.target/aarch64/get_lane_f16_1.c ?

> gcc/ChangeLog:
> 
> 2015-10-06  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	* config/aarch64/iterators.md: Add missing core element mode for
> 	 mode.

This ChangeLog entry is incomplete:

 	* config/aarch64/iterators.md (vwcore): Add missing cases for
 	 V4HF/V8HF modes.

> 
> gcc/testsuite/ChangeLog:
> 
> 2015-10-06  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	* gcc.target/aarch64/vcore_ice_test.c: New test.
> 

Please remeber to also update this with the new test name I suggested
above.

OK with those changes.

Thanks,
James

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-10-06  8:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-01 20:41 [AARCH64] Add missing entries in iterator vwcore Kugan
2015-10-05 10:33 ` James Greenhalgh
2015-10-06  1:40   ` Kugan
2015-10-06  8:15     ` James Greenhalgh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).