[SelectionDAG] Require constant index for INSERT/EXTRACT_SUBVECTOR.

[efriedma]

Summary

A variable index sounds like an interesting idea in theory, but nothing is actually taking advantage of it, and specifying/implementing the edge cases is painful. So just forbid it.

[RKSimon]

Thank you - I'd love to see this - it'll allow me to strip a load of untested code paths in ComputeNumBits/SimplifyDemanded etc..

[craig.topper]

LGTM

llvm/include/llvm/CodeGen/ISDOpcodes.h

namespace ISD {
   /// instruction sets as much as possible, and only use target-dependent
   /// operators when they have special requirements.
   ///
   /// Finally, during and after selection proper, SNodes may use special
   /// operator codes that correspond directly with MachineInstr opcodes. These
   /// are used to represent selected instructions. See the isMachineOpcode()
   /// and getMachineOpcode() member functions of SDNode.
   ///
   enum NodeType {
     /// DELETED_NODE - This is an illegal value that is used to catch
     /// errors.  This opcode is not a legal opcode for any node.
     DELETED_NODE,
 
     /// EntryToken - This is the marker used to indicate the start of a region.
     EntryToken,
 
     /// TokenFactor - This node takes multiple tokens as input and produces a
enum NodeType {
 
     /// CONCAT_VECTORS(VECTOR0, VECTOR1, ...) - Given a number of values of
     /// vector type with the same length and element type, this produces a
     /// concatenated vector result value, with length equal to the sum of the
     /// lengths of the input vectors.
     CONCAT_VECTORS,
 
     /// INSERT_SUBVECTOR(VECTOR1, VECTOR2, IDX) - Returns a vector
-    /// with VECTOR2 inserted into VECTOR1 at the (potentially
-    /// variable) element number IDX, which must be a multiple of the
-    /// VECTOR2 vector length.  The elements of VECTOR1 starting at
-    /// IDX are overwritten with VECTOR2.  Elements IDX through
-    /// vector_length(VECTOR2) must be valid VECTOR1 indices.
+    /// with VECTOR2 inserted into VECTOR1 at the constant element number
+    /// IDX, which must be a multiple of the VECTOR2 vector length. The
+    /// elements of VECTOR1 starting at IDX are overwritten with VECTOR2.
+    /// Elements IDX through vector_length(VECTOR2) must be valid VECTOR1
+    /// indices.
     INSERT_SUBVECTOR,
 
     /// EXTRACT_SUBVECTOR(VECTOR, IDX) - Returns a subvector from VECTOR (an
-    /// vector value) starting with the element number IDX, which must be a
-    /// constant multiple of the result vector length.
+    /// vector value) starting with the constant element number IDX, which
+    /// must be a multiple of the result vector length. Elements IDX through
+    /// vector_length(VECTOR) must be valid VECTOR indices.
     EXTRACT_SUBVECTOR,
 
     /// VECTOR_SHUFFLE(VEC1, VEC2) - Returns a vector, of the same type as
     /// VEC1/VEC2.  A VECTOR_SHUFFLE node also contains an array of constant int
     /// values that indicate which value (or undef) each result element will
     /// get.  These constant ints are accessible through the
     /// ShuffleVectorSDNode class.  This is quite similar to the Altivec
     /// 'vperm' instruction, except that the indices must be constants and are
namespace ISD {
   /// this value. Those that do must not be less than this value, and can
   /// be used with SelectionDAG::getMemIntrinsicNode.
   static const int FIRST_TARGET_MEMORY_OPCODE = BUILTIN_OP_END+500;
 
   //===--------------------------------------------------------------------===//
   /// MemIndexedMode enum - This enum defines the load / store indexed
   /// addressing modes.
   ///
   /// UNINDEXED    "Normal" load / store. The effective address is already
   ///              computed and is available in the base pointer. The offset
   ///              operand is always undefined. In addition to producing a
   ///              chain, an unindexed load produces one value (result of the
   ///              load); an unindexed store does not produce a value.
   ///
   /// PRE_INC      Similar to the unindexed mode where the effective address is
   /// PRE_DEC      the value of the base pointer add / subtract the offset.
   ///              It considers the computation as being folded into the load /
   ///              store operation (i.e. the load / store does the address
   ///              computation as well as performing the memory transaction).
   ///              The base operand is always undefined. In addition to
   ///              producing a chain, pre-indexed load produces two values
   ///              (result of the load and the result of the address
   ///              computation); a pre-indexed store produces one value (result
   ///              of the address computation).
   ///
   /// POST_INC     The effective address is the value of the base pointer. The
   /// POST_DEC     value of the offset operand is then added to / subtracted
   ///              from the base after memory transaction. In addition to
   ///              producing a chain, post-indexed load produces two values
   ///              (the result of the load and the result of the base +/- offset
   ///              computation); a post-indexed store produces one value (the
   ///              the result of the base +/- offset computation).
   enum MemIndexedMode {
     UNINDEXED = 0,
     PRE_INC,
     PRE_DEC,
     POST_INC,
     POST_DEC
   };
 
   static const int LAST_INDEXED_MODE = POST_DEC + 1;
 
   //===--------------------------------------------------------------------===//
   /// MemIndexType enum - This enum defines how to interpret MGATHER/SCATTER's
   /// index parameter when calculating addresses.
   ///
   /// SIGNED_SCALED     Addr = Base + ((signed)Index * sizeof(element))
   /// SIGNED_UNSCALED   Addr = Base + (signed)Index
   /// UNSIGNED_SCALED   Addr = Base + ((unsigned)Index * sizeof(element))
   /// UNSIGNED_UNSCALED Addr = Base + (unsigned)Index
   enum MemIndexType {
     SIGNED_SCALED = 0,
     SIGNED_UNSCALED,
     UNSIGNED_SCALED,
     UNSIGNED_UNSCALED

   //===--------------------------------------------------------------------===//
   /// ISD::CondCode enum - These are ordered carefully to make the bitfields
   /// below work out, when considering SETFALSE (something that never exists
   /// dynamically) as 0.  "U" -> Unsigned (for integer operands) or Unordered
   /// (for floating point), "L" -> Less than, "G" -> Greater than, "E" -> Equal
   /// to.  If the "N" column is 1, the result of the comparison is undefined if
   /// the input is a NAN.
   ///
   /// All of these (except for the 'always folded ops') should be handled for
   /// floating point.  For integer, only the SETEQ,SETNE,SETLT,SETLE,SETGT,
   /// SETGE,SETULT,SETULE,SETUGT, and SETUGE opcodes are used.
   ///
   /// Note that these are laid out in a specific order to allow bit-twiddling
   /// to transform conditions.
   enum CondCode {
     // Opcode          N U L G E       Intuitive operation
     SETFALSE,      //    0 0 0 0       Always false (always folded)
     SETOEQ,        //    0 0 0 1       True if ordered and equal
     SETOGT,        //    0 0 1 0       True if ordered and greater than
     SETOGE,        //    0 0 1 1       True if ordered and greater than or equal
     SETOLT,        //    0 1 0 0       True if ordered and less than

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

case ISD::EXTRACT_ELEMENT:
     if (N1C) {
       unsigned ElementSize = VT.getSizeInBits();
       unsigned Shift = ElementSize * N2C->getZExtValue();
       APInt ShiftedVal = N1C->getAPIntValue().lshr(Shift);
       return getConstant(ShiftedVal.trunc(ElementSize), DL, VT);
     }
     break;
   case ISD::EXTRACT_SUBVECTOR:
-    if (VT.isSimple() && N1.getValueType().isSimple()) {
-      assert(VT.isVector() && N1.getValueType().isVector() &&
-             "Extract subvector VTs must be a vectors!");
-      assert(VT.getVectorElementType() ==
-             N1.getValueType().getVectorElementType() &&
-             "Extract subvector VTs must have the same element type!");
-      assert(VT.getSimpleVT() <= N1.getSimpleValueType() &&
-             "Extract subvector must be from larger vector to smaller vector!");
-
-      if (N2C) {
-        assert((VT.getVectorNumElements() + N2C->getZExtValue()
-                <= N1.getValueType().getVectorNumElements())
-               && "Extract subvector overflow!");
-      }
+    assert(VT.getVectorNumElements() <=
+           N1.getValueType().getVectorNumElements() &&
+    assert(N2C && "Extract subvector index must be a constant");
+    assert(VT.getVectorNumElements() + N2C->getZExtValue() <=
+           N1.getValueType().getVectorNumElements() &&
+           "Extract subvector overflow!");
 
-      // Trivial extraction.
-      if (VT.getSimpleVT() == N1.getSimpleValueType())
-        return N1;
+    if (VT == N1.getValueType())
 
-      // EXTRACT_SUBVECTOR of an UNDEF is an UNDEF.
-      if (N1.isUndef())
-        return getUNDEF(VT);
 
-      // EXTRACT_SUBVECTOR of CONCAT_VECTOR can be simplified if the pieces of
-      // the concat have the same type as the extract.
-      if (N2C && N1.getOpcode() == ISD::CONCAT_VECTORS &&
-          N1.getNumOperands() > 0 &&
-          VT == N1.getOperand(0).getValueType()) {
-        unsigned Factor = VT.getVectorNumElements();
-        return N1.getOperand(N2C->getZExtValue() / Factor);
-      }
 
-      // EXTRACT_SUBVECTOR of INSERT_SUBVECTOR is often created
-      // during shuffle legalization.
-      if (N1.getOpcode() == ISD::INSERT_SUBVECTOR && N2 == N1.getOperand(2) &&
-          VT == N1.getOperand(1).getValueType())
-        return N1.getOperand(1);
-    }
     break;
   }
 
   // Perform trivial constant folding.
   if (SDValue SV = FoldConstantArithmetic(Opcode, DL, VT, {N1, N2}))
     return SV;
 
   if (SDValue V = foldConstantFPMath(Opcode, DL, VT, N1, N2))
case ISD::INSERT_VECTOR_ELT: {
 
     break;
   }
   case ISD::INSERT_SUBVECTOR: {
     // Inserting undef into undef is still undef.
     if (N1.isUndef() && N2.isUndef())
       return getUNDEF(VT);
     SDValue Index = N3;
-    if (VT.isSimple() && N1.getValueType().isSimple()
-        && N2.getValueType().isSimple()) {
-      assert(VT.isVector() && N1.getValueType().isVector() &&
-             N2.getValueType().isVector() &&
-             "Insert subvector VTs must be a vectors");
-      assert(VT == N1.getValueType() &&
-             "Dest and insert subvector source types must match!");
-      assert(N2.getSimpleValueType() <= N1.getSimpleValueType() &&
-             "Insert subvector must be from smaller vector to larger vector!");
-      if (isa<ConstantSDNode>(Index)) {
-        assert((N2.getValueType().getVectorNumElements() +
-                cast<ConstantSDNode>(Index)->getZExtValue()
-                <= VT.getVectorNumElements())
-               && "Insert subvector overflow!");
-      }
+    assert(isa<ConstantSDNode>(Index) &&
+           "Insert subvector index must be constant");
+    assert(N2.getValueType().getVectorNumElements() +
+           cast<ConstantSDNode>(Index)->getZExtValue() <=
+           VT.getVectorNumElements() &&
+           "Insert subvector overflow!");
 
-      // Trivial insertion.
-      if (VT.getSimpleVT() == N2.getSimpleValueType())
-        return N2;
+    if (VT == N2.getValueType())
 
-      // If this is an insert of an extracted vector into an undef vector, we
-      // can just use the input to the extract.
-      if (N1.isUndef() && N2.getOpcode() == ISD::EXTRACT_SUBVECTOR &&
-          N2.getOperand(1) == N3 && N2.getOperand(0).getValueType() == VT)
-        return N2.getOperand(0);
-    }
     break;
   }
   case ISD::BITCAST:
     // Fold bit_convert nodes from a type to themselves.
     if (N1.getValueType() == VT)
       return N1;
     break;
   }