diff mbox

[AARCH64] Add missing entries in iterator vwcore

Message ID 5613268A.6010201@linaro.org
State New
Headers show

Commit Message

Kugan Vivekanandarajah Oct. 6, 2015, 1:40 a.m. UTC
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();
>> +}
>

Comments

James Greenhalgh Oct. 6, 2015, 8:15 a.m. UTC | #1
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
diff mbox

Patch

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();
+}