Types of fractional LMUL and LMUL=1 are all using VR register class. When
using inline asm, it will use the first type in the register class as the
type for the register. It is not necessary the same as the value type. We
need to use INSERT_SUBVECTOR/EXTRACT_SUBVECToR/BITCAST to make it legal
to put the value in the corresponding register class.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
6214 | ValueVT.getVectorElementCount().getKnownMinValue() is equivalents to ValueVT.getVectorMinNumElements() I think. Or would ValueVT.getSizeInBits().getKnownMinValue() replace the whole thing? |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
6218 | In what situations are the element types not equal? Does the PartVT come from the first type in the RC? | |
6219 | Is this guaranteed not to become 0? | |
6220 | Maybe put a /*IsScalable*/ on the true? | |
llvm/test/CodeGen/RISCV/rvv/inline-asm.ll | ||
2 | Nit, but -o - and < %s isn't too common a combination: < %s or -o - %s? |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
6218 | Yes, the PartVT comes from the first type in the RC. Another way to handle it is to search the corresponding type in the RC in GetRegistersForValue(). The problem comes from we reuse VR register class for fractional LMUL types. | |
6219 | Yes, I reordered the supported types in VR. The first type has the element type i8. In addition, I created VM register class for mask register types. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
6218 | Thanks! (Un)Luckily I've seen the "first type" issue before: I wish there was a better way. | |
6219 | Cool. Then perhaps we can check/assert on this, just so the types aren't accidentally reordered in the future? This seems to me like quite a few implicit dependencies being used and handed to us via this method. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
6218 | @rogfer01 provided another way to deal with the issue in GetRegistersForValue() in their repository. He searched the types in the register class for the value type. Do you think it is better to do in this way or provide target hooks to deal with it? @@ -7906,7 +7906,20 @@ static void GetRegistersForValue(SelectionDAG &DAG, const SDLoc & // Get the actual register value type. This is important, because the user // may have asked for (e.g.) the AX register in i32 type. We need to // remember that AX is actually i16 to get the right extension. - const MVT RegVT = *TRI.legalclasstypes_begin(*RC); + MVT RegVT = *TRI.legalclasstypes_begin(*RC); + // Try harder for vectors just in case sizes don't match. + // FIXME: This is caused by having nxv1i1 in the same register class as + // nxv1i64 in EPI. + if (RegVT.isVector() && RefOpInfo.ConstraintVT.isVector() && + RegVT.getSizeInBits() != RefOpInfo.ConstraintVT.getSizeInBits()) { + auto E = TRI.legalclasstypes_end(*RC); + auto I = std::find_if(TRI.legalclasstypes_begin(*RC), E, + [&RefOpInfo](const MVT::SimpleValueType &VTy) { + return MVT(VTy) == RefOpInfo.ConstraintVT; + }); + if (I != E) + RegVT = *I; + } |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
5690 | I might be missing something: could VM be folded into the loop below? | |
6218 | It's not really specific to vectors, so if we were to go down that route I'd probably expect us to have to offer it up to everything. If I remember correctly, I once had i8,i16,i32,v2i8,v4i8,v2i16 (maybe others) in the same "scalar" register class. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
6218 | It seems only RISC-V encountered the problem when choosing the type for the register classes. That is why I provided the target hooks to avoid the problems instead of adding checking in GetRegistersForValue(). If you think it is appropriate to add checking here, I think it is a simpler way to solve the problem. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
6218 | I think it's fine as you've got it, thanks. |
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | ||
---|---|---|
8087 ↗ | (On Diff #329164) | I prefer to use target hooks to solve the issue. |
I might be missing something: could VM be folded into the loop below?