Re: [PATCH][AArch64] PR target/78362: Make sure to only take REGNO of a register
On Wed, Nov 16, 2016 at 04:57:29PM +, 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
>
> PR target/78362
> * config/aarch64/aarch64.md (add3): 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
>
> PR target/78362
> * gcc.c-torture/compile/pr78362.c: New test.
Re: [PATCH][AArch64] PR target/78362: Make sure to only take REGNO of a register
On Wed, Nov 16, 2016 at 4:57 PM, 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?
LGTM but can't approve.
Ramana
> Thanks,
> Kyrill
>
> 2016-11-16 Kyrylo Tkachov
>
> PR target/78362
> * config/aarch64/aarch64.md (add3): 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
>
> PR target/78362
> * gcc.c-torture/compile/pr78362.c: New test.
Re: [PATCH][AArch64] PR target/78362: Make sure to only take REGNO of a register
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
PR target/78362
* config/aarch64/aarch64.md (add3): 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
PR target/78362
* gcc.c-torture/compile/pr78362.c: New test.
Re: [PATCH][AArch64] PR target/78362: Make sure to only take REGNO of a register
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
PR target/78362
* config/aarch64/aarch64.md (add3): 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
PR target/78362
* gcc.c-torture/compile/pr78362.c: New test.
[PATCH][AArch64] PR target/78362: Make sure to only take REGNO of a register
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
PR target/78362
* config/aarch64/aarch64.md (add3): 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
PR target/78362
* gcc.c-torture/compile/pr78362.c: New test.
commit 068224c568d6f06f68512f12ecebea8bfc873fe9
Author: Kyrylo Tkachov
Date: Tue Nov 15 14:52:33 2016 +
[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 "add3"
(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)
&& 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, 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 000..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;
+}
