[PATCH] Missing TRULY_NOOP_TRUNCATION after restored optimize_bit_field_compare

[Richard Guenther]

On Fri, Dec 12, 2008 at 7:30 PM, Adam Nemet <anemet@caviumnetworks.com> wrote:
> Now that optimize_bit_field_compare is back we're more likely to read
> bit-inserted fields *without* bit-extraction.  This means that on MIPS64 if
> the top bits of a 32-bit bitfield are manipulated wrong code is generated
> unless the top field is properly sign-extended (because for example in
> pr33887-1.C, the constant we're comparing against is 64-bit sign-extended.)
>
> This happens because for the testcase below store_bit_field_1 generates this:
>
> (set (zero_extract:DI (subreg:DI (reg:SI 193) 0)
>            (const_int 3)
>            (const_int 29))
>        (subreg:DI (reg:SI 194) 0))
>
> With the patch we now add a truncation of the result:
>
> (set (reg:SI 193)
>     (truncate:SI (subreg:DI (reg:SI 193) 0)))
>
> This is very similar to my recent fix to extract_bit_field_1
> (http://gcc.gnu.org/ml/gcc-patches/2008-10/msg01072.html) and I had on my list
> to look at stores as well but apparently I wasn't quick enough.
>
> Bootstrapped and regtested on mips64octeon-linux-gnu with {,-mabi=64}.  These
> are the tests that regressed after optimize_bit_field_compare but are
> recovered now:
>
> gcc:
>
>  FAIL->PASS (total: 9)
>    gcc.c-torture/execute/bf-sign-1.c execution,  -O1
>    gcc.c-torture/execute/bf-sign-1.c execution,  -O2
>    gcc.c-torture/execute/bf-sign-1.c execution,  -O3 -fomit-frame-pointer
>    gcc.c-torture/execute/bf-sign-1.c execution,  -O3 -g
>    gcc.c-torture/execute/bf-sign-1.c execution,  -Os
>    gcc.c-torture/execute/pr19689.c execution,  -O2
>    gcc.c-torture/execute/pr19689.c execution,  -O3 -fomit-frame-pointer
>    gcc.c-torture/execute/pr19689.c execution,  -O3 -g
>    gcc.c-torture/execute/pr19689.c execution,  -Os
>
> g++:
>
>  FAIL->PASS (total: 4)
>    g++.dg/torture/pr33887-1.C  -O2  execution test
>    g++.dg/torture/pr33887-1.C  -O3 -fomit-frame-pointer  execution test
>    g++.dg/torture/pr33887-1.C  -O3 -g  execution test
>    g++.dg/torture/pr33887-1.C  -Os  execution test
>
> OK to install?

Ok.

Thanks,
Richard.

> Adam
>
>        * expmed.c (store_bit_field_1): Properly truncate the paradoxical
>        subreg of op0 to the original op0.
>
> testsuite/
>        * gcc.target/mips/ins-2.c: New test.
>
> Index: expmed.c
> ===================================================================
> --- expmed.c    (revision 142638)
> +++ expmed.c    (working copy)
> @@ -748,6 +748,16 @@ store_bit_field_1 (rtx str_rtx, unsigned
>       if (pat)
>        {
>          emit_insn (pat);
> +
> +         /* If the mode of insertion operation is wider than the mode
> +            of the target register we created a paradoxical subreg for
> +            the target.  Truncate the paradoxical subreg of the
> +            target to itself properly.  */
> +         if (!TRULY_NOOP_TRUNCATION (GET_MODE_BITSIZE (GET_MODE (op0)),
> +                                     GET_MODE_BITSIZE (op_mode))
> +             && (REG_P (xop0)
> +                 || GET_CODE (xop0) == SUBREG))
> +             convert_move (op0, xop0, true);
>          return true;
>        }
>       delete_insns_since (last);
> Index: testsuite/gcc.target/mips/ins-2.c
> ===================================================================
> --- testsuite/gcc.target/mips/ins-2.c   (revision 0)
> +++ testsuite/gcc.target/mips/ins-2.c   (revision 0)
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-mips-options "-O -meb -mips64r2 -mgp64" } */
> +/* { dg-final { scan-assembler-times "\tins\t|\tdins\t" 1 } } */
> +/* { dg-final { scan-assembler-times "\tsll\t|\tins\t" 1 } } */
> +
> +struct s
> +{
> +  int a:3;
> +  int b:29;
> +};
> +
> +NOMIPS16 void
> +f (int a)
> +{
> +  struct s s;
> +  asm volatile ("" : "=r"(s));
> +  s.a = a;
> +  asm volatile ("" :: "r"(s));
> +}
>