[Patch ARM] Fix PR51819.

[Patch ARM] Fix PR51819.

[Ramana Radhakrishnan]

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

Hi,

PR51819 is a case where we were actually putting out alignment hints
for the wrong memory size. The attached patch corrects this and
another latent issue that I spotted.

Committed after testing on arm-linux-gnueabi with Neon configurations.

cheers
Ramana

2012-01-20  Ramana Radhakrishnan  <ramana.radhakrishnan@linaro.org>

        PR target/51819
        * config/arm/arm.c (arm_print_operand): Correct output of alignment
        hints for neon loads and stores.

[-- Attachment #2: final-pr51819.txt --]
[-- Type: text/plain, Size: 597 bytes --]

Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	(revision 183333)
+++ gcc/config/arm/arm.c	(working copy)
@@ -17711,9 +17711,9 @@
 	/* Only certain alignment specifiers are supported by the hardware.  */
 	if (memsize == 16 && (align % 32) == 0)
 	  align_bits = 256;
-	else if ((memsize == 8 || memsize == 16) && (align % 16) == 0)
+	else if (memsize == 16 && (align % 16) == 0)
 	  align_bits = 128;
-	else if ((align % 8) == 0)
+	else if (memsize >= 8 && (align % 8) == 0)
 	  align_bits = 64;
 	else
 	  align_bits = 0;

Re: [Patch ARM] Fix PR51819.

[Ulrich Weigand]

Ramana Radhakrishnan wrote:

> PR51819 is a case where we were actually putting out alignment hints
> for the wrong memory size. The attached patch corrects this and
> another latent issue that I spotted.


Your patch did:

 	/* Only certain alignment specifiers are supported by the hardware.  */
 	if (memsize == 16 && (align % 32) == 0)
 	  align_bits = 256;
-	else if ((memsize == 8 || memsize == 16) && (align % 16) == 0)
+	else if (memsize == 16 && (align % 16) == 0)
 	  align_bits = 128;
-	else if ((align % 8) == 0)
+	else if (memsize >= 8 && (align % 8) == 0)
 	  align_bits = 64;
 	else
 	  align_bits = 0;


However, I'm still seeing incorrect memory hints (with an add-on patch):

  Error: bad alignment -- `vld1.64 {d18-d19},[r3:256]'


The ISA document specifies the following supported alignment hints:

  64  8-byte alignment
 128 16-byte alignment, available only if <list> contains two or four registers
 256 32-byte alignment, available only if <list> contains four registers


Shouldn't the check be implemented along the following lines?

	if (memsize == 32 && (align % 32) == 0)
 	  align_bits = 256;
	else if ((memsize == 16 || memsize == 32) && (align % 16) == 0)
	  align_bits = 128;
	else if (memsize >= 8 && (align % 8) == 0)
	  align_bits = 64;
	else
	  align_bits = 0;

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [Patch ARM] Fix PR51819.

[Ramana Radhakrishnan]

Hi Uli,

Apologies for the delayed response.

>
> Shouldn't the check be implemented along the following lines?
>
>        if (memsize == 32 && (align % 32) == 0)
>          align_bits = 256;
>        else if ((memsize == 16 || memsize == 32) && (align % 16) == 0)
>          align_bits = 128;
>        else if (memsize >= 8 && (align % 8) == 0)
>          align_bits = 64;
>        else
>          align_bits = 0;

This looks OK to me. Looking at the ISA documents and the variants of
the vldn instructions your summary is correct. The alignment specifier
should not be greater than the memory size being transferred and
checking this in this form is OK .

regards,
Ramana



>
> Bye,
> Ulrich
>
> --
>  Dr. Ulrich Weigand
>  GNU Toolchain for Linux on System z and Cell BE
>  Ulrich.Weigand@de.ibm.com
>

Re: [Patch ARM] Fix PR51819.

[Ulrich Weigand]

Ramana Radhakrishnan wrote:

> This looks OK to me. Looking at the ISA documents and the variants of
> the vldn instructions your summary is correct. The alignment specifier
> should not be greater than the memory size being transferred and
> checking this in this form is OK .

Thanks, Ramana!  I've checked in the following patch.

Bye,
Ulrich


2012-04-16  Ulrich Weigand  <ulrich.weigand@linaro.org>

	* config/arm/arm.c (arm_print_operand): Fix invalid alignment
	hints for 'A' operand types.

=== modified file 'gcc/config/arm/arm.c'
--- gcc/config/arm/arm.c	2012-03-29 10:35:24 +0000
+++ gcc/config/arm/arm.c	2012-03-31 15:29:54 +0000
@@ -17880,9 +17880,9 @@
 	memsize = MEM_SIZE (x);
 
 	/* Only certain alignment specifiers are supported by the hardware.  */
-	if (memsize == 16 && (align % 32) == 0)
+	if (memsize == 32 && (align % 32) == 0)
 	  align_bits = 256;
-	else if (memsize == 16 && (align % 16) == 0)
+	else if ((memsize == 16 || memsize == 32) && (align % 16) == 0)
 	  align_bits = 128;
 	else if (memsize >= 8 && (align % 8) == 0)
 	  align_bits = 64;


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com