[PATCH] D105853: [ISel] Expand saddsat and ssubsat via asr and xor

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/sadd_sat.ll:16
-; CHECK-NEXT:    mov w9, #2147483647
-; CHECK-NEXT:    cmp w8, #0 // =0
-; CHECK-NEXT:    cinv w8, w9, ge
----------------
We could save two instructions here without really changing the algorithm if we use the carry flag from the adds, i.e. `(UADDO LHS, RHS)`, instead of checking whether the result of the adds is negative.  That's the same number of instructions as your new sequence, but with a shorter critical path, so probably better?


================
Comment at: llvm/test/CodeGen/X86/ssub_sat.ll:33
+; X64-NEXT:    subl %esi, %eax
+; X64-NEXT:    cmovol %ecx, %eax
 ; X64-NEXT:    retq
----------------
dmgreen wrote:
> Now that I look again, some of the individual X86 tests have increased, although the total size of the files has gone down.
> 
> What do we usually do about such things? I could just custom lower this in ARM/AArch64, but it appears to be better in general.
There are basically two options: use a target hook, or try to pick some reasonable default and write custom lowering for targets where the default is bad.  There's no great answer here.

Note that even the existing x86 code isn't really ideal: if we replace the addl with leal, we can save an instruction by merging the cmpl with the subl.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105853/new/

https://reviews.llvm.org/D105853