Patchwork [ARM] Fix minipool handling of aligned labels

login
register
mail settings
Submitter Richard Sandiford
Date Aug. 11, 2011, 11:56 a.m.
Message ID <m339h8w48s.fsf@richards-thinkpad.stglab.manchester.uk.ibm.com>
Download mbox | patch
Permalink /patch/109599/
State New
Headers show

Comments

Richard Sandiford - Aug. 11, 2011, 11:56 a.m.
Trying to compile a certain embedded benchmark with -falign-loops=8
or above fails due to out-of-range references to the literal pool.
This is because arm_reorg doesn't take label alignment into account.

Fixed with the patch below.  Tested on arm-linux-gnueabi.  I also
tried compiling libav with and without the patch (not using -falign-loops
for either run).  As hoped, there were no changes in assembly output.
OK to install?

Richard


gcc/
	* config/arm/arm.c (get_label_padding): New function.
	(create_fix_barrier, md_reorg): Likewise.
Richard Earnshaw - Aug. 11, 2011, 1:53 p.m.
On 11/08/11 12:56, Richard Sandiford wrote:
> Trying to compile a certain embedded benchmark with -falign-loops=8
> or above fails due to out-of-range references to the literal pool.
> This is because arm_reorg doesn't take label alignment into account.
> 
> Fixed with the patch below.  Tested on arm-linux-gnueabi.  I also
> tried compiling libav with and without the patch (not using -falign-loops
> for either run).  As hoped, there were no changes in assembly output.
> OK to install?
> 
> Richard
> 
> 
> gcc/
> 	* config/arm/arm.c (get_label_padding): New function.
> 	(create_fix_barrier, md_reorg): Likewise.
> 

OK.

R.
Steven Bosscher - Aug. 12, 2011, 7:25 p.m.
On Thu, Aug 11, 2011 at 1:56 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:

> gcc/
>        * config/arm/arm.c (get_label_padding): New function.
>        (create_fix_barrier, md_reorg): Likewise.

The ChangeLog doesn't match the patch:
* create_fix_barrier is not a new function.
* md_reorg should be arm_reorg

Ciao!
Steven



> Index: gcc/config/arm/arm.c
> ===================================================================
> --- gcc/config/arm/arm.c        2011-08-11 11:49:18.780977049 +0100
> +++ gcc/config/arm/arm.c        2011-08-11 12:14:42.074629364 +0100
> @@ -11719,6 +11719,19 @@ get_jump_table_size (rtx insn)
>   return 0;
>  }
>
> +/* Return the maximum amount of padding that will be inserted before
> +   label LABEL.  */
> +
> +static HOST_WIDE_INT
> +get_label_padding (rtx label)
> +{
> +  HOST_WIDE_INT align, min_insn_size;
> +
> +  align = 1 << label_to_alignment (label);
> +  min_insn_size = TARGET_THUMB ? 2 : 4;
> +  return align > min_insn_size ? align - min_insn_size : 0;
> +}
> +
>  /* Move a minipool fix MP from its current location to before MAX_MP.
>    If MAX_MP is NULL, then MP doesn't need moving, but the addressing
>    constraints may need updating.  */
> @@ -12265,8 +12278,12 @@ create_fix_barrier (Mfix *fix, HOST_WIDE
>         within range.  */
>       gcc_assert (GET_CODE (from) != BARRIER);
>
> -      /* Count the length of this insn.  */
> -      count += get_attr_length (from);
> +      /* Count the length of this insn.  This must stay in sync with the
> +        code that pushes minipool fixes.  */
> +      if (LABEL_P (from))
> +       count += get_label_padding (from);
> +      else
> +       count += get_attr_length (from);
>
>       /* If there is a jump table, add its length.  */
>       tmp = is_jump_table (from);
> @@ -12696,6 +12713,11 @@ arm_reorg (void)
>              insn = table;
>            }
>        }
> +      else if (LABEL_P (insn))
> +       /* Add the worst-case padding due to alignment.  We don't add
> +          the _current_ padding because the minipool insertions
> +          themselves might change it.  */
> +       address += get_label_padding (insn);
>     }
>
>   fix = minipool_fix_head;
>
Richard Sandiford - Aug. 12, 2011, 8:28 p.m.
Steven Bosscher <stevenb.gcc@gmail.com> writes:
> On Thu, Aug 11, 2011 at 1:56 PM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>
>> gcc/
>>        * config/arm/arm.c (get_label_padding): New function.
>>        (create_fix_barrier, md_reorg): Likewise.
>
> The ChangeLog doesn't match the patch:
> * create_fix_barrier is not a new function.
> * md_reorg should be arm_reorg

Sorry, now fixed.

Richard

Patch

Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	2011-08-11 11:49:18.780977049 +0100
+++ gcc/config/arm/arm.c	2011-08-11 12:14:42.074629364 +0100
@@ -11719,6 +11719,19 @@  get_jump_table_size (rtx insn)
   return 0;
 }
 
+/* Return the maximum amount of padding that will be inserted before
+   label LABEL.  */
+
+static HOST_WIDE_INT
+get_label_padding (rtx label)
+{
+  HOST_WIDE_INT align, min_insn_size;
+
+  align = 1 << label_to_alignment (label);
+  min_insn_size = TARGET_THUMB ? 2 : 4;
+  return align > min_insn_size ? align - min_insn_size : 0;
+}
+
 /* Move a minipool fix MP from its current location to before MAX_MP.
    If MAX_MP is NULL, then MP doesn't need moving, but the addressing
    constraints may need updating.  */
@@ -12265,8 +12278,12 @@  create_fix_barrier (Mfix *fix, HOST_WIDE
 	 within range.  */
       gcc_assert (GET_CODE (from) != BARRIER);
 
-      /* Count the length of this insn.  */
-      count += get_attr_length (from);
+      /* Count the length of this insn.  This must stay in sync with the
+	 code that pushes minipool fixes.  */
+      if (LABEL_P (from))
+	count += get_label_padding (from);
+      else
+	count += get_attr_length (from);
 
       /* If there is a jump table, add its length.  */
       tmp = is_jump_table (from);
@@ -12696,6 +12713,11 @@  arm_reorg (void)
 	      insn = table;
 	    }
 	}
+      else if (LABEL_P (insn))
+	/* Add the worst-case padding due to alignment.  We don't add
+	   the _current_ padding because the minipool insertions
+	   themselves might change it.  */
+	address += get_label_padding (insn);
     }
 
   fix = minipool_fix_head;