Re: [PTX] simplify movs

[Tom de Vries <Tom_deVries at mentor dot com>]

On 12/02/2015 04:09 PM, Nathan Sidwell wrote:
> +/* Output a pattern for a move instruction.  */
> +
> +const char *
> +nvptx_output_mov_insn (rtx dst, rtx src)
> +{
> +  machine_mode dst_mode = GET_MODE (dst);
> +  machine_mode dst_inner = (GET_CODE (dst) == SUBREG
> +			    ? GET_MODE (XEXP (dst, 0)) : dst_mode);
> +  machine_mode src_inner = (GET_CODE (src) == SUBREG
> +			    ? GET_MODE (XEXP (src, 0)) : dst_mode);
> +
> +  if (REG_P (dst) && REGNO (dst) == NVPTX_RETURN_REGNUM && dst_mode == HImode)
> +    /* Special handling for the return register.  It's never really an
> +       HI object, and only occurs as the destination of a move
> +       insn.  */
> +    dst_inner = SImode;
> +
> +  if (src_inner == dst_inner)
> +    return "%.\tmov%t0\t%0, %1;";
> +
> +  if (CONSTANT_P (src))
> +    return (GET_MODE_CLASS (dst_inner) == MODE_INT
> +	    && GET_MODE_CLASS (src_inner) != MODE_FLOAT
> +	    ? "%.\tmov%t0\t%0, %1;" : "%.\tmov.b%T0\t%0, %1;");

Hi,

src_inner uses dst_mode rather than GET_MODE (src). I'm trying to 
understand if that is intentional or not.


F.i., for this insn:
....

(insn 7 6 8 2

    (set (reg:QI 67)
           (const_int 1 [0x1])) 2 {*movqi_insn}
    (nil))
...

when entering nvptx_output_mov_insn we have:
- GET_MODE (dst) == QI and GET_MODE (src) == VOID, but
- dst_inner == QI and src_inner == QI

So we handle this insn using this clause:
...
  if (src_inner == dst_inner)
    return "%.\tmov%t0\t%0, %1;";
...

rather than using the const handling clause:
...
  if (CONSTANT_P (src))
    return (GET_MODE_CLASS (dst_inner) == MODE_INT
            && GET_MODE_CLASS (src_inner) != MODE_FLOAT
            ? "%.\tmov%t0\t%0, %1;" : "%.\tmov.b%T0\t%0, %1;");
...

Using attached patch, we get dst_inner == QI and src_inner == VOID, and 
the insn is handled by the const handling clause instead, and the same 
string is returned as before.


I can imagine that src_inner uses dst_mode to avoid setting src_inner to 
VOIDmode (in which case a comment explaining that would avoid the 
impression of a copy-pasto). But AFAICT, it's not necessary.

Thanks,
- Tom

Fix src_inner in nvptx_output_mov_insn

---
 gcc/config/nvptx/nvptx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index 4c35c16..6951e27 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -2146,10 +2146,11 @@ const char *
 nvptx_output_mov_insn (rtx dst, rtx src)
 {
   machine_mode dst_mode = GET_MODE (dst);
+  machine_mode src_mode = GET_MODE (src);
   machine_mode dst_inner = (GET_CODE (dst) == SUBREG
 			    ? GET_MODE (XEXP (dst, 0)) : dst_mode);
   machine_mode src_inner = (GET_CODE (src) == SUBREG
-			    ? GET_MODE (XEXP (src, 0)) : dst_mode);
+			    ? GET_MODE (XEXP (src, 0)) : src_mode);
 
   rtx sym = src;
   if (GET_CODE (sym) == CONST)