[SystemZ] Load all vector and FP constants in Select()

[jonpa]

Summary

(Continued from and replacing https://reviews.llvm.org/D58142 and https://reviews.llvm.org/D57926)

Hmm. Actually, I'm now wondering why we need to reject anything in the first place. Can't we improve isFPImmLegal to accept *anything* that can be constructed via any of the vector instructions (VGBM, VGM, VREPI)?

OK - did a new attempt with this patch to follow this broader principle.

So in end, we just need two routines:

• can a FP immediate or BUILD_VECTOR be loaded?

isVectorConstantLegal()

• actually load a FP immediate or BUILD_VECTOR

loadVectorConstant()

Since the current code for for VREP / VGM is based on finding the smallest splat, I wanted to reuse BuildVectorSDNode::isConstantSplat(). For an APFloat, it was not simple to build a temporary BVN (DAG pointer not available), but the finding of the splat without any undefined bits was not that much work to implement.

To try VGBM, the int bits are used, and they are either found with isConstantSplat() called with 128 as minimum splat size, or for APFloat, with a conversion to APInt.

Added a new struct type to wrap this called SystemZVectorConstantInfo.

This should handle any and all constants, and all previously removed tests have now be restored.

as a general principle if we have an instruction that can do something, we should be using it, if it's possible without a lot of overhead ...

There actually seems to be a few less VGMGs now on SPEC after all, and it seems that this is because a scalar FP constant can now reuse a vector splat constant of the same value. This is what the new test vec-const-19.ll checks.

Opcode differences on SPEC over all files:
master <> patch

vgmg           :                 3982                 3945      -37
mdbr           :                 6957                 6949       -8
vgbm           :                 3885                 3893       +8 ?
wfmdb          :                19561                19569       +8

• Could we actually handle FP128 as well with a present FeatureVectorEnhancements1?

• Still a little unsure about the way to do the actual selection in loadVectorConstant(). It seems that the new node must be in the DAG before it is selected, so ReplaceNode() needs to be called first, and then SelectCode(). The bitcast handling varies depending on if VecVT and VT are the same. The fact that VGBM is a machine node, but ROTATE_MASK and REPLICATE are not also makes for some more handling. I guess one could consider selecting them all as machine nodes directly, and one could do an explicit comparison of VT and VecVT perhaps

[uweigand]

Could we actually handle FP128 as well with a present FeatureVectorEnhancements1?

Sure, why not ...

Still a little unsure about the way to do the actual selection in loadVectorConstant(). It seems that the new node must be in the DAG before it is selected, so ReplaceNode() needs to be called first, and then SelectCode(). The bitcast handling varies depending on if VecVT and VT are the same. The fact that VGBM is a machine node, but ROTATE_MASK and REPLICATE are not also makes for some more handling. I guess one could consider selecting them all as machine nodes directly, and one could do an explicit comparison of VT and VecVT perhaps

Maybe it would be simplest to add back SystemZISD::BYTE_MASK and then just handle everything as ISD nodes? I agree you should only create a BITCAST node if the types are actually different.

Another question: it seems odd to have to call getTargetLowering in ::Select, that seems a bit of a layering violation. Why does ​isVectorConstantLegal have to be a member function of SystemZTargetLowering in the first place? Can't this be just a stand-alone function?

[jonpa]

Added handling for fp128 on z14, with some new tests that checks that this is working in fp-const-11.ll. NFC on spec on z14.

Maybe it would be simplest to add back SystemZISD::BYTE_MASK and then just handle everything as ISD nodes? I agree you should only create a BITCAST node if the types are actually different.

Another question: it seems odd to have to call getTargetLowering in ::Select, that seems a bit of a layering violation. Why does isVectorConstantLegal have to be a member function of SystemZTargetLowering in the first place? Can't this be just a stand-alone function?

I followed those suggestions and it indeed looks better now, thanks.

[uweigand]

This looks generally good to me. Some additional options to possibly make the code simpler occurred to me:

• This looks a bit repetitve to me:

if (VCI.Opcode == SystemZISD::BYTE_MASK)
  Op = CurDAG->getNode(SystemZISD::BYTE_MASK, DL, VCI.VecVT,
                       CurDAG->getConstant(VCI.Mask, DL, MVT::i32));
else if (VCI.Opcode == SystemZISD::REPLICATE)
  Op = CurDAG->getNode(SystemZISD::REPLICATE, DL, VCI.VecVT,

Would it make sense to instead store in VCI just the Opcode and an Operands array? And then just create a DAG node with that Opcode and that list of (i32) operands, without checking specifically which Opcode it is?

• Would it make sense to not just store FBImm or BVN in the VCI, but instead perform the equivalent of getIntBits() and getSplat() in the two constructors and store the result values of those in the VCI instead? This would eliminate those functions (which are completely different for FBImm and BVN anyway) and maybe just make the logic simpler overall.

lib/Target/SystemZ/SystemZISelDAGToDAG.cpp

1151

We're now so late that I don't think we need the isOpaque flag any more.

lib/Target/SystemZ/SystemZISelLowering.h

702

Do we really need to convert to APInt here just to determine the type?

709

This function (and possibly the type?) should probably be at least in the lvm::SystemZ:: namespace, to avoid polluting the common llvm:: namespace. Or else (maybe better?) make it a member function of SystemZVectorConstantInfo?

[jonpa]

Patch updated per review, thanks.

Would it make sense to instead store in VCI just the Opcode and an Operands array? And then just create a DAG node with that Opcode and that list of (i32) operands, without checking specifically which Opcode it is?

Yes, that seems to simplify things. I however am somewhat wary to create DAG nodes during all the queries during legalization, so I instead first make a vector of unsigned and then make the actual DAG operands in loadVectorConstant().

Would it make sense to not just store FBImm or BVN in the VCI, but instead perform the equivalent of getIntBits() and getSplat() in the two constructors and store the result values of those in the VCI instead? This would eliminate those functions (which are completely different for FBImm and BVN anyway) and maybe just make the logic simpler overall.

That seems all good to me, except I am not sure if we would like to avoid computing the splat in the cases were it is not needed (BYTE_MASK case). If that was not a concern, it would probably be better to just have the constructors find the various data members common for both cases and then forget if it was a BVN or ConstantFP.

lib/Target/SystemZ/SystemZISelDAGToDAG.cpp

1151

right

lib/Target/SystemZ/SystemZISelLowering.h

702

Seems that we could do either

(&FPImm.getSemantics() == &APFloat::IEEEquad())

or

APFloat::getSizeInBits(FP.getSemantics()) > 64

709

Seems to work fine to make isVectorConstantLegal() a member function. I am guessing the rest is fine, since SystemZ is part of the name, or?

[uweigand]

In D58270#1408154, @jonpa wrote:

Yes, that seems to simplify things. I however am somewhat wary to create DAG nodes during all the queries during legalization, so I instead first make a vector of unsigned and then make the actual DAG operands in loadVectorConstant().

Yes, that's what I meant. This is looking good to me now ...

That seems all good to me, except I am not sure if we would like to avoid computing the splat in the cases were it is not needed (BYTE_MASK case). If that was not a concern, it would probably be better to just have the constructors find the various data members common for both cases and then forget if it was a BVN or ConstantFP.

I don't think this will make much of a difference compile-time wise, so I'd prefer to go with the version where the code looks simpler ...

[jonpa]

I don't think this will make much of a difference compile-time wise, so I'd prefer to go with the version where the code looks simpler ...

OK. It does seem to simplify things to just do the work in the constructors - patch updated.

[uweigand]

OK, this version LGTM. Thanks!

[jonpa]

Thanks for help and review!

r354896

lib/Target/SystemZ/SystemZISelDAGToDAG.cpp

class SystemZDAGToDAGISel : public SelectionDAGISel {
   //   (Opcode UpperVal LowerVal)
   //
   // If Op0 is nonnull, then Node can be implemented using:
   //
   //   (Opcode (Opcode Op0 UpperVal) LowerVal)
   void splitLargeImmediate(unsigned Opcode, SDNode *Node, SDValue Op0,
                            uint64_t UpperVal, uint64_t LowerVal);
 
+  typedef SystemZTargetLowering::SystemZVectorConstantInfo
+          SystemZVectorConstantInfo;
+  void loadVectorConstant(const SystemZVectorConstantInfo &VCI,
+                          SDNode *Node);
+
   // Try to use gather instruction Opcode to implement vector insertion N.
   bool tryGather(SDNode *N, unsigned Opcode);
 
   // Try to use scatter instruction Opcode to implement store Store.
   bool tryScatter(StoreSDNode *Store, unsigned Opcode);
 
   // Change a chain of {load; op; store} of the same value into a simple op
   // through memory of that value, if the uses of the modified value and its
void SystemZDAGToDAGISel::splitLargeImmediate(unsigned Opcode, SDNode *Node,
   SDValue Lower = CurDAG->getConstant(LowerVal, DL, VT);
   SDValue Or = CurDAG->getNode(Opcode, DL, VT, Upper, Lower);
 
   ReplaceNode(Node, Or.getNode());
 
   SelectCode(Or.getNode());
 }
 
+void SystemZDAGToDAGISel::
+loadVectorConstant(const SystemZVectorConstantInfo &VCI, SDNode *Node) {
+  SDLoc DL(Node);
+  EVT VT = Node->getValueType(0);
+  SDValue Op;
+  SDValue BitCast;
+
+  if (VCI.Opcode == SystemZ::VGBM)
+    Op = SDValue(CurDAG->getMachineNode(SystemZ::VGBM, DL, MVT::v16i8,
+                    CurDAG->getTargetConstant(VCI.Mask, DL, MVT::i32)), 0);
+  else if (VCI.Opcode == SystemZ::VREPI)
+    Op = CurDAG->getNode(SystemZISD::REPLICATE, DL, VCI.VecVT,
+                   CurDAG->getConstant(VCI.ReplicatedImm, DL, MVT::i32, false,
+                                       true /*isOpaque*/));
+  else {
+    assert(VCI.Opcode == SystemZ::VGM && "Unhandled opcode.");
+    Op = CurDAG->getNode(
+        SystemZISD::ROTATE_MASK, DL, VCI.VecVT,
+        CurDAG->getConstant(VCI.Start, DL, MVT::i32, false, true /*isOpaque*/),
+        CurDAG->getConstant(VCI.End, DL, MVT::i32, false, true /*isOpaque*/));
+  }
+
+  if (!VT.isVector()) {
+    unsigned SubRegIdx = (VT.getSizeInBits() == 32 ? SystemZ::subreg_h32
+                                                   : SystemZ::subreg_h64);
+    SDNode *SubReg = CurDAG->getTargetExtractSubreg(SubRegIdx, DL, VT, Op)
+                                                    .getNode();
+    ReplaceNode(Node, SubReg);
+    if (!Op.isMachineOpcode())
+      SelectCode(Op.getNode());
+    return;
+  }
+  
+  BitCast = CurDAG->getNode(ISD::BITCAST, DL, VT, Op);
+  ReplaceNode(Node, BitCast.getNode());
+  if (!BitCast.isMachineOpcode())
+    SelectCode(BitCast.getNode());
+  if (Op != BitCast && !Op.isMachineOpcode()) {
+    assert(!Op.use_empty() && "Expected bitcasted SDValue to remain in DAG");
+    SelectCode(Op.getNode());
+  }
+}
+
 bool SystemZDAGToDAGISel::tryGather(SDNode *N, unsigned Opcode) {
   SDValue ElemV = N->getOperand(2);
   auto *ElemN = dyn_cast<ConstantSDNode>(ElemV);
   if (!ElemN)
     return false;
 
   unsigned Elem = ElemN->getZExtValue();
   EVT VT = N->getValueType(0);

 void SystemZDAGToDAGISel::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;
   }
 
+  const SystemZTargetLowering *TLI =
+    static_cast<const SystemZTargetLowering*>(getTargetLowering());
+
   unsigned Opcode = Node->getOpcode();
   switch (Opcode) {
   case ISD::OR:
     if (Node->getOperand(1).getOpcode() != ISD::Constant)
       if (tryRxSBG(Node, SystemZ::ROSBG))
         return;
     goto or_xor;
 
if (ElemBitSize == 32) {
       if (tryGather(Node, SystemZ::VGEG))
         return;
     }
     break;
   }
 
   case ISD::BUILD_VECTOR: {
     auto *BVN = cast<BuildVectorSDNode>(Node);
-    SDLoc DL(Node);
-    EVT VT = Node->getValueType(0);
-    uint64_t Mask = 0;
-    if (SystemZTargetLowering::tryBuildVectorByteMask(BVN, Mask)) {
-      SDNode *Res = CurDAG->getMachineNode(SystemZ::VGBM, DL, VT,
-                                CurDAG->getTargetConstant(Mask, DL, MVT::i32));
-      ReplaceNode(Node, Res);
+    SystemZVectorConstantInfo VCI(BVN);
+    if (TLI->isVectorConstantLegal(VCI)) {
+      loadVectorConstant(VCI, Node);
       return;
     }
     break;
   }
 
   case ISD::ConstantFP: {
     APFloat Imm = cast<ConstantFPSDNode>(Node)->getValueAPF();
     if (Imm.isZero() || Imm.isNegZero())
       break;
-    const SystemZInstrInfo *TII = getInstrInfo();
-    EVT VT = Node->getValueType(0);
-    unsigned Start, End;
-    unsigned BitWidth = VT.getSizeInBits();
-    bool Success = SystemZTargetLowering::analyzeFPImm(Imm, BitWidth, Start,
-              End, static_cast<const SystemZInstrInfo *>(TII)); (void)Success;
+    SystemZVectorConstantInfo VCI(Imm);
+    bool Success = TLI->isVectorConstantLegal(VCI); (void)Success;
     assert(Success && "Expected legal FP immediate");
-    SDLoc DL(Node);
-    unsigned Opcode = (BitWidth == 32 ? SystemZ::VGMF : SystemZ::VGMG);
-    SDNode *Res = CurDAG->getMachineNode(Opcode, DL, VT,
-                            CurDAG->getTargetConstant(Start, DL, MVT::i32),
-                            CurDAG->getTargetConstant(End, DL, MVT::i32));
-    unsigned SubRegIdx = (BitWidth == 32 ? SystemZ::subreg_h32
-                                         : SystemZ::subreg_h64);
-    Res = CurDAG->getTargetExtractSubreg(SubRegIdx, DL, VT, SDValue(Res, 0))
-            .getNode();
-    ReplaceNode(Node, Res);
+    loadVectorConstant(VCI, Node);
     return;
   }
 
   case ISD::STORE: {
     if (tryFoldLoadStoreIntoMemOperand(Node))
       return;
     auto *Store = cast<StoreSDNode>(Node);
     unsigned ElemBitSize = Store->getValue().getValueSizeInBits();

lib/Target/SystemZ/SystemZISelLowering.h

public:
   ISD::NodeType getExtendForAtomicOps() const override {
     return ISD::ANY_EXTEND;
   }
 
   bool supportSwiftError() const override {
     return true;
   }
 
-  static bool tryBuildVectorByteMask(BuildVectorSDNode *BVN, uint64_t &Mask);
-  static bool analyzeFPImm(const APFloat &Imm, unsigned BitWidth,
-                 unsigned &Start, unsigned &End, const SystemZInstrInfo *TII);
+  struct SystemZVectorConstantInfo {
+  private:
+    APFloat FPImm;
+    BuildVectorSDNode *BVN;
+
+    void init() { Mask = 0; ReplicatedImm = 0; Start = 0; End = 0; Opcode = 0; }
+  public:
+    
+    uint64_t Mask;          // VGBM
+    int64_t  ReplicatedImm; // VREPI
+    unsigned Start;         // VGM Start
+    unsigned End;           // VGM End
+    unsigned Opcode;
+    MVT      VecVT;
+    SystemZVectorConstantInfo(APFloat f) : FPImm(f), BVN(nullptr) {
+      init();
+      assert(FPImm.bitcastToAPInt().getBitWidth() <= 64 && "Unhandled FP!");
+    };
+    SystemZVectorConstantInfo(BuildVectorSDNode *b) : FPImm(0.0),
+                                                      BVN(b) {
+      init();
+      assert(BVN->isConstant() && "Expected a constant BUILD_VECTOR");
+    };
+
+    APInt getIntBits();
+    void getSplat(APInt &SplatBits, APInt &SplatUndef, unsigned &SplatBitSize);
+  };
+
+  bool isVectorConstantLegal(SystemZVectorConstantInfo &VCI) const;
+
 private:
   const SystemZSubtarget &Subtarget;
 
   // Implement LowerOperation for individual opcodes.
   SDValue getVectorCmp(SelectionDAG &DAG, unsigned Opcode,
                        const SDLoc &DL, EVT VT,
                        SDValue CmpOp0, SDValue CmpOp1) const;
   SDValue lowerVectorSETCC(SelectionDAG &DAG, const SDLoc &DL,
private:
   MachineBasicBlock *emitLoadAndTestCmp0(MachineInstr &MI,
                                          MachineBasicBlock *MBB,
                                          unsigned Opcode) const;
 
   const TargetRegisterClass *getRepRegClassFor(MVT VT) const override;
 };
 } // end namespace llvm
 
 #endif

lib/Target/SystemZ/SystemZISelLowering.cpp

case MVT::f128:
     return Subtarget.hasVectorEnhancements1();
   default:
     break;
   }
 
   return false;
 }
 
-
-// Return true if Imm can be generated with a vector instruction, such as VGM.
+// Return true if Bits can be generated with a vector instruction, such as
+// VGM, VGMB or VREPI. VCI is used to remember the details.
 bool SystemZTargetLowering::
-analyzeFPImm(const APFloat &Imm, unsigned BitWidth, unsigned &Start,
-             unsigned &End, const SystemZInstrInfo *TII) {
-  APInt IntImm = Imm.bitcastToAPInt();
-  if (IntImm.getActiveBits() > 64)
-    return false;
-
-  // See if this immediate could be generated with VGM.
-  bool Success = TII->isRxSBGMask(IntImm.getZExtValue(), BitWidth, Start, End);
-  if (!Success)
-    return false;
-  // isRxSBGMask returns the bit numbers for a full 64-bit value,
-  // with 0 denoting 1 << 63 and 63 denoting 1.  Convert them to
-  // bit numbers for an BitsPerElement value, so that 0 denotes
-  // 1 << (BitsPerElement-1).
-  Start -= 64 - BitWidth;
-  End -= 64 - BitWidth;
+isVectorConstantLegal(SystemZVectorConstantInfo &VCI) const {
+  if (!Subtarget.hasVector())
+    return false;
+
+  // Try using VECTOR GENERATE BYTE MASK.  This is the architecturally-
+  // preferred way of creating all-zero and all-one vectors so give it
+  // priority over other methods below.
+  APInt IntBits = VCI.getIntBits();
+  unsigned I = 0;
+  for (; I < SystemZ::VectorBytes; ++I) {
+    uint64_t Byte = IntBits.lshr(I * 8).trunc(8).getZExtValue();
+    if (Byte == 0xff)
+      VCI.Mask |= 1ULL << I;
+    else if (Byte != 0)
+      break;
+  }
+  if (I == SystemZ::VectorBytes) {
+    VCI.Opcode = SystemZ::VGBM;
+    return true;
+  }
+
+  APInt SplatBits, SplatUndef;
+  unsigned SplatBitSize;
+  VCI.getSplat(SplatBits, SplatUndef, SplatBitSize);
+  if (SplatBitSize > 64)
+    return false;
+
+  const SystemZInstrInfo *TII =
+    static_cast<const SystemZInstrInfo *>(Subtarget.getInstrInfo());
+
+  auto tryValue = [&](uint64_t Value) -> bool {
+    // Try VECTOR REPLICATE IMMEDIATE
+    int64_t SignedValue = SignExtend64(Value, SplatBitSize);
+    if (isInt<16>(SignedValue)) {
+      VCI.ReplicatedImm = SignedValue;
+      VCI.Opcode = SystemZ::VREPI;
+      VCI.VecVT = MVT::getVectorVT(MVT::getIntegerVT(SplatBitSize),
+                                   SystemZ::VectorBits / SplatBitSize);
+      return true;
+    }
+    // Try VECTOR GENERATE MASK
+    if (TII->isRxSBGMask(Value, SplatBitSize, VCI.Start, VCI.End)) {
+      // isRxSBGMask returns the bit numbers for a full 64-bit value, with 0
+      // denoting 1 << 63 and 63 denoting 1.  Convert them to bit numbers for
+      // an SplatBitSize value, so that 0 denotes 1 << (SplatBitSize-1).
+      VCI.Start -= 64 - SplatBitSize;
+      VCI.End -= 64 - SplatBitSize;
+      VCI.Opcode = SystemZ::VGM;
+      VCI.VecVT = MVT::getVectorVT(MVT::getIntegerVT(SplatBitSize),
+                                   SystemZ::VectorBits / SplatBitSize);
       return true;
     }
+    return false;
+  };
+
+  // First try assuming that any undefined bits above the highest set bit
+  // and below the lowest set bit are 1s.  This increases the likelihood of
+  // being able to use a sign-extended element value in VECTOR REPLICATE
+  // IMMEDIATE or a wraparound mask in VECTOR GENERATE MASK.
+  uint64_t SplatBitsZ = SplatBits.getZExtValue();
+  uint64_t SplatUndefZ = SplatUndef.getZExtValue();
+  uint64_t Lower =
+      (SplatUndefZ & ((uint64_t(1) << findFirstSet(SplatBitsZ)) - 1));
+  uint64_t Upper =
+      (SplatUndefZ & ~((uint64_t(1) << findLastSet(SplatBitsZ)) - 1));
+  if (tryValue(SplatBitsZ | Upper | Lower))
+    return true;
+
+  // Now try assuming that any undefined bits between the first and
+  // last defined set bits are set.  This increases the chances of
+  // using a non-wraparound mask.
+  uint64_t Middle = SplatUndefZ & ~Upper & ~Lower;
+  return tryValue(SplatBitsZ | Middle);
+}
+
+APInt SystemZTargetLowering::SystemZVectorConstantInfo::getIntBits() {
+  if (BVN != nullptr) {
+    APInt SplatBits, SplatUndef;
+    unsigned SplatBitSize;
+    bool HasAnyUndefs;
+    BVN->isConstantSplat(SplatBits, SplatUndef, SplatBitSize,
+                         HasAnyUndefs, 128, true);
+    return SplatBits;
+  }
+  return FPImm.bitcastToAPInt().zext(128);
+}
+
+void SystemZTargetLowering::SystemZVectorConstantInfo::
+getSplat(APInt &SplatBits, APInt &SplatUndef, unsigned &SplatBitSize) {
+  if (BVN != nullptr) {
+    bool HasAnyUndefs;
+    BVN->isConstantSplat(SplatBits, SplatUndef, SplatBitSize,
+                         HasAnyUndefs, 8, true);
+    return;
+  }
+
+  // Find the splat for the FP Constant.
+  SplatBits = FPImm.bitcastToAPInt();
+  unsigned Width = SplatBits.getBitWidth();
+  while (Width > 8) {
+    unsigned HalfSize = Width / 2;
+    APInt HighValue = SplatBits.lshr(HalfSize).trunc(HalfSize);
+    APInt LowValue = SplatBits.trunc(HalfSize);
+
+    // If the two halves do not match, stop here.
+    if (HighValue != LowValue || 8 > HalfSize)
+      break;
+
+    SplatBits = HighValue;
+    Width = HalfSize;
+  }
+  SplatUndef = 0;
+  SplatBitSize = Width;
+}
 
 bool SystemZTargetLowering::isFPImmLegal(const APFloat &Imm, EVT VT) const {
   // We can load zero using LZ?R and negative zero using LZ?R;LC?BR.
   if (Imm.isZero() || Imm.isNegZero())
     return true;
 
-  if (!Subtarget.hasVector())
+  if (Imm.bitcastToAPInt().getBitWidth() > 64)
     return false;
-  const SystemZInstrInfo *TII =
-      static_cast<const SystemZInstrInfo *>(Subtarget.getInstrInfo());
-  unsigned Start, End;
-  return analyzeFPImm(Imm, VT.getSizeInBits(), Start, End, TII);
+
+  SystemZVectorConstantInfo VCI(Imm);
+  return isVectorConstantLegal(VCI);
 }
 
 bool SystemZTargetLowering::isLegalICmpImmediate(int64_t Imm) const {
   // We can use CGFI or CLGFI.
   return isInt<32>(Imm) || isUInt<32>(Imm);
 }
 
 bool SystemZTargetLowering::isLegalAddImmediate(int64_t Imm) const {
else if (Op1.isUndef())
     Op0 = Op1 = DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i64, Op0);
   else {
     Op0 = DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i64, Op0);
     Op1 = DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i64, Op1);
   }
   return DAG.getNode(SystemZISD::JOIN_DWORDS, DL, MVT::v2i64, Op0, Op1);
 }
 
-// Try to represent constant BUILD_VECTOR node BVN using a BYTE MASK style
-// mask.  Store the mask value in Mask on success.
-bool SystemZTargetLowering::
-tryBuildVectorByteMask(BuildVectorSDNode *BVN, uint64_t &Mask) {
-  EVT ElemVT = BVN->getValueType(0).getVectorElementType();
-  unsigned BytesPerElement = ElemVT.getStoreSize();
-  for (unsigned I = 0, E = BVN->getNumOperands(); I != E; ++I) {
-    SDValue Op = BVN->getOperand(I);
-    if (!Op.isUndef()) {
-      uint64_t Value;
-      if (Op.getOpcode() == ISD::Constant)
-        Value = cast<ConstantSDNode>(Op)->getZExtValue();
-      else if (Op.getOpcode() == ISD::ConstantFP)
-        Value = (cast<ConstantFPSDNode>(Op)->getValueAPF().bitcastToAPInt()
-                 .getZExtValue());
-      else
-        return false;
-      for (unsigned J = 0; J < BytesPerElement; ++J) {
-        uint64_t Byte = (Value >> (J * 8)) & 0xff;
-        if (Byte == 0xff)
-          Mask |= 1ULL << ((E - I - 1) * BytesPerElement + J);
-        else if (Byte != 0)
-          return false;
-      }
-    }
-  }
-  return true;
-}
-
-// Try to load a vector constant in which BitsPerElement-bit value Value
-// is replicated to fill the vector.  VT is the type of the resulting
-// constant, which may have elements of a different size from BitsPerElement.
-// Return the SDValue of the constant on success, otherwise return
-// an empty value.
-static SDValue tryBuildVectorReplicate(SelectionDAG &DAG,
-                                       const SystemZInstrInfo *TII,
-                                       const SDLoc &DL, EVT VT, uint64_t Value,
-                                       unsigned BitsPerElement) {
-  // Signed 16-bit values can be replicated using VREPI.
-  // Mark the constants as opaque or DAGCombiner will convert back to
-  // BUILD_VECTOR.
-  int64_t SignedValue = SignExtend64(Value, BitsPerElement);
-  if (isInt<16>(SignedValue)) {
-    MVT VecVT = MVT::getVectorVT(MVT::getIntegerVT(BitsPerElement),
-                                 SystemZ::VectorBits / BitsPerElement);
-    SDValue Op = DAG.getNode(
-        SystemZISD::REPLICATE, DL, VecVT,
-        DAG.getConstant(SignedValue, DL, MVT::i32, false, true /*isOpaque*/));
-    return DAG.getNode(ISD::BITCAST, DL, VT, Op);
-  }
-  // See whether rotating the constant left some N places gives a value that
-  // is one less than a power of 2 (i.e. all zeros followed by all ones).
-  // If so we can use VGM.
-  unsigned Start, End;
-  if (TII->isRxSBGMask(Value, BitsPerElement, Start, End)) {
-    // isRxSBGMask returns the bit numbers for a full 64-bit value,
-    // with 0 denoting 1 << 63 and 63 denoting 1.  Convert them to
-    // bit numbers for an BitsPerElement value, so that 0 denotes
-    // 1 << (BitsPerElement-1).
-    Start -= 64 - BitsPerElement;
-    End -= 64 - BitsPerElement;
-    MVT VecVT = MVT::getVectorVT(MVT::getIntegerVT(BitsPerElement),
-                                 SystemZ::VectorBits / BitsPerElement);
-    SDValue Op = DAG.getNode(
-        SystemZISD::ROTATE_MASK, DL, VecVT,
-        DAG.getConstant(Start, DL, MVT::i32, false, true /*isOpaque*/),
-        DAG.getConstant(End, DL, MVT::i32, false, true /*isOpaque*/));
-    return DAG.getNode(ISD::BITCAST, DL, VT, Op);
-  }
-  return SDValue();
-}
-
 // If a BUILD_VECTOR contains some EXTRACT_VECTOR_ELTs, it's usually
 // better to use VECTOR_SHUFFLEs on them, only using BUILD_VECTOR for
 // the non-EXTRACT_VECTOR_ELT elements.  See if the given BUILD_VECTOR
 // would benefit from this representation and return it if so.
 static SDValue tryBuildVectorShuffle(SelectionDAG &DAG,
                                      BuildVectorSDNode *BVN) {
   EVT VT = BVN->getValueType(0);
   unsigned NumElements = VT.getVectorNumElements();
for (unsigned I = 0; I < NumElements; ++I)
     if (!Done[I] && !Elems[I].isUndef() && Elems[I] != ReplicatedVal)
       Result = DAG.getNode(ISD::INSERT_VECTOR_ELT, DL, VT, Result, Elems[I],
                            DAG.getConstant(I, DL, MVT::i32));
   return Result;
 }
 
 SDValue SystemZTargetLowering::lowerBUILD_VECTOR(SDValue Op,
                                                  SelectionDAG &DAG) const {
-  const SystemZInstrInfo *TII =
-    static_cast<const SystemZInstrInfo *>(Subtarget.getInstrInfo());
   auto *BVN = cast<BuildVectorSDNode>(Op.getNode());
   SDLoc DL(Op);
   EVT VT = Op.getValueType();
 
   if (BVN->isConstant()) {
-    // Try using VECTOR GENERATE BYTE MASK.  This is the architecturally-
-    // preferred way of creating all-zero and all-one vectors so give it
-    // priority over other methods below.
-    uint64_t Mask;
-    if (ISD::isBuildVectorAllZeros(Op.getNode()) ||
-        ISD::isBuildVectorAllOnes(Op.getNode()) ||
-        (VT.isInteger() && tryBuildVectorByteMask(BVN, Mask)))
+    SystemZVectorConstantInfo VCI(BVN);
+    if (isVectorConstantLegal(VCI))
       return Op;
 
-    // Try using some form of replication.
-    APInt SplatBits, SplatUndef;
-    unsigned SplatBitSize;
-    bool HasAnyUndefs;
-    if (BVN->isConstantSplat(SplatBits, SplatUndef, SplatBitSize, HasAnyUndefs,
-                             8, true) &&
-        SplatBitSize <= 64) {
-      // First try assuming that any undefined bits above the highest set bit
-      // and below the lowest set bit are 1s.  This increases the likelihood of
-      // being able to use a sign-extended element value in VECTOR REPLICATE
-      // IMMEDIATE or a wraparound mask in VECTOR GENERATE MASK.
-      uint64_t SplatBitsZ = SplatBits.getZExtValue();
-      uint64_t SplatUndefZ = SplatUndef.getZExtValue();
-      uint64_t Lower = (SplatUndefZ
-                        & ((uint64_t(1) << findFirstSet(SplatBitsZ)) - 1));
-      uint64_t Upper = (SplatUndefZ
-                        & ~((uint64_t(1) << findLastSet(SplatBitsZ)) - 1));
-      uint64_t Value = SplatBitsZ | Upper | Lower;
-      SDValue Op = tryBuildVectorReplicate(DAG, TII, DL, VT, Value,
-                                           SplatBitSize);
-      if (Op.getNode())
-        return Op;
-
-      // Now try assuming that any undefined bits between the first and
-      // last defined set bits are set.  This increases the chances of
-      // using a non-wraparound mask.
-      uint64_t Middle = SplatUndefZ & ~Upper & ~Lower;
-      Value = SplatBitsZ | Middle;
-      Op = tryBuildVectorReplicate(DAG, TII, DL, VT, Value, SplatBitSize);
-      if (Op.getNode())
-        return Op;
-    }
-
     // Fall back to loading it from memory.
     return SDValue();
   }
 
   // See if we should use shuffles to construct the vector from other vectors.
   if (SDValue Res = tryBuildVectorShuffle(DAG, BVN))
     return Res;
 

test/CodeGen/SystemZ/vec-const-05.ll

-; Test vector byte masks, v4f32 version. Only all-zero vectors are handled.
+; Test vector byte masks, v4f32 version.
 ;
 ; RUN: llc < %s -mtriple=s390x-linux-gnu -mcpu=z13 | FileCheck %s
 
 ; Test an all-zeros vector.
-define <4 x float> @f0() {
-; CHECK-LABEL: f0:
+define <4 x float> @f1() {
+; CHECK-LABEL: f1:
 ; CHECK: vgbm %v24, 0
 ; CHECK: br %r14
   ret <4 x float> zeroinitializer
 }
 
-; Test that undefs are treated as zero.
-define <4 x float> @f1() {
-; CHECK-LABEL: f1:
-; CHECK: vgbm %v24, 0
+; Test an all-ones vector.
+define <4 x float> @f2() {
+; CHECK-LABEL: f2:
+; CHECK: vgbm %v24, 65535
 ; CHECK: br %r14
-  ret <4 x float> <float zeroinitializer, float undef,
-                   float zeroinitializer, float undef>
+  ret <4 x float> <float 0xffffffffe0000000, float 0xffffffffe0000000,
+                   float 0xffffffffe0000000, float 0xffffffffe0000000>
+}
+
+; Test a mixed vector (mask 0xc731).
+define <4 x float> @f3() {
+; CHECK-LABEL: f3:
+; CHECK: vgbm %v24, 50993
+; CHECK: br %r14
+  ret <4 x float> <float 0xffffe00000000000, float 0x381fffffe0000000,
+                   float 0x379fffe000000000, float 0x371fe00000000000>
+}
+
+; Test that undefs are treated as zero (mask 0xc031).
+define <4 x float> @f4() {
+; CHECK-LABEL: f4:
+; CHECK: vgbm %v24, 49201
+; CHECK: br %r14
+  ret <4 x float> <float 0xffffe00000000000, float undef,
+                   float 0x379fffe000000000, float 0x371fe00000000000>
+}
+
+; Test that we don't use VGBM if one of the bytes is not 0 or 0xff.
+define <4 x float> @f5() {
+; CHECK-LABEL: f5:
+; CHECK-NOT: vgbm
+; CHECK: br %r14
+  ret <4 x float> <float 0xffffe00000000000, float 0x381fffffc0000000,
+                   float 0x379fffe000000000, float 0x371fe00000000000>
 }
 
 ; Test an all-zeros v2f32 that gets promoted to v4f32.
-define <2 x float> @f2() {
-; CHECK-LABEL: f2:
+define <2 x float> @f6() {
+; CHECK-LABEL: f6:
 ; CHECK: vgbm %v24, 0
 ; CHECK: br %r14
   ret <2 x float> zeroinitializer
 }
+
+; Test a mixed v2f32 that gets promoted to v4f32 (mask 0xc700).
+define <2 x float> @f7() {
+; CHECK-LABEL: f7:
+; CHECK: vgbm %v24, 50944
+; CHECK: br %r14
+  ret <2 x float> <float 0xffffe00000000000, float 0x381fffffe0000000>
+}

test/CodeGen/SystemZ/vec-const-06.ll

-; Test vector byte masks, v2f64 version. Only all-zero vectors are handled.
+; Test vector byte masks, v2f64 version.
 ;
 ; RUN: llc < %s -mtriple=s390x-linux-gnu -mcpu=z13 | FileCheck %s
 
 ; Test an all-zeros vector.
-define <2 x double> @f0() {
-; CHECK-LABEL: f0:
+define <2 x double> @f1() {
+; CHECK-LABEL: f1:
 ; CHECK: vgbm %v24, 0
 ; CHECK: br %r14
   ret <2 x double> zeroinitializer
 }
 
-; Test that undefs are treated as zero.
-define <2 x double> @f1() {
-; CHECK-LABEL: f1:
-; CHECK: vgbm %v24, 0
+; Test an all-ones vector.
+define <2 x double> @f2() {
+; CHECK-LABEL: f2:
+; CHECK: vgbm %v24, 65535
+; CHECK: br %r14
+  ret <2 x double> <double 0xffffffffffffffff, double 0xffffffffffffffff>
+}
+
+; Test a mixed vector (mask 0x8c76).
+define <2 x double> @f3() {
+; CHECK-LABEL: f3:
+; CHECK: vgbm %v24, 35958
+; CHECK: br %r14
+  ret <2 x double> <double 0xff000000ffff0000, double 0x00ffffff00ffff00>
+}
+
+; Test that undefs are treated as zero (mask 0x8c00).
+define <2 x double> @f4() {
+; CHECK-LABEL: f4:
+; CHECK: vgbm %v24, 35840
+; CHECK: br %r14
+  ret <2 x double> <double 0xff000000ffff0000, double undef>
+}
+
+; Test that we don't use VGBM if one of the bytes is not 0 or 0xff.
+define <2 x double> @f5() {
+; CHECK-LABEL: f5:
+; CHECK-NOT: vgbm
 ; CHECK: br %r14
-  ret <2 x double> <double zeroinitializer, double undef>
+  ret <2 x double> <double 0xfe000000ffff0000, double 0x00ffffff00ffff00>
 }

test/CodeGen/SystemZ/vec-const-19.ll

+; Test that a scalar FP constant can be reused from a vector splat constant
+; of the same value.
+;
+; RUN: llc < %s -mtriple=s390x-linux-gnu -mcpu=z13 | FileCheck %s
+
+define void @fun()  {
+; CHECK-LABEL: fun:
+; CHECK: vgmg %v0, 2, 10
+; CHECK-NOT: vgmg %v0, 2, 10
+
+  %tmp = fadd <2 x double> zeroinitializer, <double 1.000000e+00, double 1.000000e+00>
+  %tmp1 = fmul <2 x double> %tmp, <double 5.000000e-01, double 5.000000e-01>
+  store <2 x double> %tmp1, <2 x double>* undef
+  %tmp2 = load double, double* undef
+  %tmp3 = fmul double %tmp2, 5.000000e-01
+  store double %tmp3, double* undef
+  ret void
+}