Skip to content

Conversation

@kunalspathak
Copy link
Contributor

@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.

@ghost ghost 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
Contributor Author

@dotnet/jit-contrib @jakobbotsch

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

Diffs:

image
image

I will double check the reason for arm regression.

@kunalspathak
Copy link
Contributor 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
Contributor 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
Contributor Author

/ba-g failure is related to #102479

@kunalspathak kunalspathak merged commit 761c9a5 into dotnet:main May 22, 2024
@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
Contributor 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
Contributor 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