diff mbox

[AARCH64] movi type attribute confusion

Message ID CABXYE2VR5FzZkbRSVg=sQWMZtatWsGTNbsYeZFugZiKfr4y_Rw@mail.gmail.com
State New
Headers show

Commit Message

Jim Wilson June 12, 2015, 8:43 p.m. UTC
We have 5 patterns that can emit the movi instruction.  These patterns
map it to 4 different type attributes.  The mov<mode>_aarch64 pattern
uses mov_imm.  The movdi_aarch64 pattern uses fmov.  The movtf_aarch64
pattern uses fconstd.  And the two aarch64_simd_mov<mode> patterns for
VD and VQ use neon_move.  Bitwise identical instructions should always
map to the same attribute type, so we need to change these patterns to
agree on the right attribute.  movi is an integer simd instruction, so
neon_move is the only choice that makes sense.  The following patch
corrects the first 3 patterns to use neon_move like the last two.

We could optionally create a new type attribute, e.g. neon_move_imm.
I can do that if people think it would be better.

This patch was tested with a make bootstrap and make check on an APM
box running Ubuntu 14.04.

FYI This patch overlaps with my movtf-zero patch which is still
waiting review, but the overlap is trivial to resolve so this should
not be a problem.

Jim

Comments

Marcus Shawcroft June 15, 2015, 12:05 p.m. UTC | #1
On 12 June 2015 at 21:43, Jim Wilson <jim.wilson@linaro.org> wrote:
> We have 5 patterns that can emit the movi instruction.  These patterns
> map it to 4 different type attributes.  The mov<mode>_aarch64 pattern
> uses mov_imm.  The movdi_aarch64 pattern uses fmov.  The movtf_aarch64
> pattern uses fconstd.  And the two aarch64_simd_mov<mode> patterns for
> VD and VQ use neon_move.  Bitwise identical instructions should always
> map to the same attribute type, so we need to change these patterns to
> agree on the right attribute.  movi is an integer simd instruction, so
> neon_move is the only choice that makes sense.  The following patch
> corrects the first 3 patterns to use neon_move like the last two.
>
> We could optionally create a new type attribute, e.g. neon_move_imm.
> I can do that if people think it would be better.
>
> This patch was tested with a make bootstrap and make check on an APM
> box running Ubuntu 14.04.
>
> FYI This patch overlaps with my movtf-zero patch which is still
> waiting review, but the overlap is trivial to resolve so this should
> not be a problem.

The movdi* and movtf* hunks look fine.  The mov<mode>_aarch64 pattern
calls aarch64_output_scalar_simd_mov_immediate which can emit either
mvni, movi, with or without msl and lsl.  In the case of the plain
movi neon_move looks sensible, the other possbile outputs should
ideally be represented by logic_immediate and logic_shift_immediate.
Using neon_move is a step in the right direction.  OK /Marcus
diff mbox

Patch

2015-06-12  Jim Wilson  <jim.wilson@linaro.org>

	* config/aarch64/aarch64.md (mov<mode>_aarch64): Change alternative 2
	to use neon_move instead of mov_imm.
	(movdi_aarch64): Change alternative 14 to use neon_move not fmov.
	(movtf_aarch64): Change alternative 4 to use neon_move_q not fconstd.

Index: config/aarch64/aarch64.md
===================================================================
--- config/aarch64/aarch64.md	(revision 224441)
+++ config/aarch64/aarch64.md	(working copy)
@@ -827,7 +827,7 @@  (define_insn "*mov<mode>_aarch64"
        gcc_unreachable ();
      }
 }
-  [(set_attr "type" "mov_reg,mov_imm,mov_imm,load1,load1,store1,store1,\
+  [(set_attr "type" "mov_reg,mov_imm,neon_move,load1,load1,store1,store1,\
                      neon_to_gp<q>,neon_from_gp<q>,neon_dup")
    (set_attr "simd" "*,*,yes,*,*,*,*,yes,yes,yes")]
 )
@@ -912,7 +912,7 @@  (define_insn_and_split "*movdi_aarch64"
        DONE;
     }"
   [(set_attr "type" "mov_reg,mov_reg,mov_reg,mov_imm,mov_imm,load1,load1,store1,store1,\
-                     adr,adr,f_mcr,f_mrc,fmov,fmov")
+                     adr,adr,f_mcr,f_mrc,fmov,neon_move")
    (set_attr "fp" "*,*,*,*,*,*,yes,*,yes,*,*,yes,yes,yes,*")
    (set_attr "simd" "*,*,*,*,*,*,*,*,*,*,*,*,*,*,yes")]
 )
@@ -1063,7 +1063,7 @@  (define_insn "*movtf_aarch64"
    str\\t%q1, %0
    ldp\\t%0, %H0, %1
    stp\\t%1, %H1, %0"
-  [(set_attr "type" "logic_reg,multiple,f_mcr,f_mrc,fconstd,fconstd,\
+  [(set_attr "type" "logic_reg,multiple,f_mcr,f_mrc,neon_move_q,fconstd,\
                      f_loadd,f_stored,neon_load1_2reg,neon_store1_2reg")
    (set_attr "length" "4,8,8,8,4,4,4,4,4,4")
    (set_attr "fp" "*,*,yes,yes,*,yes,yes,yes,*,*")