[AArch64] Generate CMN when comparing a short int with minus

[David.Xu]

Summary

This patch generates cmn instruction when comparing a short int with minus

ldrh w8, [x0]
orr w9, wzr, #0xffff
cmp w8, w9

to

ldrsh w8, [x0]
cmn w8, #1

This patch fixes the issue by signed extending the operand.The performance test shows there are some improvements:
spec.cpu2000.ref.253_perlbmk -2.15%,
spec.cpu2006.ref.482_sphinx3 -1.59%.

As to why this patch just deals with short int, please look at the comments in the patch.

Please have a look.

David

[t.p.northover]

Hi David,

This looks fine mostly; just a couple of minor nits.

Cheers.

Tim.

lib/Target/AArch64/AArch64ISelLowering.cpp

1084–1117

I think you should probably mention the fundamental property you're relying on here: that (zext LHS) == (zext RHS) if and only if (sext LHS) == (sext RHS).

Your checks are all aimed at determining

• whether LHS and RHS really are zexted
• whether making the transformation would be profitable.

1096

int16_t would be better here.

[David.Xu]

Comments about fundamental property are added in the patch.

Please have a look.

David

[David.Xu]

ping ....

David

[mcrosier]

LGTM, once the new comments are addressed. Any additional concerns can be addressed in post-commit reviews.

lib/Target/AArch64/AArch64ISelLowering.cpp

1084

Wording improvements:

The imm operand of ADDS is an unsigned immediate, in the range 0 to 4095. For the i8 operand, the largest immediate is 255, so this can be easily encoded in the compare instruction. For i16 operand, however, the largest immediate cannot be encoded in the compare.

Therefore, use a sign extending load and cmn to avoid materializing the -1 constant. For example,

movz w1, #65535
ldrh w0, [x0, #0]
cmp w0, w1

>

ldrsh w0, [x0, #0]
cmn w0, #1

Fundamental, we're relying on the property that (zext LHS) == (zext RHS) if and only if (sext LHS) == (sext RHS). The checks are in place to ensure both the LHS and RHS are truely zero extended and to make sure the transformation is profitable.

1097

clang-format?

[mcrosier]

This was committed in r216651.

lib/Target/AArch64/AArch64ISelLowering.cpp

 
 static SDValue getAArch64Cmp(SDValue LHS, SDValue RHS, ISD::CondCode CC,
                              SDValue &AArch64cc, SelectionDAG &DAG, SDLoc dl) {
+  SDValue Cmp;
+  AArch64CC::CondCode AArch64CC;
   if (ConstantSDNode *RHSC = dyn_cast<ConstantSDNode>(RHS.getNode())) {
     EVT VT = RHS.getValueType();
     uint64_t C = RHSC->getZExtValue();

       }
     }
   }
-
-  SDValue Cmp = emitComparison(LHS, RHS, CC, dl, DAG);
-  AArch64CC::CondCode AArch64CC = changeIntCCToAArch64CC(CC);
+  // Because the imm operand of ADDS is an unsigned immediate, in the range 0 to
+  // 4095.For i8 operand, the largest immediate is 255, it can be handled well.
+  // For i16 operand, the largest immediate is larger than 4095, it should be
+  // optimized here.
+  // eg:movz w1, #65535; ldrh w0, [x0, #0]; cmp w0, w1
+  // => ldrsh w0, [x0, #0]; cmn w0, #1
+  // In fact, (zext LHS) cmp (zext RHS) is changed to (sext LHS) cmp (sext RHS).
+  // Checking whether it is profitable to do the optimization:
+  // 1.LHS and RHS must be 16 bits zero extend operands, and then sign extend
+  //   them
+  // 2.LHS also must be a LOADNODE and have just one use, in order to combine
+  //   ldrh with sign_extend_inreg to ldrsh.
+  if ((CC == ISD::SETEQ || CC == ISD::SETNE) && isa<ConstantSDNode>(RHS)) {
+    if ((cast<ConstantSDNode>(RHS)->getZExtValue() >> 16 == 0) &&
+        isa<LoadSDNode>(LHS) &&
+        cast<LoadSDNode>(LHS)->getExtensionType() == ISD::ZEXTLOAD &&
+        cast<LoadSDNode>(LHS)->getMemoryVT() == MVT::i16 &&
+        LHS.getNode()->hasNUsesOfValue(1, 0)) {
+      int16_t ValueofRHS = cast<ConstantSDNode>(RHS)->getZExtValue();
+      if (ValueofRHS < 0 && isLegalArithImmed(-ValueofRHS)) {
+        SDValue SExt =
+            DAG.getNode(ISD::SIGN_EXTEND_INREG, dl, LHS.getValueType(), LHS,
+                        DAG.getValueType(MVT::i16));
+        Cmp = emitComparison(
+            SExt, DAG.getConstant(ValueofRHS, RHS.getValueType()), CC, dl, DAG);
+        AArch64CC = changeIntCCToAArch64CC(CC);
+        AArch64cc = DAG.getConstant(AArch64CC, MVT::i32);
+        return Cmp;
+      }
+    }
+  }
+  Cmp = emitComparison(LHS, RHS, CC, dl, DAG);
+  AArch64CC = changeIntCCToAArch64CC(CC);
   AArch64cc = DAG.getConstant(AArch64CC, MVT::i32);
   return Cmp;
 }

test/CodeGen/AArch64/cmpwithshort.ll

+; RUN: llc -O3 -march=aarch64 < %s | FileCheck %s 
+
+define i16 @test_1cmp_signed_1(i16* %ptr1) {
+; CHECK-LABLE: @test_1cmp_signed_1
+; CHECK: ldrsh
+; CHECK-NEXT: cmn
+entry:
+  %addr = getelementptr inbounds i16* %ptr1, i16 0
+  %val = load i16* %addr, align 2
+  %cmp = icmp eq i16 %val, -1
+  br i1 %cmp, label %if, label %if.then
+if:
+  ret i16 1
+if.then:
+  ret i16 0
+}
+
+define i16 @test_1cmp_signed_2(i16* %ptr1) {
+; CHECK-LABLE: @test_1cmp_signed_2
+; CHECK: ldrsh
+; CHECK-NEXT: cmn
+entry:
+  %addr = getelementptr inbounds i16* %ptr1, i16 0
+  %val = load i16* %addr, align 2
+  %cmp = icmp sge i16 %val, -1
+  br i1 %cmp, label %if, label %if.then
+if:
+  ret i16 1
+if.then:
+  ret i16 0
+}
+
+define i16 @test_1cmp_unsigned_1(i16* %ptr1) {
+; CHECK-LABLE: @test_1cmp_unsigned_1
+; CHECK: ldrsh
+; CHECK-NEXT: cmn
+entry:
+  %addr = getelementptr inbounds i16* %ptr1, i16 0
+  %val = load i16* %addr, align 2
+  %cmp = icmp uge i16 %val, -1
+  br i1 %cmp, label %if, label %if.then
+if:
+  ret i16 1
+if.then:
+  ret i16 0
+}