[SystemZ] Add support for .insn directives for vector instructions

[jonpa]

Summary

Support VRI, VRR, VRS, VRV, VRX, VSI instruction formats with the .insn directive.

[uweigand]

llvm/lib/Target/SystemZ/AsmParser/SystemZAsmParser.cpp

643

I'm wondering why you're using BDAddr32 here; while there are shifts using the VRS format, there are also other instruction using the VRS format where the base register is used as 64-bit. While we don't know which instruction this is being using for, I think it would be better (and simpler) to use BDAddr64Disp12 here.

llvm/lib/Target/SystemZ/SystemZInstrFormats.td

1767

I'd prefer to just name this DirectiveInsnVRI (without the "e") like all the others.

llvm/lib/Target/SystemZ/SystemZInstrInfo.td

2246

I think we should not use AnyReg for vector registers here, but rather VR128. Note that these are not interchangable as vector regs need 5 bits while all other regs only need 4, so it matters whether the format has a slot for a VR vs. any other reg.

In fact, I think AnyReg should probably not even accept vector registers; passing a VR to any of the *other* insn formats that do not expect a VR will lose one bit. (You might want to confirm with how this is handled by GAS as well.)

2256

Given the comment in SystemZAsmParser.cpp, this should then be bdaddr12only instead of shift12only.

[jonpa]

Updated per review.

llvm/lib/Target/SystemZ/AsmParser/SystemZAsmParser.cpp

643

Ah, I guess I was looking at the GNU source code (opcodes_s390-opc.c) and used the instructions given as examples there as models for my patch...

Changed to BDAddr64Displ12.

llvm/lib/Target/SystemZ/SystemZInstrInfo.td

2246

Ah, I see. Changed to VR128 for the vector reg operands.

I tried removing the vector regs from AnyReg register class, but that did not produce an error:

.insn rr,0x1800,%r2,%r0  =>   0:   18 20                   lr      %r2,%r0
.insn rr,0x1800,%v2,%r0  =>   0:   18 20                   lr      %r2,%r0

I also checked with GAS, and the behavior was the same (registers just parsed for the numbers, and the preceeding letter (r/v/f...) seems ignored). Since this matches GAS, I did not take pursue this further...(?)

[uweigand]

See the inline question about vector registers. This does not affect the current patch however, so this should be good to go in. LGTM.

llvm/lib/Target/SystemZ/SystemZInstrInfo.td

2246

Can you also check with a register number >= 16? For < 16, I guess there's no real difference between %vN and %fN (which is allowed for AnyReg) ...

llvm/lib/Target/SystemZ/AsmParser/SystemZAsmParser.cpp

 #include "SystemZGenAsmMatcher.inc"
 
 // Used for the .insn directives; contains information needed to parse the
 // operands in the directive.
 struct InsnMatchEntry {
   StringRef Format;
   uint64_t Opcode;
   int32_t NumOperands;
-  MatchClassKind OperandKinds[5];
+  MatchClassKind OperandKinds[7];
 };
 
 // For equal_range comparison.
 struct CompareInsn {
   bool operator() (const InsnMatchEntry &LHS, StringRef RHS) {
     return LHS.Format < RHS;
   }
   bool operator() (StringRef LHS, const InsnMatchEntry &RHS) {
/* Format, Opcode, NumOperands, OperandKinds */
     { MCK_U48Imm, MCK_BDAddr64Disp12, MCK_U16Imm } },
   { "siy", SystemZ::InsnSIY, 3,
     { MCK_U48Imm, MCK_BDAddr64Disp20, MCK_U8Imm } },
   { "ss", SystemZ::InsnSS, 4,
     { MCK_U48Imm, MCK_BDXAddr64Disp12, MCK_BDAddr64Disp12, MCK_AnyReg } },
   { "sse", SystemZ::InsnSSE, 3,
     { MCK_U48Imm, MCK_BDAddr64Disp12, MCK_BDAddr64Disp12 } },
   { "ssf", SystemZ::InsnSSF, 4,
-    { MCK_U48Imm, MCK_BDAddr64Disp12, MCK_BDAddr64Disp12, MCK_AnyReg } }
+    { MCK_U48Imm, MCK_BDAddr64Disp12, MCK_BDAddr64Disp12, MCK_AnyReg } },
+  { "vri", SystemZ::InsnVRIe, 6,
+    { MCK_U48Imm, MCK_AnyReg, MCK_AnyReg, MCK_U12Imm, MCK_U4Imm, MCK_U4Imm } },
+  { "vrr", SystemZ::InsnVRR, 7,
+    { MCK_U48Imm, MCK_AnyReg, MCK_AnyReg, MCK_AnyReg, MCK_U4Imm, MCK_U4Imm,
+      MCK_U4Imm } },
+  { "vrs", SystemZ::InsnVRS, 5,
+    { MCK_U48Imm, MCK_AnyReg, MCK_AnyReg, MCK_BDAddr32Disp12, MCK_U4Imm } },
+  { "vrv", SystemZ::InsnVRV, 4,
+    { MCK_U48Imm, MCK_AnyReg, MCK_BDVAddr64Disp12, MCK_U4Imm } },
+  { "vrx", SystemZ::InsnVRX, 4,
+    { MCK_U48Imm, MCK_AnyReg, MCK_BDXAddr64Disp12, MCK_U4Imm } },
+  { "vsi", SystemZ::InsnVSI, 4,
+    { MCK_U48Imm, MCK_AnyReg, MCK_BDAddr64Disp12, MCK_U8Imm } }
 };
 
 static void printMCExpr(const MCExpr *E, raw_ostream &OS) {
   if (!E)
     return;
   if (auto *CE = dyn_cast<MCConstantExpr>(E))
     OS << *CE;
   else if (auto *UE = dyn_cast<MCUnaryExpr>(E))
for (int i = 0; i < Entry->NumOperands; i++) {
     // Parse operands.
     OperandMatchResultTy ResTy;
     if (Kind == MCK_AnyReg)
       ResTy = parseAnyReg(Operands);
     else if (Kind == MCK_BDXAddr64Disp12 || Kind == MCK_BDXAddr64Disp20)
       ResTy = parseBDXAddr64(Operands);
     else if (Kind == MCK_BDAddr64Disp12 || Kind == MCK_BDAddr64Disp20)
       ResTy = parseBDAddr64(Operands);
+    else if (Kind == MCK_BDAddr32Disp12)
+      ResTy = parseBDAddr32(Operands);
+    else if (Kind == MCK_BDVAddr64Disp12)
+      ResTy = parseBDVAddr64(Operands);
     else if (Kind == MCK_PCRel32)
       ResTy = parsePCRel32(Operands);
     else if (Kind == MCK_PCRel16)
       ResTy = parsePCRel16(Operands);
     else {
       // Only remaining operand kind is an immediate.
       const MCExpr *Expr;
       SMLoc StartLoc = Parser.getTok().getLoc();
for (size_t i = 0; i < Operands.size(); i++) {
     // Add operands to instruction.
     SystemZOperand &ZOperand = static_cast<SystemZOperand &>(Operand);
     if (ZOperand.isReg())
       ZOperand.addRegOperands(Inst, 1);
     else if (ZOperand.isMem(BDMem))
       ZOperand.addBDAddrOperands(Inst, 2);
     else if (ZOperand.isMem(BDXMem))
       ZOperand.addBDXAddrOperands(Inst, 3);
+    else if (ZOperand.isMem(BDVMem))
+      ZOperand.addBDVAddrOperands(Inst, 3);
     else if (ZOperand.isImm())
       ZOperand.addImmOperands(Inst, 1);
     else
       llvm_unreachable("unexpected operand type");
   }
 
   // Emit as a regular instruction.
   Parser.getStreamer().emitInstruction(Inst, getSTI());

llvm/lib/Target/SystemZ/SystemZInstrFormats.td

 class DirectiveInsnSSF<dag outs, dag ins, string asmstr, list<dag> pattern>
   : InstSSF<0, outs, ins, asmstr, pattern> {
   bits<48> enc;
 
   let Inst{47-40} = enc{47-40};
   let Inst{35-32} = enc{35-32};
 }
 
+class DirectiveInsnVRIe<dag outs, dag ins, string asmstr, list<dag> pattern>
+  : InstVRIe<0, outs, ins, asmstr, pattern> {
+  bits<48> enc;
+
+  let Inst{47-40} = enc{47-40};
+  let Inst{7-0}   = enc{7-0};
+}
+
+class DirectiveInsnVRR<dag outs, dag ins, string asmstr, list<dag> pattern>
+  : InstVRRc<0, outs, ins, asmstr, pattern> {
+  bits<48> enc;
+
+  let Inst{47-40} = enc{47-40};
+  let Inst{7-0}   = enc{7-0};
+}
+
+class DirectiveInsnVRS<dag outs, dag ins, string asmstr, list<dag> pattern>
+  : InstVRSc<0, outs, ins, asmstr, pattern> {
+  bits<48> enc;
+
+  let Inst{47-40} = enc{47-40};
+  let Inst{7-0}   = enc{7-0};
+}
+
+class DirectiveInsnVRV<dag outs, dag ins, string asmstr, list<dag> pattern>
+  : InstVRV<0, outs, ins, asmstr, pattern> {
+  bits<48> enc;
+
+  let Inst{47-40} = enc{47-40};
+  let Inst{7-0}   = enc{7-0};
+}
+
+class DirectiveInsnVRX<dag outs, dag ins, string asmstr, list<dag> pattern>
+  : InstVRX<0, outs, ins, asmstr, pattern> {
+  bits<48> enc;
+
+  let Inst{47-40} = enc{47-40};
+  let Inst{7-0}   = enc{7-0};
+}
+
+class DirectiveInsnVSI<dag outs, dag ins, string asmstr, list<dag> pattern>
+  : InstVSI<0, outs, ins, asmstr, pattern> {
+  bits<48> enc;
+
+  let Inst{47-40} = enc{47-40};
+  let Inst{7-0}   = enc{7-0};
+}
+
+
 //===----------------------------------------------------------------------===//
 // Variants of instructions with condition mask
 //===----------------------------------------------------------------------===//
 //
 // For instructions using a condition mask (e.g. conditional branches,
 // compare-and-branch instructions, or conditional move instructions),
 // we generally need to create multiple instruction patterns:
 //

llvm/lib/Target/SystemZ/SystemZInstrInfo.td

let isCodeGenOnly = 1, hasSideEffects = 1 in {
   def InsnSSE : DirectiveInsnSSE<(outs),
                                  (ins imm64zx48:$enc,
                                       bdaddr12only:$BD1,bdaddr12only:$BD2),
                                  ".insn sse,$enc,$BD1,$BD2", []>;
   def InsnSSF : DirectiveInsnSSF<(outs),
                                  (ins imm64zx48:$enc, bdaddr12only:$BD1,
                                       bdaddr12only:$BD2, AnyReg:$R3),
                                  ".insn ssf,$enc,$BD1,$BD2,$R3", []>;
+  def InsnVRIe : DirectiveInsnVRIe<(outs),
+                                   (ins imm64zx48:$enc, AnyReg:$V1, AnyReg:$V2,
+                                    imm32zx12:$I3, imm32zx4:$M4, imm32zx4:$M5),
+                                   ".insn vri,$enc,$V1,$V2,$I3,$M4,$M5", []>;
+  def InsnVRR : DirectiveInsnVRR<(outs),
+                                 (ins imm64zx48:$enc, AnyReg:$V1, AnyReg:$V2,
+                                  AnyReg:$V3, imm32zx4:$M4, imm32zx4:$M5,
+                                  imm32zx4:$M6),
+                                  ".insn vrr,$enc,$V1,$V2,$V3,$M4,$M5,$M6", []>;
+  def InsnVRS : DirectiveInsnVRS<(outs),
+                                 (ins imm64zx48:$enc, AnyReg:$R1, AnyReg:$V3,
+                                  shift12only:$BD2, imm32zx4:$M4),
+                                 ".insn vrs,$enc,$BD2,$M4", []>;
+  def InsnVRV : DirectiveInsnVRV<(outs),
+                                 (ins imm64zx48:$enc, AnyReg:$V1,
+                                      bdvaddr12only:$VBD2, imm32zx4:$M3),
+                                 ".insn vrv,$enc,$V1,$VBD2,$M3", []>;
+  def InsnVRX : DirectiveInsnVRX<(outs),
+                                 (ins imm64zx48:$enc, AnyReg:$V1,
+                                  bdxaddr12only:$XBD2, imm32zx4:$M3),
+                                 ".insn vrx,$enc,$V1,$XBD2,$M3", []>;
+  def InsnVSI : DirectiveInsnVSI<(outs),
+                                 (ins imm64zx48:$enc, AnyReg:$V1,
+                                  bdaddr12only:$BD2, imm32zx8:$I3),
+                                  ".insn vsi,$enc,$V1,$BD2,$I3", []>;
 }
 
 //===----------------------------------------------------------------------===//
 // Peepholes.
 //===----------------------------------------------------------------------===//
 
 // Avoid generating 2 XOR instructions. (xor (and x, y), y) is
 // equivalent to (and (xor x, -1), y)

llvm/test/MC/SystemZ/directive-insn-vector.s

+# RUN: llvm-mc -triple s390x-linux-gnu -filetype=obj %s | \
+# RUN: llvm-objdump --mcpu=z14 -d - | FileCheck %s
+
+# Test the .insn directive for vector instructions.
+
+#CHECK: e7 23 2f ff 10 13     vgef    %v2, 4095(%v3,%r2), 1
+  .insn vrv,0xe70000000013,%v2,4095(%v3,%r2),1
+
+#CHECK: e7 56 ff f1 20 4a     vftci   %v5, %v6, 4095, 2, 1
+  .insn vri,0xe7000000004a,%v5,%v6,4095,2,1
+
+#CHECK: e7 20 2f ff 30 06     vl      %v2, 4095(%r2), 3
+  .insn vrx,0xe70000000006,%v2,4095(%r2),3
+
+#CHECK: e7 16 00 01 00 21     vlgvb   %r1, %v6, 1
+  .insn vrs,0xe70000003021,%r1,%v6,1(%r0),0
+#CHECK: e7 16 00 00 30 21     vlgvg   %r1, %v6, 0
+  .insn vrs,0xe70000003021,%r1,%v6,0(%r0),3
+
+#CHECK: e7 37 00 00 00 56     vlr     %v3, %v7
+  .insn vrr,0xe70000000056,%v3,%v7,0,0,0,0
+#CHECK: e7 37 60 18 30 eb     wfchdbs %f3, %f7, %f6
+  .insn vrr,0xe700000000eb,%v3,%v7,%v6,3,8,1
+
+#CHECK: e6 0c 20 0c 01 35     vlrl    %v16, 12(%r2), 12
+  .insn vsi,0xe60000000035,%v16,12(%r2),12
+