Re: [PATCH] ARM cmpsi2_addneg fixes (PR target/89506)

[Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>]

Hi Jakub,

>> This is overcomplicating something simple - adds/subs are completely
>> symmetrical on all Arm targets. So for addition you simply place adds 
>> alternative first and then subs. For comparison/subtraction place the 
>
> As I wrote, that is what I have done, 

That's not what your proposed patch does:

-  "TARGET_32BIT && INTVAL (operands[2]) == -INTVAL (operands[3])"
-  "@
-   adds%?\\t%0, %1, %3
-   subs%?\\t%0, %1, #%n3"
+  "TARGET_32BIT
+   && (INTVAL (operands[2])
+       == trunc_int_for_mode (-INTVAL (operands[3]), SImode))"
+{
+  /* For 0 and INT_MIN it is essential that we use subs, as adds
+     will result in different condition codes (like cmn rather than
+     like cmp).  For other immediates, we should choose whatever
+     will have smaller encoding.  */
+  if (operands[2] == const0_rtx
+      || INTVAL (operands[2]) == -HOST_WIDE_INT_C (0x80000000)
+      || which_alternative == 1)
+    return "subs%?\\t%0, %1, #%n3";
+  else
+    return "adds%?\\t%0, %1, %3";
+}

Adds/subs were in the incorrect order before and should simply be swapped
rather than replaced by complex C code (which would be unique just to this pattern
when there are lot of similar patterns that do the right thing already).

> and regtest revealed two code size
> regressions because of that.  Is -1 vs. 1 the only case of immediate
> valid for both "I" and "L" constraints where the former is longer than the
> latter?

Yes -1 is the only case which can result in larger code on Thumb-2, so -1 should
be disallowed by the I constraint (or even better, the underlying query). That way
it will work correctly for all add/sub patterns, not just this one.

Wilco