[PATCH] D135302: [AArch64] Support SETCCCARRY lowering

[Eli Friedman via Phabricator via llvm-commits]

efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:417
   setOperationAction(ISD::JumpTable, MVT::i64, Custom);
+  setOperationAction(ISD::SETCCCARRY, MVT::i32, Custom);
+  setOperationAction(ISD::SETCCCARRY, MVT::i64, Custom);
----------------
fzhinkin wrote:
> efriedma wrote:
> > Do we ever actually use SETCCCARRY for i32 on targets where i64 is legal?
> `SelectionDAGLegalize::LegalizeOp` uses default case to handle SETCCCARRY and calls `TLI.getOperationAction`  with SETCCCARRY's type (which is `i32`). I guess that this question never arose previously because for other targets there were either no 64-bit registers (ARM), or because various value types were supported (X86, M86k). 
> 
> Not sure if it should be fixed along with this change or via separate commit.
Oh, I see what you mean.  The current way is fine; just wasn't really thinking about the output type.


================
Comment at: llvm/test/CodeGen/AArch64/i128-cmp.ll:24
 ; CHECK-NEXT:    eor x9, x0, x2
 ; CHECK-NEXT:    orr x8, x9, x8
 ; CHECK-NEXT:    cmp x8, #0
----------------
fzhinkin wrote:
> efriedma wrote:
> > fzhinkin wrote:
> > > Also checking if it's possible to use setcccarry to lower setcc eq/ne. It seems like it may improve code on x86, ARM and AArch64, but I'd prefer fixing it separately.
> > Not sure how you could use setcccarry for equality; the carry bit doesn't contain enough information.  You need something more like the AArch64 ccmp.
> Correct me if I'm wrong, but SETCCCARRY is like a regular SETCC which takes carry flag into account when comparing values. So we can generate the carry flag by comparing/subtracting registers holding least significant bits of i128 and then use SETCCCARRY to compare registers holding most significant bits (exactly how it works for other CCs).
> 
> It would be something like:
> ```
> cmp x2, x0
> sbcs xzr, x3, x1
> cset w0, eq
> ```
> 
> Of course, on AArch64 `sbcs` in this case could be replaced with `ccmp`, but by using SETCCCARRY we can also improve code generated for other targets (X86 in particular). For targets supporting conditional comparisons (ARM, AArch64) SETCCARRY could then be replaced with more appropriate instruction.
> ```cmp x2, x0
> sbcs xzr, x3, x1
> cset w0, eq```

Equality would generally mean x2 equals x0 and x3 equals x1.  This throws away the equality bit of the first comparison, and adding the carry bit to the second comparison just means it returns "equal" in some cases where x3 and x1 aren't equal.

If you try to run some actual examples, I'm sure you'll see what I mean.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135302