[ARM][MVE] Add target flag for narrowing insts

[samparker]

Summary

Add a flag, 'RetainsPreviousHalf', for operations that operate on top/bottom halves of their input and only write to half of their destination, leaving the other half to retain its previous value. Extracting this out of D76235 to make testing more manageable.

[SjoerdMeijer]

Looks like a good direction to me.

Question about the name retainsPreviousHalf. Since it is not immediately obvious what the Half refers to, that could e.g. be a registers, how do you feel about e.g. retainsPreviousHalfElement? That looks like the terminology used by the ArmARM, e.g one of the instruction descriptions is:

" ... writing the result to either the top half (T variant) or bottom half (B variant) of the result element. The other half of the destination vector element
retains its previous value."

llvm/lib/Target/ARM/ARMInstrFormats.td

411

I wanted to add that a comment would be good here, but then noticed there is one in ARMBaseInfo.h. Looks like the comment for thumbArithFlagSetting is duplicated, and is present here and in ARMBaseInfo.h

[samparker]

Cheers. Updated the name.

[SjoerdMeijer]

Thanks, LGTM

llvm/lib/Target/ARM/ARMInstrFormats.td

class InstTemplate<AddrMode am, int sz, IndexMode im,
   bit isUnaryDataProc = 0;
   bit canXformTo16Bit = 0;
   // The instruction is a 16-bit flag setting Thumb instruction. Used
   // by the parser to determine whether to require the 'S' suffix on the
   // mnemonic (when not in an IT block) or preclude it (when in an IT block).
   bit thumbArithFlagSetting = 0;
 
   bit validForTailPredication = 0;
+  bit retainsPreviousHalfElement = 0;
 
   // If this is a pseudo instruction, mark it isCodeGenOnly.
   let isCodeGenOnly = !eq(!cast<string>(f), "Pseudo");
 
   // The layout of TSFlags should be kept in sync with ARMBaseInfo.h.
   let TSFlags{4-0}   = AM.Value;
   let TSFlags{6-5}   = IndexModeBits;
   let TSFlags{12-7} = Form;
   let TSFlags{13}    = isUnaryDataProc;
   let TSFlags{14}    = canXformTo16Bit;
   let TSFlags{18-15} = D.Value;
   let TSFlags{19}    = thumbArithFlagSetting;
   let TSFlags{20}    = validForTailPredication;
+  let TSFlags{21}    = retainsPreviousHalfElement;
 
   let Constraints = cstr;
   let Itinerary = itin;
 }
 
 class Encoding {
   field bits<32> Inst;
   // Mask of bits that cause an encoding to be UNPREDICTABLE.

llvm/lib/Target/ARM/ARMInstrMVE.td

class MVE_VxSHRN<string iname, string suffix, bit bit_12, bit bit_28,
   let Inst{25-23} = 0b101;
   let Inst{21} = 0b0;
   let Inst{20-16} = imm{4-0};
   let Inst{12} = bit_12;
   let Inst{11-6} = 0b111111;
   let Inst{4} = 0b0;
   let Inst{0} = 0b1;
   let validForTailPredication = 1;
+  let retainsPreviousHalfElement = 1;
 }
 
 def MVE_VRSHRNi16bh : MVE_VxSHRN<"vrshrnb", "i16", 0b0, 0b1, shr_imm8> {
   let Inst{20-19} = 0b01;
 }
 def MVE_VRSHRNi16th : MVE_VxSHRN<"vrshrnt", "i16", 0b1, 0b1, shr_imm8> {
   let Inst{20-19} = 0b01;
 }
class MVE_VxQRSHRUN<string iname, string suffix, bit bit_28, bit bit_12,
   let Inst{25-23} = 0b101;
   let Inst{21} = 0b0;
   let Inst{20-16} = imm{4-0};
   let Inst{12} = bit_12;
   let Inst{11-6} = 0b111111;
   let Inst{4} = 0b0;
   let Inst{0} = 0b0;
   let validForTailPredication = 1;
+  let retainsPreviousHalfElement = 1;
 }
 
 def MVE_VQRSHRUNs16bh : MVE_VxQRSHRUN<
     "vqrshrunb", "s16", 0b1, 0b0, shr_imm8> {
   let Inst{20-19} = 0b01;
 }
 def MVE_VQRSHRUNs16th : MVE_VxQRSHRUN<
     "vqrshrunt", "s16", 0b1, 0b1, shr_imm8> {
class MVE_VxQRSHRN<string iname, string suffix, bit bit_0, bit bit_12,
   let Inst{25-23} = 0b101;
   let Inst{21} = 0b0;
   let Inst{20-16} = imm{4-0};
   let Inst{12} = bit_12;
   let Inst{11-6} = 0b111101;
   let Inst{4} = 0b0;
   let Inst{0} = bit_0;
   let validForTailPredication = 1;
+  let retainsPreviousHalfElement = 1;
 }
 
 multiclass MVE_VxQRSHRN_types<string iname, bit bit_0, bit bit_12> {
   def s16 : MVE_VxQRSHRN<iname, "s16", bit_0, bit_12, shr_imm8> {
     let Inst{28} = 0b0;
     let Inst{20-19} = 0b01;
   }
   def u16 : MVE_VxQRSHRN<iname, "u16", bit_0, bit_12, shr_imm8> {
class MVE_VxMOVxN<string iname, string suffix, bit bit_28, bit bit_17,
   let Inst{19-18} = size;
   let Inst{17} = bit_17;
   let Inst{16} = 0b1;
   let Inst{12} = T;
   let Inst{8} = 0b0;
   let Inst{7} = !if(!eq(bit_17, 0), 1, 0);
   let Inst{0} = 0b1;
   let validForTailPredication = 1;
+  let retainsPreviousHalfElement = 1;
 }
 
 multiclass MVE_VxMOVxN_halves<string iname, string suffix,
                               bit bit_28, bit bit_17, bits<2> size> {
   def bh : MVE_VxMOVxN<iname # "b", suffix, bit_28, bit_17, size, 0b0>;
   def th : MVE_VxMOVxN<iname # "t", suffix, bit_28, bit_17, size, 0b1>;
 }
 
: MVE_qDest_qSrc<iname, suffix, (outs MQPR:$Qd),
                    vpred, cstr, []> {
   let Inst{28} = op;
   let Inst{21-16} = 0b111111;
   let Inst{12} = T;
   let Inst{8-7} = 0b00;
   let Inst{0} = 0b1;
 
   let Predicates = [HasMVEFloat];
+  let retainsPreviousHalfElement = 1;
 }
 
 multiclass MVE_VCVT_f2h_m<string iname, int half> {
   def "": MVE_VCVT_ff<iname, "f16.f32", 0b0, half,
                       (ins MQPR:$Qd_src), vpred_n, "$Qd = $Qd_src">;
   defvar Inst = !cast<Instruction>(NAME);
 
   let Predicates = [HasMVEFloat] in {

llvm/lib/Target/ARM/MCTargetDesc/ARMBaseInfo.h

enum {
     // instruction. Used by the parser to determine whether to require the 'S'
     // suffix on the mnemonic (when not in an IT block) or preclude it (when
     // in an IT block).
     ThumbArithFlagSetting = 1 << 19,
 
     // Whether an instruction can be included in an MVE tail-predicated loop.
     ValidForTailPredication = 1 << 20,
 
+    // Whether an instruction writes to the top/bottom half of a vector element
+    // and leaves the other half untouched.
+    RetainsPreviousHalfElement = 1 << 21,
+
     //===------------------------------------------------------------------===//
     // Code domain.
     DomainShift   = 15,
     DomainMask    = 15 << DomainShift,
     DomainGeneral = 0 << DomainShift,
     DomainVFP     = 1 << DomainShift,
     DomainNEON    = 2 << DomainShift,
     DomainNEONA8  = 4 << DomainShift,

llvm/unittests/Target/ARM/MachineInstrTest.cpp

 #include "ARMBaseInstrInfo.h"
 #include "ARMSubtarget.h"
 #include "ARMTargetMachine.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
 
 #include "gtest/gtest.h"
 
 using namespace llvm;
 
+TEST(MachineInstructionRetainsPreviousHalfElement, IsCorrect) {
+  using namespace ARM;
+
+  auto RetainsPreviousHalfElement = [](unsigned Opcode) {
+    switch (Opcode) {
+    default:
+      break;
+    case MVE_VMOVNi16bh:
+    case MVE_VMOVNi16th:
+    case MVE_VMOVNi32bh:
+    case MVE_VMOVNi32th:
+    case MVE_VQMOVNs16bh:
+    case MVE_VQMOVNs16th:
+    case MVE_VQMOVNs32bh:
+    case MVE_VQMOVNs32th:
+    case MVE_VQMOVNu16bh:
+    case MVE_VQMOVNu16th:
+    case MVE_VQMOVNu32bh:
+    case MVE_VQMOVNu32th:
+    case MVE_VQMOVUNs16bh:
+    case MVE_VQMOVUNs16th:
+    case MVE_VQMOVUNs32bh:
+    case MVE_VQMOVUNs32th:
+    case MVE_VQRSHRNbhs16:
+    case MVE_VQRSHRNbhs32:
+    case MVE_VQRSHRNbhu16:
+    case MVE_VQRSHRNbhu32:
+    case MVE_VQRSHRNths16:
+    case MVE_VQRSHRNths32:
+    case MVE_VQRSHRNthu16:
+    case MVE_VQRSHRNthu32:
+    case MVE_VQRSHRUNs16bh:
+    case MVE_VQRSHRUNs16th:
+    case MVE_VQRSHRUNs32bh:
+    case MVE_VQRSHRUNs32th:
+    case MVE_VQSHRNbhs16:
+    case MVE_VQSHRNbhs32:
+    case MVE_VQSHRNbhu16:
+    case MVE_VQSHRNbhu32:
+    case MVE_VQSHRNths16:
+    case MVE_VQSHRNths32:
+    case MVE_VQSHRNthu16:
+    case MVE_VQSHRNthu32:
+    case MVE_VQSHRUNs16bh:
+    case MVE_VQSHRUNs16th:
+    case MVE_VQSHRUNs32bh:
+    case MVE_VQSHRUNs32th:
+    case MVE_VRSHRNi16bh:
+    case MVE_VRSHRNi16th:
+    case MVE_VRSHRNi32bh:
+    case MVE_VRSHRNi32th:
+    case MVE_VSHRNi16bh:
+    case MVE_VSHRNi16th:
+    case MVE_VSHRNi32bh:
+    case MVE_VSHRNi32th:
+    case MVE_VCVTf16f32bh:
+    case MVE_VCVTf16f32th:
+    case MVE_VCVTf32f16bh:
+    case MVE_VCVTf32f16th:
+      return true;
+    }
+    return false;
+  };
+
+  LLVMInitializeARMTargetInfo();
+  LLVMInitializeARMTarget();
+  LLVMInitializeARMTargetMC();
+
+  auto TT(Triple::normalize("thumbv8.1m.main-arm-none-eabi"));
+  std::string Error;
+  const Target *T = TargetRegistry::lookupTarget(TT, Error);
+  if (!T) {
+    dbgs() << Error;
+    return;
+  }
+
+  TargetOptions Options;
+  auto TM = std::unique_ptr<LLVMTargetMachine>(
+    static_cast<LLVMTargetMachine*>(
+      T->createTargetMachine(TT, "generic", "", Options, None, None,
+                             CodeGenOpt::Default)));
+  ARMSubtarget ST(TM->getTargetTriple(), std::string(TM->getTargetCPU()),
+                  std::string(TM->getTargetFeatureString()),
+                  *static_cast<const ARMBaseTargetMachine *>(TM.get()), false);
+  const ARMBaseInstrInfo *TII = ST.getInstrInfo();
+  auto MII = TM->getMCInstrInfo();
+
+  for (unsigned i = 0; i < ARM::INSTRUCTION_LIST_END; ++i) {
+    const MCInstrDesc &Desc = TII->get(i);
+
+    uint64_t Flags = Desc.TSFlags;
+    if ((Flags & ARMII::DomainMask) != ARMII::DomainMVE)
+      continue;
+
+    bool Valid = (Flags & ARMII::RetainsPreviousHalfElement) != 0;
+    ASSERT_EQ(RetainsPreviousHalfElement(i), Valid)
+              << MII->getName(i)
+              << ": mismatched expectation for tail-predicated safety\n";
+  }
+}
 // Test for instructions that aren't immediately obviously valid within a
 // tail-predicated loop. This should be marked up in their tablegen
 // descriptions. Currently we, conservatively, disallow:
 // - cross beat carries.
-// - narrowing of results.
 // - complex operations.
 // - horizontal operations.
 // - byte swapping.
 // - interleaved memory instructions.
 // TODO: Add to this list once we can handle them safely.
 TEST(MachineInstrValidTailPredication, IsCorrect) {
 
   using namespace ARM;