[PATCH] D101833: [AArch64][SVE] Fix missed immediate selection due to mishandling of signedness

[Bradley Smith via Phabricator via llvm-commits]

bsmith updated this revision to Diff 343005.
bsmith marked 2 inline comments as done.
bsmith added a comment.

- Fix clang format issues
- Move range check to be an assert since it should always be true


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101833

Files:
  llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
  llvm/test/CodeGen/AArch64/sve-int-imm.ll


Index: llvm/test/CodeGen/AArch64/sve-int-imm.ll
===================================================================
--- llvm/test/CodeGen/AArch64/sve-int-imm.ll
+++ llvm/test/CodeGen/AArch64/sve-int-imm.ll
@@ -75,6 +75,26 @@
   ret <vscale x 2 x i64> %res
 }
 
+define <vscale x 16 x i8> @add_i8_signedness(<vscale x 16 x i8> %a) {
+; CHECK-LABEL: add_i8_signedness
+; CHECK: add  z0.b, z0.b, #255
+; CHECK-NEXT: ret
+  %elt = insertelement <vscale x 16 x i8> undef, i8 255, i32 0
+  %splat = shufflevector <vscale x 16 x i8> %elt, <vscale x 16 x i8> undef, <vscale x 16 x i32> zeroinitializer
+  %res =  add <vscale x 16 x i8> %a, %splat
+  ret <vscale x 16 x i8> %res
+}
+
+define <vscale x 8 x i16> @add_i16_signedness(<vscale x 8 x i16> %a) {
+; CHECK-LABEL: add_i16_signedness
+; CHECK: add  z0.h, z0.h, #65280
+; CHECK-NEXT: ret
+  %elt = insertelement <vscale x 8 x i16> undef, i16 65280, i32 0
+  %splat = shufflevector <vscale x 8 x i16> %elt, <vscale x 8 x i16> undef, <vscale x 8 x i32> zeroinitializer
+  %res =  add <vscale x 8 x i16> %a, %splat
+  ret <vscale x 8 x i16> %res
+}
+
 ; SUBR
 define <vscale x 16 x i8> @subr_i8_low(<vscale x 16 x i8> %a) {
 ; CHECK-LABEL: subr_i8_low
Index: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
===================================================================
--- llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
+++ llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
@@ -3106,20 +3106,32 @@
 
 bool AArch64DAGToDAGISel::SelectSVEAddSubImm(SDValue N, MVT VT, SDValue &Imm, SDValue &Shift) {
   if (auto CNode = dyn_cast<ConstantSDNode>(N)) {
-    const int64_t ImmVal = CNode->getZExtValue();
+    const int64_t ImmVal = CNode->getSExtValue();
     SDLoc DL(N);
 
     switch (VT.SimpleTy) {
     case MVT::i8:
+      // Can always select i8s, no shift, mask the immediate value to
+      // deal with sign-extended value from lowering.
+      Shift = CurDAG->getTargetConstant(0, DL, MVT::i32);
+      Imm = CurDAG->getTargetConstant(ImmVal & 0xFF, DL, MVT::i32);
+      return true;
+    case MVT::i16:
+      // i16 values get sign-extended to 32-bits during lowering.
       if ((ImmVal & 0xFF) == ImmVal) {
         Shift = CurDAG->getTargetConstant(0, DL, MVT::i32);
         Imm = CurDAG->getTargetConstant(ImmVal, DL, MVT::i32);
         return true;
+      } else if ((ImmVal & 0xFF) == 0) {
+        assert((ImmVal >= -32768) && (ImmVal <= 32512));
+        Shift = CurDAG->getTargetConstant(8, DL, MVT::i32);
+        Imm = CurDAG->getTargetConstant((ImmVal / 256) & 0xFF, DL, MVT::i32);
+        return true;
       }
       break;
-    case MVT::i16:
     case MVT::i32:
     case MVT::i64:
+      // Range of immediate won't trigger signedness problems for 32/64b.
       if ((ImmVal & 0xFF) == ImmVal) {
         Shift = CurDAG->getTargetConstant(0, DL, MVT::i32);
         Imm = CurDAG->getTargetConstant(ImmVal, DL, MVT::i32);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D101833.343005.patch
Type: text/x-patch
Size: 2899 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210505/caa5fea2/attachment.bin>