[llvm] [DAG] Add legalization handling for ABDS/ABDU (PR #92576)

Björn Pettersson via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 12 05:00:43 PDT 2024


bjope wrote:

For info: We have seen some problems with test/CodeGen/AArch64/abds.ll downstream after this patch was reapplied.

Looking at debug/print-after-all traces for @abd_ext_i128 I see that the ISel may create nodes in different order:
```
Legalizing node: t25: i128 = abds t9, t10
Analyzing result type: i128
Expand integer result: t25: i128 = abds t9, t10
Creating new node: t28: i32 = setcc t9, t10, setgt:ch
Creating new node: t29: i128 = sub t10, t9
Creating new node: t30: i128 = sub t9, t10
Creating new node: t31: i128 = select t28, t30, t29
```
vs
```
Legalizing node: t25: i128 = abds t9, t10
Analyzing result type: i128
Expand integer result: t25: i128 = abds t9, t10
Creating new node: t28: i32 = setcc t9, t10, setgt:ch
Creating new node: t29: i128 = sub t9, t10
Creating new node: t30: i128 = sub t10, t9
Creating new node: t31: i128 = select t28, t29, t30
```
And then it looks like the ISel scheduler may pick nodes in different order? And at least we get slightly different VReg usage in the MIR output after ISel. Later, at MachineCSE there is a limit on how far back it looks for common subexpressions, and we might end up either doing a CSE or not, depending on what the MIR looks like.

My current best thinking is that it is a difference depending on if I compile LLVM with clang or gcc. And then it probably is things like this in TargetLowering.cpp
```
  // abdu(lhs, rhs) -> or(usubsat(lhs,rhs), usubsat(rhs,lhs))
  if (!IsSigned && isOperationLegal(ISD::USUBSAT, VT))
    return DAG.getNode(ISD::OR, dl, VT,
                       DAG.getNode(ISD::USUBSAT, dl, VT, LHS, RHS),
                       DAG.getNode(ISD::USUBSAT, dl, VT, RHS, LHS));
```
and/or things like this in DagCombiner.cpp:
```
    SDValue ABD = DAG.getNode(ABDOpcode, DL, MaxVT,
                              DAG.getNode(ISD::TRUNCATE, DL, MaxVT, Op0),
                              DAG.getNode(ISD::TRUNCATE, DL, MaxVT, Op1));
```
that makes a difference since the order in which arguments are evaluated isn't well-defined.
Generally, nestled calls to getNode like that is nasty when comparing results from LLVM built with gcc vs clang.

https://github.com/llvm/llvm-project/pull/92576


More information about the llvm-commits mailing list