diff mbox series

[MIPS] Sanitize the constant argument for rotr<mode>3

Message ID 1573570538-4992-1-git-send-email-dmladjenovic@wavecomp.com
State New
Headers show
Series [MIPS] Sanitize the constant argument for rotr<mode>3 | expand

Commit Message

Dragan Mladjenovic Nov. 12, 2019, 2:56 p.m. UTC
From: "Dragan Mladjenovic" <dmladjenovic@wavecomp.com>

This was dormant for quite some time, but it started happening for me
on gcc.c-torture/compile/pr65153.c sometime after r276645 for -mabi=32 linux runs.

The pattern accepts any SMALL_OPERAND constant value while it asserts during the final
that the value is in the mode size range. I this case it happens that combine_and_move_insns
during ira makes a pattern with negative "shift count" which fails at final stage.

This simple fix just truncates the constant operand to mode size the same as shift patterns.

gcc/ChangeLog:

2019-11-12  Dragan Mladjenovic  <dmladjenovic@wavecomp.com>

	* config/mips/mips.md (rotr<mode>3): Sanitize the constant argument
	instead of asserting its value.
---

Ok, for trunk and backport to gcc 9 and 8 branches?

 gcc/config/mips/mips.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jeff Law Nov. 12, 2019, 3:29 p.m. UTC | #1
On 11/12/19 7:56 AM, Dragan Mladjenovic wrote:
> From: "Dragan Mladjenovic" <dmladjenovic@wavecomp.com>
> 
> This was dormant for quite some time, but it started happening for me
> on gcc.c-torture/compile/pr65153.c sometime after r276645 for -mabi=32 linux runs.
> 
> The pattern accepts any SMALL_OPERAND constant value while it asserts during the final
> that the value is in the mode size range. I this case it happens that combine_and_move_insns
> during ira makes a pattern with negative "shift count" which fails at final stage.
> 
> This simple fix just truncates the constant operand to mode size the same as shift patterns.
> 
> gcc/ChangeLog:
> 
> 2019-11-12  Dragan Mladjenovic  <dmladjenovic@wavecomp.com>
> 
> 	* config/mips/mips.md (rotr<mode>3): Sanitize the constant argument
> 	instead of asserting its value.
> ---
> 
> Ok, for trunk and backport to gcc 9 and 8 branches?
OK.  But I'm not sure the formatting is right.  The bit-and operator
should be indented so that it lines up with the start of INTVAL (...).

jeff
Dragan Mladjenovic Nov. 13, 2019, 7:41 p.m. UTC | #2
On 12.11.2019. 16:29, Jeff Law wrote:
> On 11/12/19 7:56 AM, Dragan Mladjenovic wrote:
>> From: "Dragan Mladjenovic" <dmladjenovic@wavecomp.com>
>>
>> This was dormant for quite some time, but it started happening for me
>> on gcc.c-torture/compile/pr65153.c sometime after r276645 for -mabi=32 linux runs.
>>
>> The pattern accepts any SMALL_OPERAND constant value while it asserts during the final
>> that the value is in the mode size range. I this case it happens that combine_and_move_insns
>> during ira makes a pattern with negative "shift count" which fails at final stage.
>>
>> This simple fix just truncates the constant operand to mode size the same as shift patterns.
>>
>> gcc/ChangeLog:
>>
>> 2019-11-12  Dragan Mladjenovic  <dmladjenovic@wavecomp.com>
>>
>> 	* config/mips/mips.md (rotr<mode>3): Sanitize the constant argument
>> 	instead of asserting its value.
>> ---
>>
>> Ok, for trunk and backport to gcc 9 and 8 branches?
> OK.  But I'm not sure the formatting is right.  The bit-and operator
> should be indented so that it lines up with the start of INTVAL (...).
>

Thanks. Now I see it. Committed with fixed indention on trunk, gcc-9 and 
gcc-8 as r278152, r278154 and r278155 respectively.

Best regards,
Dragan
diff mbox series

Patch

diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index 658f5e6..1d63aca 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -5845,8 +5845,8 @@ 
   "ISA_HAS_ROR"
 {
   if (CONST_INT_P (operands[2]))
-    gcc_assert (INTVAL (operands[2]) >= 0
-		&& INTVAL (operands[2]) < GET_MODE_BITSIZE (<MODE>mode));
+    operands[2] = GEN_INT (INTVAL (operands[2])
+         & (GET_MODE_BITSIZE (<MODE>mode) - 1));
 
   return "<d>ror\t%0,%1,%2";
 }