* [ARM, AArch64] Make aarch64-common.c files more robust.
@ 2013-09-30 10:05 James Greenhalgh
2013-09-30 11:17 ` Marcus Shawcroft
2013-10-01 17:01 ` Richard Earnshaw
0 siblings, 2 replies; 3+ messages in thread
From: James Greenhalgh @ 2013-09-30 10:05 UTC (permalink / raw)
To: gcc-patches; +Cc: marcus.shawcroft, ramana.radhakrishnan, richard.earnshaw
[-- Attachment #1: Type: text/plain, Size: 797 bytes --]
Hi,
Recently I've found myself getting a number of segfaults from
code calling in to the arm_early_load/alu_dep functions in
aarch64-common.c. These functions expect a particular form
for the RTX patterns they work over, but some of them do
not validate this form.
This patch fixes that, removing segmentation faults I see
when tuning for Cortex-A15 and Cortex-A7.
Tested on aarch64-none-elf and arm-none-eabi with no regressions.
OK?
Thanks,
James
---
gcc/
2013-09-30 James Greenhalgh <james.greenhalgh@arm.com>
* config/arm/aarch-common.c
(arm_early_load_addr_dep): Add sanity checking.
(arm_no_early_alu_shift_dep): Likewise.
(arm_no_early_alu_shift_value_dep): Likewise.
(arm_no_early_mul_dep): Likewise.
(arm_no_early_store_addr_dep): Likewise.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ARM-AArch64-Make-aarch64-common.c-files-more-robust.patch --]
[-- Type: text/x-patch; name=0001-ARM-AArch64-Make-aarch64-common.c-files-more-robust.patch, Size: 4284 bytes --]
diff --git a/gcc/config/arm/aarch-common.c b/gcc/config/arm/aarch-common.c
index 69366af..ea50848 100644
--- a/gcc/config/arm/aarch-common.c
+++ b/gcc/config/arm/aarch-common.c
@@ -44,7 +44,12 @@ arm_early_load_addr_dep (rtx producer, rtx consumer)
value = COND_EXEC_CODE (value);
if (GET_CODE (value) == PARALLEL)
value = XVECEXP (value, 0, 0);
+
+ if (GET_CODE (value) != SET)
+ return 0;
+
value = XEXP (value, 0);
+
if (GET_CODE (addr) == COND_EXEC)
addr = COND_EXEC_CODE (addr);
if (GET_CODE (addr) == PARALLEL)
@@ -54,6 +59,10 @@ arm_early_load_addr_dep (rtx producer, rtx consumer)
else
addr = XVECEXP (addr, 0, 0);
}
+
+ if (GET_CODE (addr) != SET)
+ return 0;
+
addr = XEXP (addr, 1);
return reg_overlap_mentioned_p (value, addr);
@@ -74,21 +83,41 @@ arm_no_early_alu_shift_dep (rtx producer, rtx consumer)
value = COND_EXEC_CODE (value);
if (GET_CODE (value) == PARALLEL)
value = XVECEXP (value, 0, 0);
+
+ if (GET_CODE (value) != SET)
+ return 0;
+
value = XEXP (value, 0);
+
if (GET_CODE (op) == COND_EXEC)
op = COND_EXEC_CODE (op);
if (GET_CODE (op) == PARALLEL)
op = XVECEXP (op, 0, 0);
+
+ if (GET_CODE (op) != SET)
+ return 0;
+
op = XEXP (op, 1);
+ if (!INSN_P (op))
+ return 0;
+
early_op = XEXP (op, 0);
+
/* This is either an actual independent shift, or a shift applied to
the first operand of another operation. We want the whole shift
operation. */
if (REG_P (early_op))
early_op = op;
- return !reg_overlap_mentioned_p (value, early_op);
+ if (GET_CODE (op) == ASHIFT
+ || GET_CODE (op) == ROTATE
+ || GET_CODE (op) == ASHIFTRT
+ || GET_CODE (op) == LSHIFTRT
+ || GET_CODE (op) == ROTATERT)
+ return !reg_overlap_mentioned_p (value, early_op);
+ else
+ return 0;
}
/* Return nonzero if the CONSUMER instruction (an ALU op) does not
@@ -106,13 +135,25 @@ arm_no_early_alu_shift_value_dep (rtx producer, rtx consumer)
value = COND_EXEC_CODE (value);
if (GET_CODE (value) == PARALLEL)
value = XVECEXP (value, 0, 0);
+
+ if (GET_CODE (value) != SET)
+ return 0;
+
value = XEXP (value, 0);
+
if (GET_CODE (op) == COND_EXEC)
op = COND_EXEC_CODE (op);
if (GET_CODE (op) == PARALLEL)
op = XVECEXP (op, 0, 0);
+
+ if (GET_CODE (op) != SET)
+ return 0;
+
op = XEXP (op, 1);
+ if (!INSN_P (op))
+ return 0;
+
early_op = XEXP (op, 0);
/* This is either an actual independent shift, or a shift applied to
@@ -121,7 +162,14 @@ arm_no_early_alu_shift_value_dep (rtx producer, rtx consumer)
if (!REG_P (early_op))
early_op = XEXP (early_op, 0);
- return !reg_overlap_mentioned_p (value, early_op);
+ if (GET_CODE (op) == ASHIFT
+ || GET_CODE (op) == ROTATE
+ || GET_CODE (op) == ASHIFTRT
+ || GET_CODE (op) == LSHIFTRT
+ || GET_CODE (op) == ROTATERT)
+ return !reg_overlap_mentioned_p (value, early_op);
+ else
+ return 0;
}
/* Return nonzero if the CONSUMER (a mul or mac op) does not
@@ -138,11 +186,20 @@ arm_no_early_mul_dep (rtx producer, rtx consumer)
value = COND_EXEC_CODE (value);
if (GET_CODE (value) == PARALLEL)
value = XVECEXP (value, 0, 0);
+
+ if (GET_CODE (value) != SET)
+ return 0;
+
value = XEXP (value, 0);
+
if (GET_CODE (op) == COND_EXEC)
op = COND_EXEC_CODE (op);
if (GET_CODE (op) == PARALLEL)
op = XVECEXP (op, 0, 0);
+
+ if (GET_CODE (op) != SET)
+ return 0;
+
op = XEXP (op, 1);
if (GET_CODE (op) == PLUS || GET_CODE (op) == MINUS)
@@ -169,11 +226,20 @@ arm_no_early_store_addr_dep (rtx producer, rtx consumer)
value = COND_EXEC_CODE (value);
if (GET_CODE (value) == PARALLEL)
value = XVECEXP (value, 0, 0);
+
+ if (GET_CODE (value) != SET)
+ return 0;
+
value = XEXP (value, 0);
+
if (GET_CODE (addr) == COND_EXEC)
addr = COND_EXEC_CODE (addr);
if (GET_CODE (addr) == PARALLEL)
addr = XVECEXP (addr, 0, 0);
+
+ if (GET_CODE (addr) != SET)
+ return 0;
+
addr = XEXP (addr, 0);
return !reg_overlap_mentioned_p (value, addr);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [ARM, AArch64] Make aarch64-common.c files more robust.
2013-09-30 10:05 [ARM, AArch64] Make aarch64-common.c files more robust James Greenhalgh
@ 2013-09-30 11:17 ` Marcus Shawcroft
2013-10-01 17:01 ` Richard Earnshaw
1 sibling, 0 replies; 3+ messages in thread
From: Marcus Shawcroft @ 2013-09-30 11:17 UTC (permalink / raw)
To: James Greenhalgh; +Cc: gcc-patches, ramana.radhakrishnan, Richard Earnshaw
On 30 September 2013 09:52, James Greenhalgh <james.greenhalgh@arm.com> wrote:
>
> Hi,
>
> aarch64-common.c. These functions expect a particular form
You meant aarch-common.c here and in the title ;-)
This is fine by me, but as a config/arm/ change needs OK from Ramana or Richard.
/Marcus
> 2013-09-30 James Greenhalgh <james.greenhalgh@arm.com>
>
> * config/arm/aarch-common.c
> (arm_early_load_addr_dep): Add sanity checking.
> (arm_no_early_alu_shift_dep): Likewise.
> (arm_no_early_alu_shift_value_dep): Likewise.
> (arm_no_early_mul_dep): Likewise.
> (arm_no_early_store_addr_dep): Likewise.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [ARM, AArch64] Make aarch64-common.c files more robust.
2013-09-30 10:05 [ARM, AArch64] Make aarch64-common.c files more robust James Greenhalgh
2013-09-30 11:17 ` Marcus Shawcroft
@ 2013-10-01 17:01 ` Richard Earnshaw
1 sibling, 0 replies; 3+ messages in thread
From: Richard Earnshaw @ 2013-10-01 17:01 UTC (permalink / raw)
To: James Greenhalgh; +Cc: gcc-patches, Marcus Shawcroft, Ramana Radhakrishnan
On 30/09/13 09:52, James Greenhalgh wrote:
>
> Hi,
>
> Recently I've found myself getting a number of segfaults from
> code calling in to the arm_early_load/alu_dep functions in
> aarch64-common.c. These functions expect a particular form
> for the RTX patterns they work over, but some of them do
> not validate this form.
>
> This patch fixes that, removing segmentation faults I see
> when tuning for Cortex-A15 and Cortex-A7.
>
> Tested on aarch64-none-elf and arm-none-eabi with no regressions.
>
> OK?
>
> Thanks,
> James
>
> ---
> gcc/
>
> 2013-09-30 James Greenhalgh <james.greenhalgh@arm.com>
>
> * config/arm/aarch-common.c
> (arm_early_load_addr_dep): Add sanity checking.
> (arm_no_early_alu_shift_dep): Likewise.
> (arm_no_early_alu_shift_value_dep): Likewise.
> (arm_no_early_mul_dep): Likewise.
> (arm_no_early_store_addr_dep): Likewise.
>
>
> 0001-ARM-AArch64-Make-aarch64-common.c-files-more-robust.patch
>
>
> diff --git a/gcc/config/arm/aarch-common.c b/gcc/config/arm/aarch-common.c
> index 69366af..ea50848 100644
> --- a/gcc/config/arm/aarch-common.c
> +++ b/gcc/config/arm/aarch-common.c
> @@ -44,7 +44,12 @@ arm_early_load_addr_dep (rtx producer, rtx consumer)
> value = COND_EXEC_CODE (value);
> if (GET_CODE (value) == PARALLEL)
> value = XVECEXP (value, 0, 0);
> +
> + if (GET_CODE (value) != SET)
> + return 0;
> +
> value = XEXP (value, 0);
Please change this to use SET_DEST.
> +
> if (GET_CODE (addr) == COND_EXEC)
> addr = COND_EXEC_CODE (addr);
> if (GET_CODE (addr) == PARALLEL)
> @@ -54,6 +59,10 @@ arm_early_load_addr_dep (rtx producer, rtx consumer)
> else
> addr = XVECEXP (addr, 0, 0);
> }
> +
> + if (GET_CODE (addr) != SET)
> + return 0;
> +
> addr = XEXP (addr, 1);
>
And this to use SET_SRC.
> return reg_overlap_mentioned_p (value, addr);
> @@ -74,21 +83,41 @@ arm_no_early_alu_shift_dep (rtx producer, rtx consumer)
> value = COND_EXEC_CODE (value);
> if (GET_CODE (value) == PARALLEL)
> value = XVECEXP (value, 0, 0);
> +
> + if (GET_CODE (value) != SET)
> + return 0;
> +
> value = XEXP (value, 0);
SET_DEST.
> +
> if (GET_CODE (op) == COND_EXEC)
> op = COND_EXEC_CODE (op);
> if (GET_CODE (op) == PARALLEL)
> op = XVECEXP (op, 0, 0);
> +
> + if (GET_CODE (op) != SET)
> + return 0;
> +
> op = XEXP (op, 1);
SET_SRC.
>
> + if (!INSN_P (op))
> + return 0;
This looks wrong. What are you really expecting here? Surely not INSN,
since then ...
> +
> early_op = XEXP (op, 0);
... this would always fault, since element 0 is an int
> +
> /* This is either an actual independent shift, or a shift applied to
> the first operand of another operation. We want the whole shift
> operation. */
> if (REG_P (early_op))
> early_op = op;
>
> - return !reg_overlap_mentioned_p (value, early_op);
> + if (GET_CODE (op) == ASHIFT
> + || GET_CODE (op) == ROTATE
> + || GET_CODE (op) == ASHIFTRT
> + || GET_CODE (op) == LSHIFTRT
> + || GET_CODE (op) == ROTATERT)
> + return !reg_overlap_mentioned_p (value, early_op);
> + else
> + return 0;
> }
>
> /* Return nonzero if the CONSUMER instruction (an ALU op) does not
> @@ -106,13 +135,25 @@ arm_no_early_alu_shift_value_dep (rtx producer, rtx consumer)
> value = COND_EXEC_CODE (value);
> if (GET_CODE (value) == PARALLEL)
> value = XVECEXP (value, 0, 0);
> +
> + if (GET_CODE (value) != SET)
> + return 0;
> +
> value = XEXP (value, 0);
SET_DEST.
> +
> if (GET_CODE (op) == COND_EXEC)
> op = COND_EXEC_CODE (op);
> if (GET_CODE (op) == PARALLEL)
> op = XVECEXP (op, 0, 0);
> +
> + if (GET_CODE (op) != SET)
> + return 0;
> +
> op = XEXP (op, 1);
>
> + if (!INSN_P (op))
> + return 0;
> +
Same issue as above.
> early_op = XEXP (op, 0);
>
> /* This is either an actual independent shift, or a shift applied to
> @@ -121,7 +162,14 @@ arm_no_early_alu_shift_value_dep (rtx producer, rtx consumer)
> if (!REG_P (early_op))
> early_op = XEXP (early_op, 0);
>
> - return !reg_overlap_mentioned_p (value, early_op);
> + if (GET_CODE (op) == ASHIFT
> + || GET_CODE (op) == ROTATE
> + || GET_CODE (op) == ASHIFTRT
> + || GET_CODE (op) == LSHIFTRT
> + || GET_CODE (op) == ROTATERT)
> + return !reg_overlap_mentioned_p (value, early_op);
> + else
> + return 0;
> }
>
> /* Return nonzero if the CONSUMER (a mul or mac op) does not
> @@ -138,11 +186,20 @@ arm_no_early_mul_dep (rtx producer, rtx consumer)
> value = COND_EXEC_CODE (value);
> if (GET_CODE (value) == PARALLEL)
> value = XVECEXP (value, 0, 0);
> +
> + if (GET_CODE (value) != SET)
> + return 0;
> +
> value = XEXP (value, 0);
SET_DEST.
> +
> if (GET_CODE (op) == COND_EXEC)
> op = COND_EXEC_CODE (op);
> if (GET_CODE (op) == PARALLEL)
> op = XVECEXP (op, 0, 0);
> +
> + if (GET_CODE (op) != SET)
> + return 0;
> +
> op = XEXP (op, 1);
SET_SRC.
>
> if (GET_CODE (op) == PLUS || GET_CODE (op) == MINUS)
> @@ -169,11 +226,20 @@ arm_no_early_store_addr_dep (rtx producer, rtx consumer)
> value = COND_EXEC_CODE (value);
> if (GET_CODE (value) == PARALLEL)
> value = XVECEXP (value, 0, 0);
> +
> + if (GET_CODE (value) != SET)
> + return 0;
> +
> value = XEXP (value, 0);
> +
> if (GET_CODE (addr) == COND_EXEC)
> addr = COND_EXEC_CODE (addr);
> if (GET_CODE (addr) == PARALLEL)
> addr = XVECEXP (addr, 0, 0);
> +
> + if (GET_CODE (addr) != SET)
> + return 0;
> +
> addr = XEXP (addr, 0);
SET_DEST.
>
> return !reg_overlap_mentioned_p (value, addr);
>
R.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-10-01 17:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-30 10:05 [ARM, AArch64] Make aarch64-common.c files more robust James Greenhalgh
2013-09-30 11:17 ` Marcus Shawcroft
2013-10-01 17:01 ` Richard Earnshaw
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).