[PATCH] D71701: [AArch64] Peephole optimization. Merge AND and TST instructions

[Pavel Kosov via Phabricator via llvm-commits]

kpdev42 marked 7 inline comments as done.
kpdev42 added a comment.

All commentaries are resolved, patch is ready for review )



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1682
+      const SDValue RHSOp = LHS.getOperand(1);
+      const SDValue LHSOp = LHS.getOperand(0);
+      const SDValue ANDSNode = DAG.getNode(AArch64ISD::ANDS, dl,
----------------
SjoerdMeijer wrote:
> Some real nits: there's probably very little benefit of declaring `RHSOp` and `LHSOp` here. For readability, it's probably better to just feed the `LHS.getOperand()` directly to `getNode()` 
Thank you. I removed these temporary (and a little bit redundant :) ) variables


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1687
+      // Replace all users of (and X, Y) with newly generated (ands X, Y)
+      DAG.ReplaceAllUsesWith(LHS.getNode(), &ANDSNode);
+      return ANDSNode.getValue(1);
----------------
dmgreen wrote:
> Can this just be DAG.ReplaceAllUsesWith(LHS, ANDSNode) ?
> 
> Otherwise it looks like it might try to treat &ANDSNode as an array.
Thank you. I changed call, this one looks much cleaner.


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

https://reviews.llvm.org/D71701