Adding MVT::i4 and MVT::i2

Hi,

as part of some work required in our RISC-V V-extension backend we would like to have vector machine value types whose elements are of type i4 and i2. Currently i4 and i2 are not MVT as such and are handled when needed as extended value types (EVTs being, as far as I understand, like a superset of MVTs).

One interesting effect of doing this is that I found that there may be nodes with illegal types after type-legalization. One example (not sure if there are others) is SIGN_EXTEND_INREG that has a special operand whose type may be illegal (only MVTs can be legal, so any EVT that is not an MVT I understand it must be considered illegal too). This operand represents the “original” type (an EVT) being extended. Currently things work for an extension of i4 or i2 because operation legalization does the right thing (i.e. Expand) when it finds an “extended” type (like i4 or i2). If the type is not extended it defers the action to the target.

By making these types MVT, now they are not extended types any more. By default SIGN_EXTEND_INREG action is Legal so this causes instruction selection problems (e.g. most backends choke when trying to sign extend inreg from an i4 or an i2).

For now I have added the following to TargetLoweringBase

for (MVT VT : {MVT::i2, MVT::i4}) {
setOperationAction(ISD::SIGN_EXTEND_INREG, VT, Expand);

}

which avoids most of the crashes though there are a few CodeGen tests that are
changing. These are AArch64/fast-isel-branch-cond-mask.ll, AMDGPU/idot8u.ll and
X86/load-local-v3i1.ll.

I presume the change should be in principle NFC so I’m left wondering if this is the
right approach. Maybe it is not because there are underlying assumptions or implications I’m unaware for MVTs smaller than i8 (currently only i1).

Is introducing sub-i8 MVTs reasonable?

Thank you very much,

The general design intention of MVTs is that we should add them when a supported target needs them, so I think this is the right approach. Are you proposing that all the vectors of i2/i4 be supported as well as MVTs?

The issues you’re pointing out above are long standardizing design problems - I think that adding those actions to TargetLoweringBase is totally reasonable. Do you know what the root of the other changed codegen tests are?

-Chris