Re: [PATCH] PR target/71549: Convert V1TImode register to TImode in debug insn

[Markus Trippelsdorf <markus at trippelsdorf dot de>]

On 2016.06.28 at 11:21 -0700, Gary Funck wrote:
> On 06/20/16 04:55:16, H.J. Lu wrote:
> > TImode register referenced in debug insn can be converted to V1TImode
> > by scalar to vector optimization.  We need to convert a debug insn if
> > it has a variable in a TImode register.
> 
> We have a situation on a few of the UPC tests, where they ICE on
> this gcc_assert().
> 
> 3820              gcc_assert (REG_P (loc)
> 3821                          && GET_MODE (loc) == V1TImode);
> 
> (gdb) p val
> $2 = (rtx) 0x7fffef6d3978
> (gdb) pr
> warning: Expression is not an assignment (and might have no effect)
> (var_location:TI newval (subreg:TI (reg/v/f:V1TI 307 [ newval ]) 0))
> 
> (gdb) p loc
> $1 = (rtx) 0x7fffef409210
> (gdb) pr
> warning: Expression is not an assignment (and might have no effect)
> (subreg:TI (reg/v/f:V1TI 307 [ newval ]) 0)
> 
> As you can see, 'loc' is already a TI mode subreg based upon a V1TI mode reg.
> 
> I didn't try tracking down how we end up with 'loc' as a subreg, but will
> note that in UPC the pointer-to-shared representation is a 16 byte struct,
> aligned on 16 bytes, so the generated code will frequently deal with TImode
> values in registers.
> 
> Given the code that follows this assert,
> 
> 3822              /* Convert V1TImode register, which has been updated by a SET
> 3823                 insn before, to SUBREG TImode.  */
> 3824              PAT_VAR_LOCATION_LOC (val) = gen_rtx_SUBREG (TImode, loc, 0);
> 3825              df_insn_rescan (insn);
> 
> converts the V1TImode register into a TImode subreg, and we already have
> that situation, I tried the following patch:
> 
> --- /a/gcc-trunk/gcc/config/i386/i386.c 2016-06-26 19:01:12.099740515 -0700
> +++ config/i386/i386.c  2016-06-28 11:17:26.323396045 -0700
> @@ -3814,6 +3814,9 @@
>             continue;
>           gcc_assert (GET_CODE (val) == VAR_LOCATION);
>           rtx loc = PAT_VAR_LOCATION_LOC (val);
> +         /* If already a SUBREG, skip.  */
> +         if (SUBREG_P (loc))
> +           continue;
>           gcc_assert (REG_P (loc)
>                       && GET_MODE (loc) == V1TImode);
>           /* Convert V1TImode register, which has been updated by a SET
> 
> 
> Can the patch be amended to include this fix?  Let me know if you need
> additional information, or would like me to try something else.

See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71801, which has a
small testcase, that is fixed by Gary's patch, too.

-- 
Markus