Dimitar Dimitrov <dimi...@dinux.eu> writes:
> After r16-160-ge6f89d78c1a752, late_combine2 started transforming the
> following RTL for pru-unknown-elf:
>
> (insn 3949 3948 3951 255 (set (reg:QI 56 r14.b0 [orig:1856 _619 ] [1856])
> (and:QI (reg:QI 1 r0.b1 [orig:1855 _201 ] [1855])
> (const_int 3 [0x3])))
> (nil))
> ...
> (insn 3961 7067 3962 255 (set (reg:SI 56 r14.b0)
> (zero_extend:SI (reg:QI 56 r14.b0 [orig:1856 _619 ] [1856])))
> (nil))
>
> into:
>
> (insn 3961 7067 3962 255 (set (reg:SI 56 r14.b0)
> (and:SI (subreg:SI (reg:QI 1 r0.b1 [orig:1855 _201 ] [1855]) 0)
> (const_int 3 [0x3])))
> (nil))
>
> That caused libbacktrace build to break for pru-unknown-elf. Register
> r0.b1 (regno 1) is not valid for SImode, which validate_subreg failed to
> reject.
>
> Fix by calling HARD_REGNO_MODE_OK to ensure that both inner and outer
> modes are valid for the hardware subreg. Remove the premature "success"
> return for paradoxical subregs, in order to allow subsequent validity
> checks to be executed.
>
> This patch fixes the broken PRU toolchain build. It leaves only two
> test case regressions for PRU, caused by rnreg pass renaming a valid
> paradoxical subreg into an invalid one.
> gcc.c-torture/execute/20040709-1.c
> gcc.c-torture/execute/20040709-2.c
> I consider these two a separate issue.
>
> I ensured that test results with and without this patch for
> x86_64-pc-linux-gnu are the same for C and C++.
>
> Ok for trunk?
>
> PR target/119966
>
> gcc/ChangeLog:
>
> * emit-rtl.cc (validate_subreg): Do not exit immediately for
> paradoxical subregs. Add mode checks for validity of
> hardware subregs.
>
> Signed-off-by: Dimitar Dimitrov <dimi...@dinux.eu>
> ---
> gcc/emit-rtl.cc | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/emit-rtl.cc b/gcc/emit-rtl.cc
> index 3e2c4309dee..d63543038bb 100644
> --- a/gcc/emit-rtl.cc
> +++ b/gcc/emit-rtl.cc
> @@ -969,10 +969,10 @@ validate_subreg (machine_mode omode, machine_mode imode,
> }
>
> /* Paradoxical subregs must have offset zero. */
> - if (maybe_gt (osize, isize))
> - return known_eq (offset, 0U);
> + if (maybe_gt (osize, isize) && !known_eq (offset, 0U))
> + return false;
>
> - /* This is a normal subreg. Verify that the offset is representable. */
> + /* Verify that the offset is representable. */
>
> /* For hard registers, we already have most of these rules collected in
> subreg_offset_representable_p. */
This part seems fine, but...
> @@ -983,6 +983,9 @@ validate_subreg (machine_mode omode, machine_mode imode,
> if ((COMPLEX_MODE_P (imode) || VECTOR_MODE_P (imode))
> && GET_MODE_INNER (imode) == omode)
> ;
> + else if (!targetm.hard_regno_mode_ok (regno, imode)
> + || !targetm.hard_regno_mode_ok (regno, omode))
> + return false;
> else if (!REG_CAN_CHANGE_MODE_P (regno, imode, omode))
> return false;
...this is IMO a logically separate change. Instead, I think we want
to add known_le (osize, isize) to:
if (maybe_lt (osize, regsize)
&& ! (lra_in_progress && (FLOAT_MODE_P (imode) || FLOAT_MODE_P (omode))))
since the test is not correct for big-endian paradoxical subregs,
and is redundant for little-endian paradoxical subregs.
Thanks,
Richard
