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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:2308
       return;
+    if (UserNode->getOperand(0) == Orig)
+      return;
----------------
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.


================
Comment at: llvm/test/CodeGen/AArch64/arm64-isel-or.ll:1
+; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -stop-after=instruction-select -o - | FileCheck %s
+; ModuleID = '<stdin>'
----------------
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.


================
Comment at: llvm/test/CodeGen/AArch64/arm64-isel-or.ll:12
+; Function Attrs: norecurse
+define dso_local i32 @main() local_unnamed_addr {
+entry:
----------------
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.


================
Comment at: llvm/test/CodeGen/AArch64/arm64-isel-or.ll:15
+  %0 = load i32, i32* @e, align 4
+  %shr = lshr i32 %0, 9
+  %or = or i32 %shr, %0
----------------
Can you shorten the test-file by using a larger shift value (and less shift/ors)?


================
Comment at: llvm/test/CodeGen/AArch64/arm64-isel-or.ll:17
+  %or = or i32 %shr, %0
+  %shr.1 = lshr i32 %or, 9
+  %or.1 = or i32 %shr.1, %or
----------------
Can you also add a similar test-case for `shl`?


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

https://reviews.llvm.org/D102759