[PATCH] D148249: [AArch64] Peep SELECT_CC patterns that match smin(a,0) and smax(a,0).

Thu Apr 13 14:28:13 PDT 2023

cameron.mcinally added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:8952
+    if ((CC == ISD::SETGT || CC == ISD::SETLT) && LHS == TVal &&
+        RHSC && RHSC->isZero() && CFVal && CFVal->isZero() &&
+        LHS.getValueType() == RHS.getValueType()) {
----------------
dmgreen wrote:
> Is this checking both operands for zero because they might still be different zeroes?
I wasn't certain a Constant 0 would share the same SDValue across nodes and already had the ConstantSDNodes from the code above. That was the only motivation. What do you think?

================
Comment at: llvm/test/CodeGen/AArch64/min-max-peep.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
+; RUN: llc -mtriple=aarch64-eabi %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-ISEL
+; RUN: llc -mtriple=aarch64-eabi %s -o - -mattr=cssc | FileCheck %s --check-prefixes=CHECK,CHECK-ISEL-CSSC
----------------
dmgreen wrote:
> CHECK would never match both +cssc base cases, but the CSSC cases should all be the same between global and sdag, from the look of it. Can they share the same prefix and remove CHECK?
I'll give that a shot. 

I copied this pattern from the existing min-max.ll test, but I now see that it only differs for i8, i16, and vectors. This change is restricted to i32 and i64.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148249