[PATCH] D153164: [AArch64][GlobalISel] Select G_UADDE/G_SADDE/G_USUBE/G_SSUBE

Thu Jun 22 11:10:47 PDT 2023

tobias-stadler added a comment.

In D153164#4440902 <https://reviews.llvm.org/D153164#4440902>, @barannikov88 wrote:

>> Register getCarryOutReg();
>
> For subtraction this is actually a borrow-out. This naming can be confusing.
> Has anyone though about deviating from SelectionDAG here and inverting the meaning of the flag for subtractions, so that it is actually a carry for both add/sub?
> I understand that we can't just invert the meaning, because many clients (including downstream) rely on the current behavior.
> So maybe via deprecation? UADDE/UADDO aren't good names anyway (I still can't figure out what do E and O letters mean).
>
> (not related to this review)
> I also had this idea about adding a variant of all ALU operations with status flags output (NZVC), and using this output in G_BRCOND_S/G_SELECT_S along with condition code to test (e.g. "overflow set, zero clear").
> Carry-producing instructions could then be replaced by these more general instructions.
> This allows eliminating G_ICMP in many cases (and actually make G_ICMP redundant because it could be replaced with G_SUB_S with status flags output).
> For example `if ((x & y) <= 0)` could be translated into G_AND_S + G_BRCOND_S with "not positive" condition code (N == 0 || Z == 0), //if the target has one//.
> I've implemented this as custom nodes in a downstream SelectionDAG as proof of concept. It works, but there are some subtleties, and it is somewhat machine dependent (e.g. RISC-V does not have status flags, as far as I know).
>
> I'm not very familiar with GISel, nor I have time to try to implement this idea upstream, but maybe it will interest someone.

As far as my patch is concerned: CarryIn/CarryOut is consistent with the terminology used in MachineIRBuilder, the docs and other places I have seen, so I'd prefer to keep it this way.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153164