[PATCH][ARM] PR target/68648: Fold NOT of CONST_INT in andsi_iorsi3_notsi splitter

[Kyrill Tkachov]

Hi Ramana,

On 15/12/15 21:20, Ramana Radhakrishnan wrote:
> On 07/12/15 10:39, Kyrill Tkachov wrote:
>> Hi all,
>>
>> In this PR we ICE because during post-reload splitting we generate the insn:
>> (insn 27 26 11 2 (set (reg:SI 0 r0 [orig:121 D.4992 ] [121])
>>          (and:SI (not:SI (const_int 1 [0x1]))
>>              (reg:SI 0 r0 [orig:121 D.4992 ] [121])))
>>       (nil))
>>
>>
>> The splitter at fault is andsi_iorsi3_notsi that accepts a const_int in operands[3]
>> and outputs (not (match_dup 3)). It should really be trying to constant fold the result
>> first.  This patch does that by calling simplify_gen_unary to generate the complement
>> of operands[3] if it's a register or the appropriate const_int rtx with the correct
>> folded result that will still properly match the arm bic-immediate instruction.
>>
>> Bootstrapped and tested on arm-none-eabi.
>>
>> Is this ok for trunk?
>>
>> This appears on GCC 4.9 and GCC 5 and I'll be testing the fix there as well.
>> Ok for those branches if testing is successful?
>>
>> Thanks,
>> Kyrill
>>
>> 2015-12-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>      PR target/68648
>>      * config/arm/arm.md (*andsi_iorsi3_notsi): Try to simplify
>>      the complement of operands[3] during splitting.
>>
>> 2015-12-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>      PR target/68648
>>      * gcc.c-torture/execute/pr68648.c: New test.
>>
>> arm-not-int.patch
>>
>>
>> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
>> index 2b48bbaf034b286d723536ec2aa6fe0f9b312911..dfb75c5f11c66c6b4a34ff3071b5a0957c3512cb 100644
>> --- a/gcc/config/arm/arm.md
>> +++ b/gcc/config/arm/arm.md
>> @@ -3274,8 +3274,22 @@ (define_insn_and_split "*andsi_iorsi3_notsi"
>>     "#"   ; "orr%?\\t%0, %1, %2\;bic%?\\t%0, %0, %3"
>>     "&& reload_completed"
>>     [(set (match_dup 0) (ior:SI (match_dup 1) (match_dup 2)))
>> -   (set (match_dup 0) (and:SI (not:SI (match_dup 3)) (match_dup 0)))]
>> -  ""
>> +   (set (match_dup 0) (and:SI (match_dup 4) (match_dup 5)))]
>> +  {
>> +     /* If operands[3] is a constant make sure to fold the NOT into it
>> +	to avoid creating a NOT of a CONST_INT.  */
>> +    rtx not_rtx = simplify_gen_unary (NOT, SImode, operands[3], SImode);
>> +    if (CONST_INT_P (not_rtx))
>> +      {
>> +	operands[4] = operands[0];
>> +	operands[5] = not_rtx;
>> +      }
>> +    else
>> +      {
>> +	operands[5] = operands[0];
>> +	operands[4] = not_rtx;
>> +      }
> Watch out indentation here - I'm not sure if this is your mail client or mine causing this sort of thing.

Looks fine in two of my editors and also in the svn diff output, so I think there
must be something on your end.

>
> Ok with that double checked.

Thanks, I've committed it as r231675.
Will backport to the branches after some time on trunk.

Kyrill

> regards
> Ramana
>
>> +  }
>>     [(set_attr "length" "8")
>>      (set_attr "ce_count" "2")
>>      (set_attr "predicable" "yes")
>> diff --git a/gcc/testsuite/gcc.c-torture/execute/pr68648.c b/gcc/testsuite/gcc.c-torture/execute/pr68648.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..fc66806a99a0abef7bd517ae2f5200b387e69ce4
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.c-torture/execute/pr68648.c
>> @@ -0,0 +1,20 @@
>> +int __attribute__ ((noinline))
>> +foo (void)
>> +{
>> +  return 123;
>> +}
>> +
>> +int __attribute__ ((noinline))
>> +bar (void)
>> +{
>> +  int c = 1;
>> +  c |= 4294967295 ^ (foo () | 4073709551608);
>> +  return c;
>> +}
>> +
>> +int
>> +main ()
>> +{
>> +  if (bar () != 0x83fd4005)
>> +    __builtin_abort ();
>> +}
>>