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

[Bradley Smith via Phabricator via llvm-commits]

bsmith created this revision.
bsmith added reviewers: paulwalker-arm, peterwaller-arm, joechrisellis, david-arm, huntergr.
Herald added subscribers: psnobl, hiraditya, kristof.beyls, tschuett.
Herald added a reviewer: efriedma.
bsmith requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

The complex selection pattern for add/sub shifted immediates is
incorrect in it's handling of incoming constant values, in that it
does not properly anticipate the values to be signed extended to
32-bits.

Co-authored-by: Graham Hunter <graham.hunter at arm.com>


Repository:
  rG LLVM Github Monorepo

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
@@ -3111,15 +3111,29 @@
 
     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, so need to
+      // check for "negative" values when shifting.
       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) &&
+                  ((int32_t)ImmVal >= -32768) &&
+                  ((int32_t)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.342722.patch
Type: text/x-patch
Size: 2735 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210504/263536f9/attachment.bin>