Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arm64/Sve: Enable Sve only if vector length is 128 #104174

Merged
merged 4 commits into from
Jun 28, 2024
Merged

Arm64/Sve: Enable Sve only if vector length is 128 #104174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
952a3a6
Enable InstructionSet_Sve only if system VL == 128
kunalspathak Jun 28, 2024
6117546
Update src/coreclr/vm/codeman.cpp
kunalspathak Jun 28, 2024
3d8b63a
Update src/coreclr/vm/codeman.cpp
jkotas Jun 28, 2024
7cf376c
Remove hardcoding of 128 for windows
kunalspathak Jun 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/coreclr/vm/arm64/asmhelpers.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,7 @@

;; uint64_t GetSveLengthFromOS(void);
LEAF_ENTRY GetSveLengthFromOS
;; TODO-SVE: Remove the hardcoded value 128 and uncomment once CI machines are updated to use MASM 14.4 or later
;; rdvl x0, 1
mov x0, #128
rdvl x0, 1
ret lr
LEAF_END

Expand Down
6 changes: 4 additions & 2 deletions src/coreclr/vm/codeman.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1514,8 +1514,10 @@ void EEJitManager::SetCpuInfo()
uint32_t maxVectorTLength = (maxVectorTBitWidth / 8);
uint64_t sveLengthFromOS = GetSveLengthFromOS();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method to be adjusted to return 0 on Windows with this change as otherwise we could be in an unsupported state since it's currently hardcoded as:

;; uint64_t GetSveLengthFromOS(void);
    LEAF_ENTRY GetSveLengthFromOS
        ;; TODO-SVE: Remove the hardcoded value 128 and uncomment once CI machines are updated to use MASM 14.4 or later
        ;; rdvl    x0, 1
        mov     x0, #128
        ret     lr
    LEAF_END

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hhm, so this is the case where if windows is running on 256-bit machine, we shouldn't be returning 128-bits because that will falsely turn on SVE. Let me uncomment the right way to do it to see if helix machines got the assembler upgraded to understand rdvl.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright, so that seemed to be working now.


// Do not enable SVE when the user specified vector length is smaller than the one offered by underlying OS.
if ((maxVectorTLength >= sveLengthFromOS) || (maxVectorTBitWidth == 0))
// For now, enable SVE only when the system vector length is 128-bits
// TODO: https://github.com/dotnet/runtime/issues/101477
if (sveLengthFromOS == 128)
// if ((maxVectorTLength >= sveLengthFromOS) || (maxVectorTBitWidth == 0))
{
CPUCompileFlags.Set(InstructionSet_Sve);
}
Expand Down