diff mbox

[Vax] zero/sign extend patterns need to be SUBREG aware

Message ID 1434448728-13318-1-git-send-email-james.greenhalgh@arm.com
State New
Headers show

Commit Message

James Greenhalgh June 16, 2015, 9:58 a.m. UTC 
Hi,

The testcase in this patch, from libgcc, causes an ICE in the Vax
build.

The problem is that the sign and zero extend patterns in vax.md
do not expect to see SUBREG rtx patterns, but use register_operand
as the predicate. Forwprop isn't doing anything wrong or unusual
in the testcase, so I think this is just an oversight in the Vax
patterns.

The problem is that the condition for this pattern treats any
operands[1] which is not a REG_P as a MEM, which is invalid
for SUBREG. In the patch we modify the condition to catch
anything which is !MEM_P before treating operands[1] as a MEM.

As far as I know, reload is going to get rid of these SUBREGs
for us, so we don't need to modify the output statement.

Tested that this restores the VAX build and that the code-gen is
sensible for the testcase.

OK?

Thanks,
James

---
gcc/

2015-06-16  James Greenhalgh  <james.greenhalgh@arm.com>

	* config/vax/vax.md: Adjust sign/zero extend patterns to
	handle SUBREGs in operands[1].

gcc/testsuite/

2015-06-16  James Greenhalgh  <james.greenhalgh@arm.com>

	* gcc.target/vax/bswapdi-1.c: New.

Comments

Jan-Benedict Glaw June 19, 2015, 3:51 p.m. UTC | #1 
Hi James,

On Tue, 2015-06-16 10:58:48 +0100, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> The testcase in this patch, from libgcc, causes an ICE in the Vax
> build.
[...]
> As far as I know, reload is going to get rid of these SUBREGs
> for us, so we don't need to modify the output statement.
> 
> Tested that this restores the VAX build and that the code-gen is
> sensible for the testcase.
> 
> OK?

Looks good to me, but Matt has to ACK this fix.

MfG, JBG
Matt Thomas June 19, 2015, 4:30 p.m. UTC | #2 
> On Jun 19, 2015, at 8:51 AM, Jan-Benedict Glaw <jbglaw@lug-owl.de> wrote:
> 
> Hi James,
> 
> On Tue, 2015-06-16 10:58:48 +0100, James Greenhalgh <james.greenhalgh@arm.com> wrote:
>> The testcase in this patch, from libgcc, causes an ICE in the Vax
>> build.
> [...]
>> As far as I know, reload is going to get rid of these SUBREGs
>> for us, so we don't need to modify the output statement.
>> 
>> Tested that this restores the VAX build and that the code-gen is
>> sensible for the testcase.
>> 
>> OK?
> 
> Looks good to me, but Matt has to ACK this fix.

I so ACK.
James Greenhalgh June 29, 2015, 3:19 p.m. UTC | #3 
On Fri, Jun 19, 2015 at 05:30:21PM +0100, Matt Thomas wrote:
> 
> > On Jun 19, 2015, at 8:51 AM, Jan-Benedict Glaw <jbglaw@lug-owl.de> wrote:
> > 
> > Hi James,
> > 
> > On Tue, 2015-06-16 10:58:48 +0100, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> >> The testcase in this patch, from libgcc, causes an ICE in the Vax
> >> build.
> > [...]
> >> As far as I know, reload is going to get rid of these SUBREGs
> >> for us, so we don't need to modify the output statement.
> >> 
> >> Tested that this restores the VAX build and that the code-gen is
> >> sensible for the testcase.
> >> 
> >> OK?
> > 
> > Looks good to me, but Matt has to ACK this fix.
> 
> I so ACK.
> 

Now that this has had a few days sitting on trunk without seeing any
complaints, would you mind if I backported it to the GCC 5 branch?

Thanks,
James
Matt Thomas June 29, 2015, 3:21 p.m. UTC | #4 
> On Jun 29, 2015, at 8:19 AM, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> 
> Now that this has had a few days sitting on trunk without seeing any
> complaints, would you mind if I backported it to the GCC 5 branch?

I don’t have a problem with that.
Jan-Benedict Glaw June 29, 2015, 6:26 p.m. UTC | #5 
On Mon, 2015-06-29 08:21:16 -0700, Matt Thomas <matt@3am-software.com> wrote:
> 
> > On Jun 29, 2015, at 8:19 AM, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> > 
> > Now that this has had a few days sitting on trunk without seeing any
> > complaints, would you mind if I backported it to the GCC 5 branch?
> 
> I don’t have a problem with that.  

Not at all!

MfG, JBG
diff mbox

Patch

diff --git a/gcc/config/vax/vax.md b/gcc/config/vax/vax.md
index 44d162f..d5caa15 100644
--- a/gcc/config/vax/vax.md
+++ b/gcc/config/vax/vax.md
@@ -780,7 +780,7 @@ 
 	(match_operand:SI 3 "general_operand" "g"))]
    "(INTVAL (operands[1]) == 8 || INTVAL (operands[1]) == 16)
    && INTVAL (operands[2]) % INTVAL (operands[1]) == 0
-   && (REG_P (operands[0])
+   && (!MEM_P (operands[0])
        || ! mode_dependent_address_p (XEXP (operands[0], 0),
 				       MEM_ADDR_SPACE (operands[0])))"
   "*
@@ -809,7 +809,7 @@ 
 			 (match_operand:SI 3 "const_int_operand" "n")))]
   "(INTVAL (operands[2]) == 8 || INTVAL (operands[2]) == 16)
    && INTVAL (operands[3]) % INTVAL (operands[2]) == 0
-   && (REG_P (operands[1])
+   && (!MEM_P (operands[1])
        || ! mode_dependent_address_p (XEXP (operands[1], 0),
 				      MEM_ADDR_SPACE (operands[1])))"
   "*
@@ -837,7 +837,7 @@ 
 			 (match_operand:SI 3 "const_int_operand" "n")))]
   "(INTVAL (operands[2]) == 8 || INTVAL (operands[2]) == 16)
    && INTVAL (operands[3]) % INTVAL (operands[2]) == 0
-   && (REG_P (operands[1])
+   && (!MEM_P (operands[1])
        || ! mode_dependent_address_p (XEXP (operands[1], 0),
 				      MEM_ADDR_SPACE (operands[1])))"
   "*
diff --git a/gcc/testsuite/gcc.target/vax/bswapdi-1.c b/gcc/testsuite/gcc.target/vax/bswapdi-1.c
new file mode 100644
index 0000000..c658d91
--- /dev/null
+++ b/gcc/testsuite/gcc.target/vax/bswapdi-1.c
@@ -0,0 +1,13 @@ 
+typedef int DItype __attribute__ ((mode (DI)));
+DItype
+__bswapdi2 (DItype u)
+{
+  return ((((u) & 0xff00000000000000ull) >> 56)
+   | (((u) & 0x00ff000000000000ull) >> 40)
+   | (((u) & 0x0000ff0000000000ull) >> 24)
+   | (((u) & 0x000000ff00000000ull) >> 8)
+   | (((u) & 0x00000000ff000000ull) << 8)
+   | (((u) & 0x0000000000ff0000ull) << 24)
+   | (((u) & 0x000000000000ff00ull) << 40)
+   | (((u) & 0x00000000000000ffull) << 56));
+}