[ARM,AArch64] Make aarch64-common.c files more robust.

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.

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.

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.