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.

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

Handle more than 64 registers - Part 2 #102297

Merged
merged 8 commits into from
May 22, 2024

Conversation

kunalspathak
Copy link
Member

@kunalspathak kunalspathak commented May 16, 2024

In this PR, I have extended the support of struct regMaskTP to non-arm64 platforms. The reason being that when I start adding code to convert 64-bits mask back and forth between regMaskTP, I will not have to add #ifdef TARGET_ARM64 in LSRA code to handle the struct regMaskTP vs. uint64_t regMaskTP separately.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 16, 2024
@kunalspathak kunalspathak marked this pull request as ready for review May 17, 2024 12:50
@kunalspathak
Copy link
Member Author

@dotnet/jit-contrib @jakobbotsch

@kunalspathak kunalspathak requested a review from jakobbotsch May 17, 2024 12:50
@kunalspathak
Copy link
Member Author

Diffs:

image
image

I will double check the reason for arm regression.

@kunalspathak
Copy link
Member Author

I will double check the reason for arm regression.

For some reason, I am not able to use the local pin tool to produce the individual method instruction count difference. I looked at vtune data for benchmarks.run_tiered and I don't see anything standing out. I am guessing the conversion of uint64 to struct might stop certain native optimizations.

image

@kunalspathak
Copy link
Member Author

ping @dotnet/jit-contrib

@JulieLeeMSFT JulieLeeMSFT added the Priority:2 Work that is important, but not critical for the release label May 20, 2024
Copy link
Member

@jakobbotsch jakobbotsch left a comment

Choose a reason for hiding this comment

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

LGTM, some minor clean up that we should do in the future.

@kunalspathak
Copy link
Member Author

/ba-g failure is related to #102479

@kunalspathak kunalspathak merged commit 761c9a5 into dotnet:main May 22, 2024
106 of 114 checks passed
@kunalspathak kunalspathak deleted the regMaskTP-nonarm64 branch May 22, 2024 00:01
@am11
Copy link
Member

am11 commented May 23, 2024

@kunalspathak, osx-arm64 build is failing locally. If I revert this change, or checkout the previous commit 0709995, it works. Possible build regression.

[ 96%] Building CXX object jit/CMakeFiles/clrjit_unix_x64_arm64.dir/simdashwintrinsic.cpp.o
Undefined symbols for architecture arm64:
  "RegSet::rsAllCalleeSavedMask", referenced from:
      RegSet::rsGetModifiedCalleeSavedRegsMask() const in codegencommon.cpp.o
  "RegSet::rsIntCalleeSavedMask", referenced from:
      RegSet::rsGetModifiedOsrIntCalleeSavedRegsMask() const in codegenxarch.cpp.o
      RegSet::rsGetModifiedIntCalleeSavedRegsMask() const in codegenxarch.cpp.o
ld: symbol(s) not found for architecture arm64
[ 96%] Building CXX object jit/CMakeFiles/clrjit_universal_arm_arm64.dir/targetarm.cpp.o
[ 96%] Building CXX object jit/CMakeFiles/clrjit_unix_x64_arm64.dir/simdcodegenxarch.cpp.o
[ 96%] Building CXX object jit/CMakeFiles/clrjit.dir/codegenarm64test.cpp.o
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [jit/libclrjit_win_x64_arm64.dylib] Error 1
make[1]: *** [jit/CMakeFiles/clrjit_win_x64_arm64.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
[ 96%] Building CXX object jit/CMakeFiles/clrjit_win_x86_arm64.dir/targetx86.cpp.o
[ 96%] Building CXX object jit/CMakeFiles/clrjit.dir/emitarm64.cpp.o
[ 96%] Building CXX object jit/CMakeFiles/clrjit_win_x86_arm64.dir/unwindx86.cpp.o
[ 96%] Building CXX object jit/CMakeFiles/clrjit_universal_arm_arm64.dir/unwindarmarch.cpp.o
[ 96%] Building CXX object jit/CMakeFiles/clrjit_universal_arm_arm64.dir/dllmain.cpp.o
[ 96%] Building CXX object jit/CMakeFiles/clrjit_win_x86_arm64.dir/hwintrinsicxarch.cpp.o
[ 96%] Building CXX object jit/CMakeFiles/clrjit.dir/emitarm64sve.cpp.o
[ 96%] Building CXX object jit/CMakeFiles/clrjit_unix_x64_arm64.dir/targetamd64.cpp.o
[ 96%] Building CXX object jit/CMakeFiles/clrjit_unix_x64_arm64.dir/unwindamd64.cpp.o
[ 97%] Building CXX object jit/CMakeFiles/clrjit_win_x86_arm64.dir/hwintrinsiccodegenxarch.cpp.o
[ 97%] Building CXX object jit/CMakeFiles/clrjit.dir/lowerarmarch.cpp.o
^[[C[ 97%] Linking CXX shared library libclrjit_universal_arm_arm64.dylib
[ 97%] Building CXX object jit/CMakeFiles/clrjit.dir/lsraarmarch.cpp.o
[ 97%] Building CXX object jit/CMakeFiles/clrjit_unix_x64_arm64.dir/hwintrinsicxarch.cpp.o
[ 97%] Building CXX object jit/CMakeFiles/clrjit_unix_x64_arm64.dir/hwintrinsiccodegenxarch.cpp.o
Undefined symbols for architecture arm64:
  "RegSet::rsAllCalleeSavedMask", referenced from:
      RegSet::rsGetModifiedCalleeSavedRegsMask() const in codegencommon.cpp.o
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [jit/libclrjit_universal_arm_arm64.dylib] Error 1
make[1]: *** [jit/CMakeFiles/clrjit_universal_arm_arm64.dir/all] Error 2
[ 97%] Building CXX object jit/CMakeFiles/clrjit_win_x86_arm64.dir/dllmain.cpp.o
[ 97%] Building CXX object jit/CMakeFiles/clrjit.dir/lsraarm64.cpp.o
[ 97%] Building CXX object jit/CMakeFiles/clrjit_unix_x64_arm64.dir/dllmain.cpp.o
[ 97%] Linking CXX shared library libclrjit_win_x86_arm64.dylib
[ 97%] Building CXX object jit/CMakeFiles/clrjit.dir/simd.cpp.o
[ 97%] Building CXX object jit/CMakeFiles/clrjit.dir/simdashwintrinsic.cpp.o
[ 97%] Building CXX object jit/CMakeFiles/clrjit.dir/targetarm64.cpp.o
Undefined symbols for architecture arm64:
  "RegSet::rsAllCalleeSavedMask", referenced from:
      RegSet::rsGetModifiedCalleeSavedRegsMask() const in codegencommon.cpp.o
  "RegSet::rsIntCalleeSavedMask", referenced from:
      RegSet::rsGetModifiedIntCalleeSavedRegsMask() const in codegenxarch.cpp.o
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [jit/libclrjit_win_x86_arm64.dylib] Error 1
make[1]: *** [jit/CMakeFiles/clrjit_win_x86_arm64.dir/all] Error 2
[ 97%] Building CXX object jit/CMakeFiles/clrjit.dir/unwindarmarch.cpp.o
[ 97%] Building CXX object jit/CMakeFiles/clrjit.dir/unwindarm64.cpp.o

@kunalspathak
Copy link
Member Author

@am11 - What machine are you building this on? can you share the command you are using and possibly the clang version?

@am11
Copy link
Member

am11 commented May 23, 2024

$ uname -ms
Darwin arm64

$ clang --version | head -1
Apple clang version 15.0.0 (clang-1500.3.9.4)

@kunalspathak
Copy link
Member Author

Thanks. I realized that this fails only if building Debug. Checked and Release should build fine.

steveharter pushed a commit to steveharter/runtime that referenced this pull request May 28, 2024
* Make regMaskTP struct for non-arm64 platforms

* some refactoring

* jit format

* fix missing paranethesis in arm

* fix riscv64 and loongarch build

* minor change

* review feedback
Ruihan-Yin pushed a commit to Ruihan-Yin/runtime that referenced this pull request May 30, 2024
* Make regMaskTP struct for non-arm64 platforms

* some refactoring

* jit format

* fix missing paranethesis in arm

* fix riscv64 and loongarch build

* minor change

* review feedback
@github-actions github-actions bot locked and limited conversation to collaborators Jun 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI Priority:2 Work that is important, but not critical for the release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants