[AArch64ISelDAGToDAG] Supplement cases for ORRWrs/ORRXrs when calculating usefulbits

[TiehuZhang]

Summary

For the case,

    
t8: i32 = or t7, t4
t10: i32 = ORRWrs t8, t8, TargetConstant:i32<73>

the userfulbits of t8 should be full bits of itself (t8 | (t8 >> shiftConstant)). Current implementation doesn't catch the case and may get a wrong result. The patch supplements the missing case.

[mdchen]

llvm/test/CodeGen/AArch64/arm64-isel-or.ll

14

This function is irrelevant to the issue and could be removed I think.

59

The test case is not supposed to do output. Please remove the printf here; also the consequently unused declaration and other related elements.

[TiehuZhang]

update the testcase

[TiehuZhang]

ping

llvm/test/CodeGen/AArch64/arm64-isel-or.ll

59

Done, thanks.

[mdchen]

LGTM, but please wait for a couple of days for experts in this area to review: )

[sdesmalen]

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

2306

nit: Can you write it as:

case AArch64::ORRWrs:
case AArch64::ORRXrs:
  if (UserNode->getOperand(0) != Orig && UserNode->getOperand(1) == Orig)
    getUsefulBitsFromOrWith..(...);
  return;

That's a little easier to read.

llvm/test/CodeGen/AArch64/arm64-isel-or.ll

2

Instead of stopping after instruction selection, I think it's easier to just match the output asm directly, since you can now check that some of the instructions are no longer removed (because they were previously undef). If you use the update_llc_test_checks.py script, you can match the output directly.

13

You can simplify the IR by taking the value as input argument instead of loading it, and returning it instead of storing it.

Also dso_local and local_unnamed_addr can be removed.

16

Can you shorten the test-file by using a larger shift value (and less shift/ors)?

18

Can you also add a similar test-case for shl?

[efriedma]

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

2317

Do these store instructions have the same issue?

[TiehuZhang]

ping

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

2306

Done, thanks for your suggestion

2317

If x(an usee) is used on an unpredicatable instruction, then all its bits are useful. The current implementation does not seem to be exceptional, I think.

[TiehuZhang]

ping

[sdesmalen]

LGTM, thanks!

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

case AArch64::ANDXri:
     return getUsefulBitsFromAndWithImmediate(SDValue(UserNode, 0), UsefulBits,
                                              Depth);
   case AArch64::UBFMWri:
   case AArch64::UBFMXri:
     return getUsefulBitsFromUBFM(SDValue(UserNode, 0), UsefulBits, Depth);
 
   case AArch64::ORRWrs:
   case AArch64::ORRXrs:
-    if (UserNode->getOperand(1) != Orig)
-      return;
-    return getUsefulBitsFromOrWithShiftedReg(SDValue(UserNode, 0), UsefulBits,
-                                             Depth);
+    if (UserNode->getOperand(0) != Orig && UserNode->getOperand(1) == Orig)
+      getUsefulBitsFromOrWithShiftedReg(SDValue(UserNode, 0), UsefulBits,
+    return;
   case AArch64::BFMWri:
   case AArch64::BFMXri:
     return getUsefulBitsFromBFM(SDValue(UserNode, 0), Orig, UsefulBits, Depth);
 
   case AArch64::STRBBui:
   case AArch64::STURBBi:
     if (UserNode->getOperand(0) != Orig)
       return;
     UsefulBits &= APInt(UsefulBits.getBitWidth(), 0xff);
     return;
 
   case AArch64::STRHHui:
   case AArch64::STURHHi:
     if (UserNode->getOperand(0) != Orig)
       return;
     UsefulBits &= APInt(UsefulBits.getBitWidth(), 0xffff);

llvm/test/CodeGen/AArch64/arm64-isel-or.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu  -o - | FileCheck %s
+; ModuleID = '<stdin>'
+source_filename = "<stdin>"
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+define i32 @Orlshr(i32 %e) {
+; CHECK-LABEL: Orlshr:
+; CHECK:       // %bb.0: // %entry
+; CHECK-NEXT:    orr w8, w0, w0, lsr #15
+; CHECK-NEXT:    orr w8, w8, w8, lsr #15
+; CHECK-NEXT:    orr w8, w8, w8, lsr #15
+; CHECK-NEXT:    orr w0, w8, w8, lsr #15
+; CHECK-NEXT:    ret
+entry:
+  %shr = lshr i32 %e, 15
+  %or = or i32 %shr, %e
+  %shr.1 = lshr i32 %or, 15
+  %or.1 = or i32 %shr.1, %or
+  %shr.2 = lshr i32 %or.1, 15
+  %or.2 = or i32 %shr.2, %or.1
+  %shr.3 = lshr i32 %or.2, 15
+  %or.3 = or i32 %shr.3, %or.2
+  ret i32 %or.3
+}
+
+define i32 @Orshl(i32 %e) {
+; CHECK-LABEL: Orshl:
+; CHECK:       // %bb.0: // %entry
+; CHECK-NEXT:    orr w8, w0, w0, lsl #15
+; CHECK-NEXT:    orr w8, w8, w8, lsl #15
+; CHECK-NEXT:    orr w8, w8, w8, lsl #15
+; CHECK-NEXT:    orr w0, w8, w8, lsl #15
+; CHECK-NEXT:    ret
+entry:
+  %shl = shl i32 %e, 15
+  %or = or i32 %shl, %e
+  %shl.1 = shl i32 %or, 15
+  %or.1 = or i32 %shl.1, %or
+  %shl.2 = shl i32 %or.1, 15
+  %or.2 = or i32 %shl.2, %or.1
+  %shl.3 = shl i32 %or.2, 15
+  %or.3 = or i32 %shl.3, %or.2
+  ret i32 %or.3
+}