[GlobalISel] Change representation of shuffle masks in MachineOperand.

[efriedma]

Summary

We're planning to remove the shufflemask operand from ShuffleVectorInst (D72467); fix GlobalISel so it doesn't depend on that Constant.

The change to prelegalizercombiner-shuffle-vector.mir happens because the input contains a literal "-1" in the mask (so the parser/verifier weren't really handling it properly). We now treat it as equivalent to "undef" in all contexts.

[arsenm]

llvm/include/llvm/CodeGen/MachineOperand.h

178

Won't this double the size of every MachineOperand? That's an extremely high cost

[arsenm]

llvm/include/llvm/CodeGen/MachineOperand.h

178

I think it's important to avoid increasing the size of MachineOperand at any cost. Whatever goes here should fit in a single pointer to avoid the increase

[efriedma]

llvm/include/llvm/CodeGen/MachineOperand.h

178

This patch doesn't make MachineOperand larger. ContentsUnion is at least two pointers wide anyway, for MO_Register etc. (There's a static_assert in the MachineOperand constructor that checks the size of both ContentsUnion and MachineOperand.)

llvm/include/llvm/CodeGen/MachineFunction.h

public:
   /// Cap must be the same capacity that was used to allocate the array.
   void deallocateOperandArray(OperandCapacity Cap, MachineOperand *Array) {
     OperandRecycler.deallocate(Cap, Array);
   }
 
   /// Allocate and initialize a register mask with @p NumRegister bits.
   uint32_t *allocateRegMask();
 
+  ArrayRef<int> allocateShuffleMask(ArrayRef<int> Mask);
+
   /// Allocate and construct an extra info structure for a `MachineInstr`.
   ///
   /// This is allocated on the function's allocator and so lives the life of
   /// the function.
   MachineInstr::ExtraInfo *createMIExtraInfo(
       ArrayRef<MachineMemOperand *> MMOs, MCSymbol *PreInstrSymbol = nullptr,
       MCSymbol *PostInstrSymbol = nullptr, MDNode *HeapAllocMarker = nullptr);
 

llvm/include/llvm/CodeGen/MachineInstrBuilder.h

const MachineInstrBuilder &addIntrinsicID(Intrinsic::ID ID) const {
     return *this;
   }
 
   const MachineInstrBuilder &addPredicate(CmpInst::Predicate Pred) const {
     MI->addOperand(*MF, MachineOperand::CreatePredicate(Pred));
     return *this;
   }
 
-  const MachineInstrBuilder &addShuffleMask(const Constant *Val) const {
+  const MachineInstrBuilder &addShuffleMask(ArrayRef<int> Val) const {
     MI->addOperand(*MF, MachineOperand::CreateShuffleMask(Val));
     return *this;
   }
 
   const MachineInstrBuilder &addSym(MCSymbol *Sym,
                                     unsigned char TargetFlags = 0) const {
     MI->addOperand(*MF, MachineOperand::CreateMCSymbol(Sym, TargetFlags));
     return *this;

llvm/include/llvm/CodeGen/MachineOperand.h

union {
     unsigned OffsetLo;        // Matches Contents.OffsetedInfo.OffsetHi.
   } SmallContents;
 
   /// ParentMI - This is the instruction that this operand is embedded into.
   /// This is valid for all operand types, when the operand is in an instr.
   MachineInstr *ParentMI;
 
   /// Contents union - This contains the payload for the various operand types.
-  union {
+  union ContentsUnion {
+    ContentsUnion() {}
     MachineBasicBlock *MBB;  // For MO_MachineBasicBlock.
     const ConstantFP *CFP;   // For MO_FPImmediate.
     const ConstantInt *CI;   // For MO_CImmediate. Integers > 64bit.
     int64_t ImmVal;          // For MO_Immediate.
     const uint32_t *RegMask; // For MO_RegisterMask and MO_RegisterLiveOut.
     const MDNode *MD;        // For MO_Metadata.
     MCSymbol *Sym;           // For MO_MCSymbol.
     unsigned CFIIndex;       // For MO_CFI.
     Intrinsic::ID IntrinsicID; // For MO_IntrinsicID.
     unsigned Pred;           // For MO_Predicate
-    const Constant *ShuffleMask; // For MO_ShuffleMask
+    ArrayRef<int> ShuffleMask; // For MO_ShuffleMask
 
     struct {                  // For MO_Register.
       // Register number is in SmallContents.RegNo.
       MachineOperand *Prev;   // Access list for register. See MRI.
       MachineOperand *Next;
     } Reg;
 
     /// OffsetedInfo - This struct contains the offset and an object identifier.
Intrinsic::ID getIntrinsicID() const {
     return Contents.IntrinsicID;
   }
 
   unsigned getPredicate() const {
     assert(isPredicate() && "Wrong MachineOperand accessor");
     return Contents.Pred;
   }
 
-  const Constant *getShuffleMask() const {
+  ArrayRef<int> getShuffleMask() const {
     assert(isShuffleMask() && "Wrong MachineOperand accessor");
     return Contents.ShuffleMask;
   }
 
   /// Return the offset from the symbol in this operand. This always returns 0
   /// for ExternalSymbol operands.
   int64_t getOffset() const {
     assert((isGlobal() || isSymbol() || isMCSymbol() || isCPI() ||
public:
   }
 
   static MachineOperand CreatePredicate(unsigned Pred) {
     MachineOperand Op(MachineOperand::MO_Predicate);
     Op.Contents.Pred = Pred;
     return Op;
   }
 
-  static MachineOperand CreateShuffleMask(const Constant *C) {
+  static MachineOperand CreateShuffleMask(ArrayRef<int> Mask) {
     MachineOperand Op(MachineOperand::MO_ShuffleMask);
-    Op.Contents.ShuffleMask = C;
+    Op.Contents.ShuffleMask = Mask;
     return Op;
   }
 
   friend class MachineInstr;
   friend class MachineRegisterInfo;
 
 private:
   // If this operand is currently a register operand, and if this is in a

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

bool CombinerHelper::matchCombineShuffleVector(MachineInstr &MI,
   if (DstNumElts % SrcNumElts != 0)
     return false;
 
   // Mask length is a multiple of the source vector length.
   // Check if the shuffle is some kind of concatenation of the input
   // vectors.
   unsigned NumConcat = DstNumElts / SrcNumElts;
   SmallVector<int, 8> ConcatSrcs(NumConcat, -1);
-  SmallVector<int, 8> Mask;
-  ShuffleVectorInst::getShuffleMask(MI.getOperand(3).getShuffleMask(), Mask);
+  ArrayRef<int> Mask = MI.getOperand(3).getShuffleMask();
   for (unsigned i = 0; i != DstNumElts; ++i) {
     int Idx = Mask[i];
     // Undef value.
     if (Idx < 0)
       continue;
     // Ensure the indices in each SrcType sized piece are sequential and that
     // the same source is used for the whole piece.
     if ((Idx % SrcNumElts != (i % SrcNumElts)) ||

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

if (MRI->getType(Idx).getSizeInBits() != PreferredVecIdxWidth) {
     Idx = MIRBuilder.buildSExtOrTrunc(VecIdxTy, Idx)->getOperand(0).getReg();
   }
   MIRBuilder.buildExtractVectorElement(Res, Val, Idx);
   return true;
 }
 
 bool IRTranslator::translateShuffleVector(const User &U,
                                           MachineIRBuilder &MIRBuilder) {
+  SmallVector<int, 8> Mask;
+  ShuffleVectorInst::getShuffleMask(cast<Constant>(U.getOperand(2)), Mask);
+  ArrayRef<int> MaskAlloc = MF->allocateShuffleMask(Mask);
   MIRBuilder.buildInstr(TargetOpcode::G_SHUFFLE_VECTOR)
       .addDef(getOrCreateVReg(U))
       .addUse(getOrCreateVReg(*U.getOperand(0)))
       .addUse(getOrCreateVReg(*U.getOperand(1)))
-      .addShuffleMask(cast<Constant>(U.getOperand(2)));
+      .addShuffleMask(MaskAlloc);
   return true;
 }
 
 bool IRTranslator::translatePHI(const User &U, MachineIRBuilder &MIRBuilder) {
   const PHINode &PI = cast<PHINode>(U);
 
   SmallVector<MachineInstr *, 4> Insts;
   for (auto Reg : getOrCreateVRegs(PI)) {

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

 LegalizerHelper::lowerShuffleVector(MachineInstr &MI) {
   Register DstReg = MI.getOperand(0).getReg();
   Register Src0Reg = MI.getOperand(1).getReg();
   Register Src1Reg = MI.getOperand(2).getReg();
   LLT Src0Ty = MRI.getType(Src0Reg);
   LLT DstTy = MRI.getType(DstReg);
   LLT IdxTy = LLT::scalar(32);
 
-  const Constant *ShufMask = MI.getOperand(3).getShuffleMask();
-
-  SmallVector<int, 32> Mask;
-  ShuffleVectorInst::getShuffleMask(ShufMask, Mask);
+  ArrayRef<int> Mask = MI.getOperand(3).getShuffleMask();
 
   if (DstTy.isScalar()) {
     if (Src0Ty.isVector())
       return UnableToLegalize;
 
     // This is just a SELECT.
     assert(Mask.size() == 1 && "Expected a single mask element");
     Register Val;

llvm/lib/CodeGen/MIRParser/MIParser.cpp

 
 bool MIParser::parseShuffleMaskOperand(MachineOperand &Dest) {
   assert(Token.is(MIToken::kw_shufflemask));
 
   lex();
   if (expectAndConsume(MIToken::lparen))
     return error("expected syntax shufflemask(<integer or undef>, ...)");
 
-  SmallVector<Constant *, 32> ShufMask;
-  LLVMContext &Ctx = MF.getFunction().getContext();
-  Type *I32Ty = Type::getInt32Ty(Ctx);
-
-  bool AllZero = true;
-  bool AllUndef = true;
-
+  SmallVector<int, 32> ShufMask;
   do {
     if (Token.is(MIToken::kw_undef)) {
-      ShufMask.push_back(UndefValue::get(I32Ty));
-      AllZero = false;
+      ShufMask.push_back(-1);
     } else if (Token.is(MIToken::IntegerLiteral)) {
-      AllUndef = false;
       const APSInt &Int = Token.integerValue();
-      if (!Int.isNullValue())
-        AllZero = false;
-      ShufMask.push_back(ConstantInt::get(I32Ty, Int.getExtValue()));
+      ShufMask.push_back(Int.getExtValue());
     } else
       return error("expected integer constant");
 
     lex();
   } while (consumeIfPresent(MIToken::comma));
 
   if (expectAndConsume(MIToken::rparen))
     return error("shufflemask should be terminated by ')'.");
 
-  if (AllZero || AllUndef) {
-    VectorType *VT = VectorType::get(I32Ty, ShufMask.size());
-    Constant *C = AllZero ? Constant::getNullValue(VT) : UndefValue::get(VT);
-    Dest = MachineOperand::CreateShuffleMask(C);
-  } else
-    Dest = MachineOperand::CreateShuffleMask(ConstantVector::get(ShufMask));
-
+  ArrayRef<int> MaskAlloc = MF.allocateShuffleMask(ShufMask);
+  Dest = MachineOperand::CreateShuffleMask(MaskAlloc);
   return false;
 }
 
 bool MIParser::parseTargetIndexOperand(MachineOperand &Dest) {
   assert(Token.is(MIToken::kw_target_index));
   lex();
   if (expectAndConsume(MIToken::lparen))
     return true;

llvm/lib/CodeGen/MachineFunction.cpp

 uint32_t *MachineFunction::allocateRegMask() {
   unsigned NumRegs = getSubtarget().getRegisterInfo()->getNumRegs();
   unsigned Size = MachineOperand::getRegMaskSize(NumRegs);
   uint32_t *Mask = Allocator.Allocate<uint32_t>(Size);
   memset(Mask, 0, Size * sizeof(Mask[0]));
   return Mask;
 }
 
+ArrayRef<int> MachineFunction::allocateShuffleMask(ArrayRef<int> Mask) {
+  int* AllocMask = Allocator.Allocate<int>(Mask.size());
+  copy(Mask, AllocMask);
+  return {AllocMask, Mask.size()};
+}
+
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 LLVM_DUMP_METHOD void MachineFunction::dump() const {
   print(dbgs());
 }
 #endif
 
 StringRef MachineFunction::getName() const {
   return getFunction().getName();

llvm/lib/CodeGen/MachineOperand.cpp

void MachineOperand::print(raw_ostream &OS, ModuleSlotTracker &MST,
   case MachineOperand::MO_Predicate: {
     auto Pred = static_cast<CmpInst::Predicate>(getPredicate());
     OS << (CmpInst::isIntPredicate(Pred) ? "int" : "float") << "pred("
        << CmpInst::getPredicateName(Pred) << ')';
     break;
   }
   case MachineOperand::MO_ShuffleMask:
     OS << "shufflemask(";
-    const Constant* C = getShuffleMask();
-    const int NumElts = C->getType()->getVectorNumElements();
-
+    ArrayRef<int> Mask = getShuffleMask();
     StringRef Separator;
-    for (int I = 0; I != NumElts; ++I) {
-      OS << Separator;
-      C->getAggregateElement(I)->printAsOperand(OS, false, MST);
+    for (int Elt : Mask) {
+      if (Elt == -1)
+        OS << Separator << "undef";
+      else
+        OS << Separator << Elt;
       Separator = ", ";
     }
 
     OS << ')';
     break;
   }
 }
 

llvm/lib/CodeGen/MachineVerifier.cpp

void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
   }
   case TargetOpcode::G_SHUFFLE_VECTOR: {
     const MachineOperand &MaskOp = MI->getOperand(3);
     if (!MaskOp.isShuffleMask()) {
       report("Incorrect mask operand type for G_SHUFFLE_VECTOR", MI);
       break;
     }
 
-    const Constant *Mask = MaskOp.getShuffleMask();
-    auto *MaskVT = dyn_cast<VectorType>(Mask->getType());
-    if (!MaskVT || !MaskVT->getElementType()->isIntegerTy(32)) {
-      report("Invalid shufflemask constant type", MI);
-      break;
-    }
-
-    if (!Mask->getAggregateElement(0u)) {
-      report("Invalid shufflemask constant type", MI);
-      break;
-    }
-
     LLT DstTy = MRI->getType(MI->getOperand(0).getReg());
     LLT Src0Ty = MRI->getType(MI->getOperand(1).getReg());
     LLT Src1Ty = MRI->getType(MI->getOperand(2).getReg());
 
     if (Src0Ty != Src1Ty)
       report("Source operands must be the same type", MI);
 
     if (Src0Ty.getScalarType() != DstTy.getScalarType())
       report("G_SHUFFLE_VECTOR cannot change element type", MI);
 
     // Don't check that all operands are vector because scalars are used in
     // place of 1 element vectors.
     int SrcNumElts = Src0Ty.isVector() ? Src0Ty.getNumElements() : 1;
     int DstNumElts = DstTy.isVector() ? DstTy.getNumElements() : 1;
 
-    SmallVector<int, 32> MaskIdxes;
-    ShuffleVectorInst::getShuffleMask(Mask, MaskIdxes);
+    ArrayRef<int> MaskIdxes = MaskOp.getShuffleMask();
 
     if (static_cast<int>(MaskIdxes.size()) != DstNumElts)
       report("Wrong result type for shufflemask", MI);
 
     for (int Idx : MaskIdxes) {
       if (Idx < 0)
         continue;
 

llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp

bool AArch64InstructionSelector::tryOptVectorDup(MachineInstr &I) const {
   Register ScalarReg = InsMI->getOperand(2).getReg();
   const RegisterBank *ScalarRB = RBI.getRegBank(ScalarReg, MRI, TRI);
   // Match the index constant 0.
   int64_t Index = 0;
   if (!mi_match(InsMI->getOperand(3).getReg(), MRI, m_ICst(Index)) || Index)
     return false;
 
   // The shuffle's second operand doesn't matter if the mask is all zero.
-  const Constant *Mask = I.getOperand(3).getShuffleMask();
-  if (!isa<ConstantAggregateZero>(Mask))
+  ArrayRef<int> Mask = I.getOperand(3).getShuffleMask();
+  if (!all_of(Mask, [](int Elem) { return Elem == 0; }))
     return false;
 
   // We're done, now find out what kind of splat we need.
   LLT VecTy = MRI.getType(I.getOperand(0).getReg());
   LLT EltTy = VecTy.getElementType();
   if (EltTy.getSizeInBits() < 32) {
     LLVM_DEBUG(dbgs() << "Could not optimize splat pattern < 32b elts yet");
     return false;
bool AArch64InstructionSelector::selectShuffleVector(
     MachineInstr &I, MachineRegisterInfo &MRI) const {
   if (tryOptVectorShuffle(I))
     return true;
   const LLT DstTy = MRI.getType(I.getOperand(0).getReg());
   Register Src1Reg = I.getOperand(1).getReg();
   const LLT Src1Ty = MRI.getType(Src1Reg);
   Register Src2Reg = I.getOperand(2).getReg();
   const LLT Src2Ty = MRI.getType(Src2Reg);
-  const Constant *ShuffleMask = I.getOperand(3).getShuffleMask();
+  ArrayRef<int> Mask = I.getOperand(3).getShuffleMask();
 
   MachineBasicBlock &MBB = *I.getParent();
   MachineFunction &MF = *MBB.getParent();
   LLVMContext &Ctx = MF.getFunction().getContext();
 
-  SmallVector<int, 8> Mask;
-  ShuffleVectorInst::getShuffleMask(ShuffleMask, Mask);
-
   // G_SHUFFLE_VECTOR is weird in that the source operands can be scalars, if
   // it's originated from a <1 x T> type. Those should have been lowered into
   // G_BUILD_VECTOR earlier.
   if (!Src1Ty.isVector() || !Src2Ty.isVector()) {
     LLVM_DEBUG(dbgs() << "Could not select a \"scalar\" G_SHUFFLE_VECTOR\n");
     return false;
   }
 

llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-shuffle-vector.mir

 body:             |
   bb.1:
     liveins: $d0, $d1
 
     ; CHECK-LABEL: name: shuffle_vector_to_concat_vector_src_flipped_23013undef_neg
     ; CHECK: liveins: $d0, $d1
     ; CHECK: [[COPY:%[0-9]+]]:_(<2 x s32>) = COPY $d0
     ; CHECK: [[COPY1:%[0-9]+]]:_(<2 x s32>) = COPY $d1
-    ; CHECK: [[SHUF:%[0-9]+]]:_(<6 x s32>) = G_SHUFFLE_VECTOR [[COPY]](<2 x s32>), [[COPY1]], shufflemask(2, 3, 0, 1, 3, -1)
+    ; CHECK: [[SHUF:%[0-9]+]]:_(<6 x s32>) = G_SHUFFLE_VECTOR [[COPY]](<2 x s32>), [[COPY1]], shufflemask(2, 3, 0, 1, 3, undef)
     ; CHECK: RET_ReallyLR implicit [[SHUF]](<6 x s32>)
     %0:_(<2 x s32>) = COPY $d0
     %1:_(<2 x s32>) = COPY $d1
     %2:_(<6 x s32>) = G_SHUFFLE_VECTOR %0(<2 x s32>), %1(<2 x s32>), shufflemask(2,3,0,1,3,-1)
     RET_ReallyLR implicit %2
 ...
 
 # Check that we canonicalize shuffle_vector(Src1, Src2, mask(0,1,2,3,4,5,6,7))

 body:             |
   bb.1:
     liveins: $q0, $q1
 
     ; CHECK-LABEL: name: shuffle_vector_to_concat_vector_src_flipped_4567012367undefundef_neg
     ; CHECK: liveins: $q0, $q1
     ; CHECK: [[COPY:%[0-9]+]]:_(<4 x s32>) = COPY $q0
     ; CHECK: [[COPY1:%[0-9]+]]:_(<4 x s32>) = COPY $q1
-    ; CHECK: [[SHUF:%[0-9]+]]:_(<12 x s32>) = G_SHUFFLE_VECTOR [[COPY]](<4 x s32>), [[COPY1]], shufflemask(4, 5, 6, 7, 0, 1, 2, 3, 6, 7, -1, -1)
+    ; CHECK: [[SHUF:%[0-9]+]]:_(<12 x s32>) = G_SHUFFLE_VECTOR [[COPY]](<4 x s32>), [[COPY1]], shufflemask(4, 5, 6, 7, 0, 1, 2, 3, 6, 7, undef, undef)
     ; CHECK: RET_ReallyLR implicit [[SHUF]](<12 x s32>)
     %0:_(<4 x s32>) = COPY $q0
     %1:_(<4 x s32>) = COPY $q1
     %2:_(<12 x s32>) = G_SHUFFLE_VECTOR %0(<4 x s32>), %1(<4 x s32>), shufflemask(4,5,6,7,0,1,2,3,6,7,-1,-1)
     RET_ReallyLR implicit %2
 ...
 # Check that we canonicalize shuffle_vector(Src1, Src2, mask(4,5,undef,undef,0,1,2,3,undef,undef,6,7))
 # into concat_vector(Src2, Src1, Src2) with vector of pointers.