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
