Message ID | 582C8FF9.9070603@foss.arm.com |
---|---|
State | New |
Headers | show |
Ping. https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01664.html Thanks, Kyrill On 16/11/16 16:57, Kyrill Tkachov wrote: > Hi all, > > As the PR says we have an RTL checking failure that occurs when building libgcc for aarch64. > The expander code for addsi3 takes the REGNO of a SUBREG in operands[1]. The three operands > in the failing case are: > {(reg:SI 78), (subreg:SI (reg:DI 77) 0), (subreg:SI (reg:DI 73 [ ivtmp.9 ]) 0)} > > According to the documentation of register_operand (which is the predicate for operands[1]), > operands[1] can be a REG or a SUBREG. If it's a subreg it may also contain a MEM before reload > (because it is guaranteed to be reloaded into a register later). Anyway, the bottom line is that > we have to be careful when taking REGNO of expressions during expand-time. > > This patch extracts the inner rtx in case we have a SUBREG and checks that it's a REG before > checking its REGNO. > > Bootstrapped and tested on aarch64-none-linux-gnu. Tested aarch64-none-elf with RTL checking enabled > (without this patch that doesn't build). > > Ok for trunk? > Thanks, > Kyrill > > 2016-11-16 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR target/78362 > * config/aarch64/aarch64.md (add<mode>3): Extract inner expression > from a subreg in operands[1] and don't call REGNO on a non-reg expression > when deciding to force operands[2] into a reg. > > 2016-11-16 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR target/78362 > * gcc.c-torture/compile/pr78362.c: New test.
Ping. Thanks, Kyrill On 23/11/16 14:16, Kyrill Tkachov wrote: > > Ping. > https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01664.html > > Thanks, > Kyrill > > On 16/11/16 16:57, Kyrill Tkachov wrote: >> Hi all, >> >> As the PR says we have an RTL checking failure that occurs when building libgcc for aarch64. >> The expander code for addsi3 takes the REGNO of a SUBREG in operands[1]. The three operands >> in the failing case are: >> {(reg:SI 78), (subreg:SI (reg:DI 77) 0), (subreg:SI (reg:DI 73 [ ivtmp.9 ]) 0)} >> >> According to the documentation of register_operand (which is the predicate for operands[1]), >> operands[1] can be a REG or a SUBREG. If it's a subreg it may also contain a MEM before reload >> (because it is guaranteed to be reloaded into a register later). Anyway, the bottom line is that >> we have to be careful when taking REGNO of expressions during expand-time. >> >> This patch extracts the inner rtx in case we have a SUBREG and checks that it's a REG before >> checking its REGNO. >> >> Bootstrapped and tested on aarch64-none-linux-gnu. Tested aarch64-none-elf with RTL checking enabled >> (without this patch that doesn't build). >> >> Ok for trunk? >> Thanks, >> Kyrill >> >> 2016-11-16 Kyrylo Tkachov <kyrylo.tkachov@arm.com> >> >> PR target/78362 >> * config/aarch64/aarch64.md (add<mode>3): Extract inner expression >> from a subreg in operands[1] and don't call REGNO on a non-reg expression >> when deciding to force operands[2] into a reg. >> >> 2016-11-16 Kyrylo Tkachov <kyrylo.tkachov@arm.com> >> >> PR target/78362 >> * gcc.c-torture/compile/pr78362.c: New test. >
On Wed, Nov 16, 2016 at 04:57:29PM +0000, Kyrill Tkachov wrote: > Hi all, > > As the PR says we have an RTL checking failure that occurs when building > libgcc for aarch64. The expander code for addsi3 takes the REGNO of a SUBREG > in operands[1]. The > three operands in the failing case are: > {(reg:SI 78), (subreg:SI (reg:DI 77) 0), (subreg:SI (reg:DI 73 [ ivtmp.9 ]) 0)} > > According to the documentation of register_operand (which is the predicate > for operands[1]), operands[1] can be a REG or a SUBREG. If it's a subreg it > may also contain a MEM before reload (because it is guaranteed to be reloaded > into a register later). Anyway, the bottom line is that we have to be careful > when taking REGNO of expressions during expand-time. > > This patch extracts the inner rtx in case we have a SUBREG and checks that > it's a REG before checking its REGNO. > > Bootstrapped and tested on aarch64-none-linux-gnu. Tested aarch64-none-elf > with RTL checking enabled (without this patch that doesn't build). > > Ok for trunk? OK. Thanks, James > Thanks, > Kyrill > > 2016-11-16 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR target/78362 > * config/aarch64/aarch64.md (add<mode>3): Extract inner expression > from a subreg in operands[1] and don't call REGNO on a non-reg expression > when deciding to force operands[2] into a reg. > > 2016-11-16 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR target/78362 > * gcc.c-torture/compile/pr78362.c: New test.
commit 068224c568d6f06f68512f12ecebea8bfc873fe9 Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com> Date: Tue Nov 15 14:52:33 2016 +0000 [AArch64] PR target/78362: Make sure to only take REGNO of a register diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 9e5eee9..1dcb6b2 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -1611,11 +1611,15 @@ (define_expand "add<mode>3" (match_operand:GPI 2 "aarch64_pluslong_operand" "")))] "" { + /* If operands[1] is a subreg extract the inner RTX. */ + rtx op1 = REG_P (operands[1]) ? operands[1] : SUBREG_REG (operands[1]); + /* If the constant is too large for a single instruction and isn't frame based, split off the immediate so it is available for CSE. */ if (!aarch64_plus_immediate (operands[2], <MODE>mode) && can_create_pseudo_p () - && !REGNO_PTR_FRAME_P (REGNO (operands[1]))) + && (!REG_P (op1) + || !REGNO_PTR_FRAME_P (REGNO (op1)))) operands[2] = force_reg (<MODE>mode, operands[2]); }) diff --git a/gcc/testsuite/gcc.c-torture/compile/pr78362.c b/gcc/testsuite/gcc.c-torture/compile/pr78362.c new file mode 100644 index 0000000..66eea7d --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr78362.c @@ -0,0 +1,11 @@ +/* PR target/78362. */ + +long a; + +void +foo (void) +{ + for (;; a--) + if ((int) a) + break; +}