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

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Dragan Mladjenovic
From: "Dragan Mladjenovic" <[hidden email]>

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  <[hidden email]>

        * 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(-)

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";
 }
--
1.9.1

Reply | Threaded
Open this post in threaded view
|

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

Jeff Law
On 11/12/19 7:56 AM, Dragan Mladjenovic wrote:

> From: "Dragan Mladjenovic" <[hidden email]>
>
> 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  <[hidden email]>
>
> * 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

Reply | Threaded
Open this post in threaded view
|

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

Dragan Mladjenovic

On 12.11.2019. 16:29, Jeff Law wrote:

> On 11/12/19 7:56 AM, Dragan Mladjenovic wrote:
>> From: "Dragan Mladjenovic" <[hidden email]>
>>
>> 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  <[hidden email]>
>>
>> * 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