[PATCH] D120481: [AArch64] Try to re-use extended operand for SETCC with v16i8 operands.

[Dave Green via Phabricator via llvm-commits]

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

In D120481#3634566 <https://reviews.llvm.org/D120481#3634566>, @fhahn wrote:

> In D120481#3632283 <https://reviews.llvm.org/D120481#3632283>, @dmgreen wrote:
>
>> This sounds good as far as I can tell. If we are dealing with a dag combine, do we need to worry about odd types at all?
>
> Do you mean worry in terms of whether it is optimal? There are a few test cases with odd types, and it seems like they are handled OK on that front.

I meant for awkward types like i23 and i128. They don't have to be optimal, so long as they work OK. Just so long as we have a couple of tests, to make sure it doesnt run into problems.

The change sounds good with a small simplification suggestion. LGTM



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:18113
+                                        Op->getOperand(0));
+  if (Op0SExt && isSignedIntSetCC(CC)) {
+    Op0ExtV = SDValue(Op0SExt, 0);
----------------
I think this might be able to combine the if blocks together for the ne/eq conditions:
```
if (Op0SExt && (isSignedIntSetCC(CC) || isIntEqualitySetCC(CC))) {
  Op0ExtV = SDValue(Op0SExt, 0);
  Op1ExtV = DAG.getNode(ISD::SIGN_EXTEND, DL, UseMVT, Op->getOperand(1));
} else if (Op0ZExt && (isUnsignedIntSetCC(CC) || isIntEqualitySetCC(CC))) {
  Op0ExtV = SDValue(Op0ZExt, 0);
  Op1ExtV = DAG.getNode(ISD::ZERO_EXTEND, DL, UseMVT, Op->getOperand(1));
} else
  return SDValue();
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120481