[RISC-V] Fix alignment for vector types #99589
Conversation
For vector256 and vector512, set possible max alignment value. Reference https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc
dotnet-policy-service
bot
added
the
community-contribution
| else if (defType.Context.Target.Architecture == TargetArchitecture.RiscV64) | ||
| { | ||
| // The alignment requirement for the fixed length vector shall be equivalent to | ||
| // the alignment requirement of its elemental type. |
There was a problem hiding this comment.
What do you mean by "elemental type" in this comment?
There was a problem hiding this comment.
I meant - could you please share the definition of "elemental type"? I do not think "elemental type" is defined anywhere in .NET. Is "elemental type" defined in the RISCV calling convention spec somewhere?
There was a problem hiding this comment.
Sorry. Actually, I haven't developed with vector. So I don't know well. This is just what I checked.
I read https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc document and update the comments.
In the doc, it mentions vector alignment twice.
The first is for fixed length vector and the second is vector types in RISC-V vector extension intrinsic document.
Initially, I wrote the comment from the fixed length vector. As I know, elemental type is type of elements of fixed length vector according to Various compilers have support for fixed-length vector types, for example GCC and Clang both support declaring a type with attributevector_size(N, where N is a positive number larger than zero in the document.
As I know, in RISC-V, max alignment is 16. So I need to update vector alignment within 16 to avoid the asserts before intrinsic is implemented. (Maybe we can start from next year.)
There was a problem hiding this comment.
The fixed length vectors mentioned in the RISCV ABI spec should be mapped to InlineArray in .NET. They should not be mapped to System.Numerics.Vector... that VectorFieldLayoutAlgorithm.cs is concerned about.
There was a problem hiding this comment.
Thank you for the comment. The fixed length vectors are not for Vector.... I removed the comment.
| @@ -10051,6 +10051,10 @@ void MethodTableBuilder::CheckForSystemTypes() | |||
| // The Procedure Call Standard for ARM 64-bit (with SVE support) defaults to | |||
| // 16-byte alignment for __m256. | |||
|
|
|||
| pLayout->m_ManagedLargestAlignmentRequirementOfAllMembers = 16; | |||
| #elif defined(TARGET_RISCV64) | |||
| // The alignment requirement for the fixed length vector shall be equivalent to | |||
There was a problem hiding this comment.
Same fix should be applied to comments in this file.
|
Thank you! |
Fixed the error caused by alignment mismatch for vector256 and vector 512.
This sets alignment to possible max alignment value for vector types reference on https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc
Part of #84834
cc @dotnet/samsung