[PATCH] D136244: [AArch64] Optimize memcmp when the result is tested for [in]equality with 0

[Allen zhong via Phabricator via llvm-commits]

Allen marked an inline comment as done.
Allen added a comment.

need rebase as fail in case llvm/test/CodeGen/AArch64/bcmp.ll, which is new precommited in e95c74b423c <https://reviews.llvm.org/rGe95c74b423c4c78102121e0d7cd8479450a285df>



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:19499-19500
+      (LHS.getOperand(0)->getOpcode() == ISD::XOR &&
+       LHS.getOperand(1)->getOpcode() == ISD::XOR) &&
+      LHS.getOperand(0)->hasOneUse() && LHS.getOperand(1)->hasOneUse()) {
+    SDValue XOR0 = LHS.getOperand(0);
----------------
bcl5980 wrote:
> LHS should be OneUse also?
The **LHS **node itself is not used in the return value when the pattern matched, so I don't think the OneUse is  needed, correct me if I'm wrong, thanks.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:19506
+    SDValue Cmp =
+        DAG.getNode(AArch64ISD::SUBS, DL, DAG.getVTList(TstVT, MVT::Glue),
+                    XOR0.getOperand(0), XOR0.getOperand(1));
----------------
bcl5980 wrote:
> You haven't pass the glue to other instructions so the glue is useless. And I think we needn't use Glue here.
Thanks, I'll updated it.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:19510
+    SDValue NZCVOp = DAG.getConstant(0, DL, MVT::i32);
+    SDValue CCmp = DAG.getNode(AArch64ISD::CCMP, DL, MVT_CC, XOR1.getOperand(0),
+                               XOR1.getOperand(1), NZCVOp, CCVal, Overflow);
----------------
bcl5980 wrote:
> I am not sure if we can just combine to ISD::SETCC ? Maybe it can combine with some other op.
sorry, I don't understand what is the **ISD::SETCC**, could you please show more detailedly? as I don't find it in my changes.


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

https://reviews.llvm.org/D136244