Summary: Add support for XAR instruction generation to the AArch64 backend

[dmgreen]

Thanks for working on this, it looks good.

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

438

Add 'try' to the start, like the other methods here?

4281

These can be SDValue

4284

It is clearer to use N0->getOpcode() != AArch64ISD::VSHL ||

4287

Check that N0.getOperand(0) == N1.getOperand(0), and that N0.getOperand(0).getOpcode() == ISD::XOR, to make sure they are the same XOR node.

4295

There is a getConstantOperandVal, which is useful when we know the value is a constant and we want to get the value.

4302

MVT::i64 should be VT I think, from N->getValueType(0)
You may need to use getTargetConstant for the immediate too.

llvm/test/CodeGen/AArch64/xar.ll

11

I believe the #54 should be part of the instruction

[SjoerdMeijer]

I was just scrolling through GitHub issues and stumbled on this. Looks like a nice little addition, any plans on picking this up?

[BK1603]

Summary: Add support for XAR instruction generation to the AArch64 backend

[BK1603]

llvm/test/CodeGen/AArch64/xar.ll

11

I don't think that 54 would be a part of the instruction, since the instruction itself (as defined in arm's reference) is XAR. I think the issue is with the naming here, I've renamed xar_54 to xar.

[BK1603]

Summary: Add support for XAR instruction generation to the AArch64 backend

[dmgreen]

Thanks for the update, this is looking OK to me. But I'm not 100% sure that we will receive notifications for phabricator any more as most work has moved to github pull requests.

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

4284

This doesn't need quite as many brackets.

4287

I think I would use

if (N0->getOperand(0)->getOpcode() != N1->getOperand(0)->getOpcode() ||
    N1->getOperand(0)->getOpcode() != ISD::XOR)

But does something need to check that N0->getOperand(0) == N1->getOperand(0)?

4299

This looks like it needs a clang-format for the patch.
imm -> Imm

4301

if (ShAmt + HsAmt != 64)

4375–4377

Maybe if (Subtarget->hasSHA3() && trySelectXAR(Node))

[BK1603]

Summary: Add support for XAR instruction generation to the AArch64 backend

[BK1603]

Thanks for the review @dmgreen

I ran clang-format on the patch and have resolved other comments.
As for notifications for this patch, I'll ping you on the github issue as well.
I'll submit github PRs for any new patches that I make from here on out.

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

4287

I feel what you suggest should be enough, we would only check for the second condition (N1->getOperand(0)->getOpcode() != ISD::XOR) if the first one is false,
which would imply that N0->getOperand(0)->getOpcode() == N1->getOperand(0)->getOpcode()

[dmgreen]

In D150076#4657942, @BK1603 wrote:

I ran clang-format on the patch and have resolved other comments.
As for notifications for this patch, I'll ping you on the github issue as well.
I'll submit github PRs for any new patches that I make from here on out.

Thanks. Can you create a github pr for this patch so I can use the submit button in the github UI?

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

4287

I mean something like this:

define <2 x i64> @xar(<2 x i64> %x, <2 x i64> %y, <2 x i64> %z) {                                 
    %a1 = xor <2 x i64> %x, %y                                                                    
    %a2 = xor <2 x i64> %x, %z                                                                    
    %b = call <2 x i64> @llvm.fshl.v2i64(<2 x i64> %a1, <2 x i64> %a2, <2 x i64> <i64 10, i64 10>)
    ret <2 x i64> %b                                                                              
}

I think it will mis-compile at the moment.

[BK1603]

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

4287

Oh right got it, misread the previous comment. I’ll incorporate this change into the GitHub PR as well

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

public:
   bool tryShiftAmountMod(SDNode *N);
 
   bool tryReadRegister(SDNode *N);
   bool tryWriteRegister(SDNode *N);
 
   bool trySelectCastFixedLengthToScalableVector(SDNode *N);
   bool trySelectCastScalableToFixedLengthVector(SDNode *N);
 
+  bool trySelectXAR(SDNode *N);
+
 // Include the pieces autogenerated from the target description.
 #include "AArch64GenDAGISel.inc"
 
 private:
   bool SelectShiftedRegister(SDValue N, bool AllowROR, SDValue &Reg,
                              SDValue &Shift);
   bool SelectShiftedRegisterFromAnd(SDValue N, SDValue &Reg, SDValue &Shift);
   bool SelectAddrModeIndexed7S(SDValue N, unsigned Size, SDValue &Base,
bool AArch64DAGToDAGISel::trySelectCastScalableToFixedLengthVector(SDNode *N) {
 
   SDLoc DL(N);
   auto RC = CurDAG->getTargetConstant(AArch64::ZPRRegClassID, DL, MVT::i64);
   ReplaceNode(N, CurDAG->getMachineNode(TargetOpcode::COPY_TO_REGCLASS, DL, VT,
                                         N->getOperand(0), RC));
   return true;
 }
 
+bool AArch64DAGToDAGISel::trySelectXAR(SDNode *N) {
+  assert(N->getOpcode() == ISD::OR && "Expected OR instruction");
+
+  SDValue N0 = N->getOperand(0);
+  SDValue N1 = N->getOperand(1);
+
+  if (N0->getOpcode() != AArch64ISD::VSHL ||
+      N1->getOpcode() != AArch64ISD::VLSHR)
+    return false;
+
+  if (N0->getOperand(0)->getOpcode() != N1->getOperand(0)->getOpcode() ||
+      N1->getOperand(0)->getOpcode() != ISD::XOR)
+    return false;
+
+  SDValue XOR = N0.getOperand(0);
+  SDValue R1 = XOR.getOperand(0);
+  SDValue R2 = XOR.getOperand(1);
+
+  unsigned HsAmt = N0.getConstantOperandVal(1);
+  unsigned ShAmt = N1.getConstantOperandVal(1);
+
+  SDLoc DL = SDLoc(N0.getOperand(1));
+  SDValue Imm = CurDAG->getTargetConstant(
+      ShAmt, DL, N0.getOperand(1).getValueType(), false);
+
+  if (ShAmt + HsAmt != 64)
+    return false;
+
+  SDValue Ops[] = {R1, R2, Imm};
+  CurDAG->SelectNodeTo(N, AArch64::XAR, N0.getValueType(), Ops);
+
+  return true;
+}
+
 void AArch64DAGToDAGISel::Select(SDNode *Node) {
   // If we have a custom node, we already have selected!
   if (Node->isMachineOpcode()) {
     LLVM_DEBUG(errs() << "== "; Node->dump(CurDAG); errs() << "\n");
     Node->setNodeId(-1);
     return;
   }
 
void AArch64DAGToDAGISel::Select(SDNode *Node) {
   case ISD::SIGN_EXTEND:
     if (tryBitfieldExtractOpFromSExt(Node))
       return;
     break;
 
   case ISD::OR:
     if (tryBitfieldInsertOp(Node))
       return;
+    if (Subtarget->hasSHA3() && trySelectXAR(Node))
+      return;
     break;
 
   case ISD::EXTRACT_SUBVECTOR: {
     if (trySelectCastScalableToFixedLengthVector(Node))
       return;
     break;
   }
 
   case ISD::INSERT_SUBVECTOR: {

llvm/test/CodeGen/AArch64/xar.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
+; RUN: llc -mtriple=aarch64-none-eabi -mattr=+sha3 < %s | FileCheck --check-prefix=SHA3 %s
+; RUN: llc -mtriple=aarch64-none-eabi -mattr=-sha3 < %s | FileCheck --check-prefix=NOSHA3 %s
+
+define <2 x i64> @xar(<2 x i64> %x, <2 x i64> %y) {
+; SHA3-LABEL: xar:
+; SHA3:       // %bb.0:
+; SHA3-NEXT:    xar v0.2d, v0.2d, v1.2d, #54
+; SHA3-NEXT:    ret
+;
+; NOSHA3-LABEL: xar:
+; NOSHA3:       // %bb.0:
+; NOSHA3-NEXT:    eor v0.16b, v0.16b, v1.16b
+; NOSHA3-NEXT:    ushr v1.2d, v0.2d, #54
+; NOSHA3-NEXT:    shl v0.2d, v0.2d, #10
+; NOSHA3-NEXT:    orr v0.16b, v0.16b, v1.16b
+; NOSHA3-NEXT:    ret
+    %a = xor <2 x i64> %x, %y
+    %b = call <2 x i64> @llvm.fshl.v2i64(<2 x i64> %a, <2 x i64> %a, <2 x i64> <i64 10, i64 10>)
+    ret <2 x i64> %b
+}
+
+declare <2 x i64> @llvm.fshl.v2i64(<2 x i64>, <2 x i64>, <2 x i64>)