[SystemZ] Use SystemZVectorConstantInfo for more immediate stores.

[jonpa]

Summary

This is a follow-up patch that came up when working on the handling of stores of replicated values. This is the case that involves loading any integer constant into a register and immediately storing it. There are some cases where VGBM / VGM can be used for this.

This (at least currently) involves a new SystemZ method "isSupportedByScalarStore()" which is used to find cases where it would be worthwhile to use a vector constant instead. I have tried two variants:

1. Replace all immediate loads that would require one or two scalar instructions.
2. Only replace cases where two scalar instructions would be needed (-keep-imm-load passed).

(1) might make sense as there are more vector registers so register pressure might be improved. It seems that one time that gave less spills, and the other time it was more, and only with small differences, so that does not really seem to be a big deal.

(2) Is a bit more of a clear improvement as one immediate load is used instead of two scalar ones always.

main <> patch  (1)
vsteg          :                 6564                 9016    +2452
stg            :               388272               385858    -2414
vstef          :                 6615                 8761    +2146
st             :               130605               128843    -1762
vgmf           :                13125                14464    +1339
vgmg           :                 8152                 9472    +1320
llilh          :                 6956                 5825    -1131
llihh          :                 2549                 1692     -857
llihl          :                 7114                 6668     -446
stfh           :                17089                16791     -298
vgbm           :                11085                11288     +203
oilh           :                 1539                 1351     -188
llihf          :                11156                10978     -178
oilf           :                 8959                 8819     -140
iilf           :                 5084                 4965     -119
iihf           :                 4918                 4814     -104
lay            :                56028                56126      +98
llilf          :                 2249                 2177      -72
oill           :                18106                18045      -61
...
OPCDIFFS: -212

main <> patch with experimental option "-keep-imm-load" passed (2)
stg            :               388272               387648     -624
vsteg          :                 6564                 7177     +613
vgmg           :                 8152                 8355     +203
oilh           :                 1539                 1351     -188
llihh          :                 2549                 2367     -182
vgmf           :                13125                13304     +179
oilf           :                 8959                 8819     -140
llihl          :                 7114                 7003     -111
llihf          :                11156                11065      -91
oill           :                18106                18045      -61
lg             :              1050050              1050003      -47
vgbm           :                11085                11122      +37
lay            :                56028                56058      +30
mvc            :                59773                59755      -18
lgr            :               842765               842758       -7
cgije          :               142634               142629       -5
iihf           :                 4918                 4913       -5
je             :               337365               337370       +5
ldr            :                51266                51270       +4
OPCDIFFS: -408

With the latest build (2) seems better, but again I am not sure how stable that is as it seems to change over time.

The patch is not quite finalized and the next step would be to decide between (1) and (2)...

Nightly performance measurements do not show any changes above 1%. Given these measurements I would take (2) as there are a few 0.25% improvements with that, but I am not sure so small differences are trustworthy...

[jonpa]

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

6446

XXX is for maybe using isSupportedByScalarStore() here as well

[jonpa]

Does not seem to give any improvements on benchmarks...

llvm/lib/Target/SystemZ/SystemZISelDAGToDAG.cpp

class SystemZDAGToDAGISel : public SelectionDAGISel {
   //
   // 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);
 
   void loadVectorConstant(const SystemZVectorConstantInfo &VCI,
-                          SDNode *Node);
+                          SDNode *Node, EVT VT);
 
   // 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
void SystemZDAGToDAGISel::splitLargeImmediate(unsigned Opcode, SDNode *Node,
   SDValue Or = CurDAG->getNode(Opcode, DL, VT, Upper, Lower);
 
   ReplaceNode(Node, Or.getNode());
 
   SelectCode(Or.getNode());
 }
 
 void SystemZDAGToDAGISel::loadVectorConstant(
-    const SystemZVectorConstantInfo &VCI, SDNode *Node) {
+    const SystemZVectorConstantInfo &VCI, SDNode *Node, EVT VT) {
   assert((VCI.Opcode == SystemZISD::BYTE_MASK ||
           VCI.Opcode == SystemZISD::REPLICATE ||
           VCI.Opcode == SystemZISD::ROTATE_MASK) &&
          "Bad opcode!");
   assert(VCI.VecVT.getSizeInBits() == 128 && "Expected a vector type");
-  EVT VT = Node->getValueType(0);
   SDLoc DL(Node);
   SmallVector<SDValue, 2> Ops;
   for (unsigned OpVal : VCI.OpVals)
     Ops.push_back(CurDAG->getTargetConstant(OpVal, DL, MVT::i32));
   SDValue Op = CurDAG->getNode(VCI.Opcode, DL, VCI.VecVT, Ops);
 
   if (VCI.VecVT == VT.getSimpleVT())
     ReplaceNode(Node, Op.getNode());
   else if (VT.getSizeInBits() == 128) {
     SDValue BitCast = CurDAG->getNode(ISD::BITCAST, DL, VT, Op);
     ReplaceNode(Node, BitCast.getNode());
     SelectCode(BitCast.getNode());
-  } else { // float or double
+  } else if (VT.isFloatingPoint()) {
     unsigned SubRegIdx =
         (VT.getSizeInBits() == 32 ? SystemZ::subreg_h32 : SystemZ::subreg_h64);
     ReplaceNode(
         Node, CurDAG->getTargetExtractSubreg(SubRegIdx, DL, VT, Op).getNode());
+  } else {
+    unsigned NumBytes = VT.getStoreSize();
+    assert((NumBytes == 4 || NumBytes == 8) && "Unexpected vector element size");
+    EVT VecVT = EVT::getVectorVT(*CurDAG->getContext(), VT,
+                                 SystemZ::VectorBytes / NumBytes);
+    SDValue BitCast = CurDAG->getBitcast(VecVT, Op);
+    SDValue ValueToUse = CurDAG->getNode(ISD::EXTRACT_VECTOR_ELT, DL, VT,
+                                 BitCast, CurDAG->getVectorIdxConstant(0, DL));
+    ReplaceNode(Node, ValueToUse.getNode());
   }
   SelectCode(Op.getNode());
 }
 
 bool SystemZDAGToDAGISel::tryGather(SDNode *N, unsigned Opcode) {
   SDValue ElemV = N->getOperand(2);
   auto *ElemN = dyn_cast<ConstantSDNode>(ElemV);
   if (!ElemN)
case ISD::INSERT_VECTOR_ELT: {
     }
     break;
   }
 
   case ISD::BUILD_VECTOR: {
     auto *BVN = cast<BuildVectorSDNode>(Node);
     SystemZVectorConstantInfo VCI(BVN);
     if (VCI.isVectorConstantLegal(*Subtarget)) {
-      loadVectorConstant(VCI, Node);
+      loadVectorConstant(VCI, Node, Node->getValueType(0));
       return;
     }
     break;
   }
 
   case ISD::ConstantFP: {
     APFloat Imm = cast<ConstantFPSDNode>(Node)->getValueAPF();
     if (Imm.isZero() || Imm.isNegZero())
       break;
     SystemZVectorConstantInfo VCI(Imm);
     bool Success = VCI.isVectorConstantLegal(*Subtarget); (void)Success;
     assert(Success && "Expected legal FP immediate");
-    loadVectorConstant(VCI, Node);
+    loadVectorConstant(VCI, Node, Node->getValueType(0));
     return;
   }
 
   case ISD::STORE: {
     if (tryFoldLoadStoreIntoMemOperand(Node))
       return;
     auto *Store = cast<StoreSDNode>(Node);
     unsigned ElemBitSize = Store->getValue().getValueSizeInBits();
     if (ElemBitSize == 32) {
       if (tryScatter(Store, SystemZ::VSCEF))
         return;
     } else if (ElemBitSize == 64) {
       if (tryScatter(Store, SystemZ::VSCEG))
         return;
     }
+    auto &Op1 = Node->getOperand(1);
+    if (auto *C = dyn_cast<ConstantSDNode>(Op1)) {
+      EVT MemVT = Store->getMemoryVT();
+      unsigned NumMemBytes = MemVT.getStoreSize();
+      const SystemZInstrInfo *TII = getInstrInfo();
+      if (C->getAPIntValue().getBitWidth() <= 64 &&
+          !TII->isSupportedByScalarStore(C->getAPIntValue(), MemVT)) {
+        SmallVector<SDNode *, 4> Stores;
+        for (auto *U : C->uses())
+          if (StoreSDNode *ST = dyn_cast<StoreSDNode>(U))
+            Stores.push_back(ST);
+        if (Stores.size() == C->use_size()) {
+          SystemZVectorConstantInfo VCI(C->getAPIntValue());
+          if (VCI.isVectorConstantLegal(*Subtarget)) {
+            loadVectorConstant(VCI, Op1.getNode(), MemVT);
+            // Need to select all stores into VSTE before the bitcast is removed.
+            for (auto *STNode : Stores)
+              SelectCode(STNode);
+            auto &StoredVal = Node->getOperand(0);
+            if (StoredVal->getOpcode() == ISD::BITCAST)
+              SelectCode(StoredVal.getNode());
+            return;
+          }
+        }
+      }
+    }
     break;
   }
   }
 
   SelectCode(Node);
 }
 
 bool SystemZDAGToDAGISel::

llvm/lib/Target/SystemZ/SystemZISelLowering.cpp

SDValue SystemZTargetLowering::combineSTORE(
   if (Subtarget.hasVector() && DCI.Level == BeforeLegalizeTypes &&
       isOnlyUsedByStores(Op1, DAG)) {
     SDValue Word = SDValue();
     EVT WordVT;
 
     // Find a replicated immediate and return it if found in Word and its
     // type in WordVT.
     auto FindReplicatedImm = [&](ConstantSDNode *C, unsigned TotBytes) {
       // Some constants are better handled with a scalar store.
-      if (C->getAPIntValue().getBitWidth() > 64 || C->isAllOnes() ||
+      if (C->getAPIntValue().getBitWidth() > 64 || C->isAllOnes() ||  // XXX
           isInt<16>(C->getSExtValue()) || MemVT.getStoreSize() <= 2)
         return;
       SystemZVectorConstantInfo VCI(APInt(TotBytes * 8, C->getZExtValue()));
       if (VCI.isVectorConstantLegal(Subtarget) &&
           VCI.Opcode == SystemZISD::REPLICATE) {
         Word = DAG.getConstant(VCI.OpVals[0], SDLoc(SN), MVT::i32);
         WordVT = VCI.VecVT.getScalarType();
       }

llvm/lib/Target/SystemZ/SystemZInstrInfo.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_TARGET_SYSTEMZ_SYSTEMZINSTRINFO_H
 #define LLVM_LIB_TARGET_SYSTEMZ_SYSTEMZINSTRINFO_H
 
 #include "SystemZ.h"
 #include "SystemZRegisterInfo.h"
+#include "llvm/ADT/APInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/TargetInstrInfo.h"
+#include "llvm/CodeGen/ValueTypes.h"
 #include <cstdint>
 
 #define GET_INSTRINFO_HEADER
 #include "SystemZGenInstrInfo.inc"
 
 namespace llvm {
 
 class SystemZSubtarget;
public:
   unsigned getLoadAndTrap(unsigned Opcode) const;
 
   // Emit code before MBBI in MI to move immediate value Value into
   // physical register Reg.
   void loadImmediate(MachineBasicBlock &MBB,
                      MachineBasicBlock::iterator MBBI,
                      unsigned Reg, uint64_t Value) const;
 
+  bool isSupportedByScalarStore(const APInt &Val, EVT MemVT) const;
+
   // Perform target specific instruction verification.
   bool verifyInstruction(const MachineInstr &MI,
                          StringRef &ErrInfo) const override;
 
   // Sometimes, it is possible for the target to tell, even without
   // aliasing information, that two MIs access different memory
   // addresses. This function returns true if two MIs access different
   // memory addresses and false otherwise.
   bool
   areMemAccessesTriviallyDisjoint(const MachineInstr &MIa,
                                   const MachineInstr &MIb) const override;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_TARGET_SYSTEMZ_SYSTEMZINSTRINFO_H

llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp

void SystemZInstrInfo::loadImmediate(MachineBasicBlock &MBB,
   Register Reg1 = MRI.createVirtualRegister(&SystemZ::GR64BitRegClass);
   BuildMI(MBB, MBBI, DL, get(SystemZ::IMPLICIT_DEF), Reg0);
   BuildMI(MBB, MBBI, DL, get(SystemZ::IIHF64), Reg1)
     .addReg(Reg0).addImm(Value >> 32);
   BuildMI(MBB, MBBI, DL, get(SystemZ::IILF64), Reg)
     .addReg(Reg1).addImm(Value & ((uint64_t(1) << 32) - 1));
 }
 
+// EXPERIMENTAL
+static cl::opt<bool> KEEP_IMM_LOAD("keep-imm-load", cl::init(false));
+
+bool SystemZInstrInfo::isSupportedByScalarStore(const APInt &Val,  // XXX
+                                                EVT MemVT) const {
+  if (MemVT.getStoreSize() <= 2)
+    return true;
+  if (Val.getBitWidth() > 64)
+    return false;
+
+  uint64_t UVal = Val.getZExtValue();
+  int64_t SVal = Val.getSExtValue();
+
+  if (isInt<16>(SVal) || Val.isAllOnes())
+    return true;
+
+  if (KEEP_IMM_LOAD)
+    if (SystemZ::isImmLL(UVal) || SystemZ::isImmLH(UVal) ||
+        SystemZ::isImmHL(UVal) || SystemZ::isImmHH(UVal) ||
+        SystemZ::isImmLF(UVal) || SystemZ::isImmHF(UVal) ||
+        isInt<32>(SVal))
+      return true;
+
+  return false;
+}
+
 bool SystemZInstrInfo::verifyInstruction(const MachineInstr &MI,
                                          StringRef &ErrInfo) const {
   const MCInstrDesc &MCID = MI.getDesc();
   for (unsigned I = 0, E = MI.getNumOperands(); I != E; ++I) {
     if (I >= MCID.getNumOperands())
       break;
     const MachineOperand &Op = MI.getOperand(I);
     const MCOperandInfo &MCOI = MCID.OpInfo[I];