Re: [AArch64, PATCH 5/5] Improve MOVI handling (Fix invalid assembler bug)

2013-06-04 Thread Marcus Shawcroft
On 3 June 2013 17:35, Ian Bolton  wrote:


> 2013-06-03  Ian Bolton  
>
> gcc/
> * config/aarch64/aarch64.md (*mov_aarch64): Call
> into function to generate MOVI instruction.
> * config/aarch64/aarch64.c (aarch64_simd_container_mode):
> New function.
> (aarch64_preferred_simd_mode): Turn into wrapper.
> (aarch64_output_scalar_simd_mov_immediate): New function.
> * config/aarch64/aarch64-protos.h: Add prototype for above.
>
> testsuite/
> * gcc.target/aarch64/movi_1.c: New test.

OK
/Marcus


[AArch64, PATCH 5/5] Improve MOVI handling (Fix invalid assembler bug)

2013-06-03 Thread Ian Bolton
(This patch is the last of five, where the first 4 did some clean-up and
this one fixes a bug with scalar MOVI.  The bug fix without the clean-up
was particularly ugly!)


GCC will currently generator invalid assembler for MOVI, if the
value in question needs to be shifted.

For example:

  prog.s:270: Error: immediate value out of range -128 to 255
 at operand 2 -- `movi v16.4h,1024'

The correct assembler for the example should be:

  movi v16.4h, 0x4, lsl 8


The fix involves calling into a function to output the instruction,
rather than just leaving for aarch64_print_operand, as is done for
vector immediates.


Regression runs have passed for Linux and bare-metal.


OK for trunk?


Cheers,
Ian


2013-06-03  Ian Bolton  

gcc/
* config/aarch64/aarch64.md (*mov_aarch64): Call
into function to generate MOVI instruction.
* config/aarch64/aarch64.c (aarch64_simd_container_mode):
New function.
(aarch64_preferred_simd_mode): Turn into wrapper.
(aarch64_output_scalar_simd_mov_immediate): New function.
* config/aarch64/aarch64-protos.h: Add prototype for above.

testsuite/
* gcc.target/aarch64/movi_1.c: New test.
diff --git a/gcc/config/aarch64/aarch64-protos.h 
b/gcc/config/aarch64/aarch64-protos.h
index d21a2f5..0dface1 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -148,6 +148,7 @@ bool aarch64_legitimate_pic_operand_p (rtx);
 bool aarch64_move_imm (HOST_WIDE_INT, enum machine_mode);
 bool aarch64_mov_operand_p (rtx, enum aarch64_symbol_context,
enum machine_mode);
+char *aarch64_output_scalar_simd_mov_immediate (rtx, enum machine_mode);
 char *aarch64_output_simd_mov_immediate (rtx, enum machine_mode, unsigned);
 bool aarch64_pad_arg_upward (enum machine_mode, const_tree);
 bool aarch64_pad_reg_upward (enum machine_mode, const_tree, bool);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 001f9c5..0ea05d8 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -5988,32 +5988,57 @@ aarch64_vector_mode_supported_p (enum machine_mode mode)
   return false;
 }
 
-/* Return quad mode as the preferred SIMD mode.  */
+/* Return appropriate SIMD container
+   for MODE within a vector of WIDTH bits.  */
 static enum machine_mode
-aarch64_preferred_simd_mode (enum machine_mode mode)
+aarch64_simd_container_mode (enum machine_mode mode, unsigned width)
 {
+  gcc_assert (width == 64 || width == 128);
   if (TARGET_SIMD)
-switch (mode)
-  {
-  case DFmode:
-return V2DFmode;
-  case SFmode:
-return V4SFmode;
-  case SImode:
-return V4SImode;
-  case HImode:
-return V8HImode;
-  case QImode:
-return V16QImode;
-  case DImode:
-  return V2DImode;
-break;
-
-  default:;
-  }
+{
+  if (width == 128)
+   switch (mode)
+ {
+ case DFmode:
+   return V2DFmode;
+ case SFmode:
+   return V4SFmode;
+ case SImode:
+   return V4SImode;
+ case HImode:
+   return V8HImode;
+ case QImode:
+   return V16QImode;
+ case DImode:
+   return V2DImode;
+ default:
+   break;
+ }
+  else
+   switch (mode)
+ {
+ case SFmode:
+   return V2SFmode;
+ case SImode:
+   return V2SImode;
+ case HImode:
+   return V4HImode;
+ case QImode:
+   return V8QImode;
+ default:
+   break;
+ }
+}
   return word_mode;
 }
 
+/* Return 128-bit container as the preferred SIMD mode for MODE.  */
+static enum machine_mode
+aarch64_preferred_simd_mode (enum machine_mode mode)
+{
+  return aarch64_simd_container_mode (mode, 128);
+}
+
 /* Return the bitmask of possible vector sizes for the vectorizer
to iterate over.  */
 static unsigned int
@@ -7280,6 +7305,18 @@ aarch64_output_simd_mov_immediate (rtx const_vector,
   return templ;
 }
 
+char*
+aarch64_output_scalar_simd_mov_immediate (rtx immediate,
+ enum machine_mode mode)
+{
+  enum machine_mode vmode;
+
+  gcc_assert (!VECTOR_MODE_P (mode));
+  vmode = aarch64_simd_container_mode (mode, 64);
+  rtx v_op = aarch64_simd_gen_const_vector_dup (vmode, INTVAL (immediate));
+  return aarch64_output_simd_mov_immediate (v_op, vmode, 64);
+}
+
 /* Split operands into moves from op[1] + op[2] into op[0].  */
 
 void
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index e1ec48f..458239e 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -774,17 +774,34 @@
 (match_operand:SHORT 1 "general_operand"  " r,M,D,m, 
m,rZ,*w,*w, r,*w"))]
   "(register_operand (operands[0], mode)
 || aarch64_reg_or_zero (operands[1], mode))"
-  "@
-   mov\\t%w0, %w1
-   mov\\t%w0, %1
-   mov