Include support for Windows on Arm on BUILD.bazel along with proper Volterra detection #220
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@Maratyszcza Sorry to ping here, is there something I need to do to get this reviewed? Like creating a Issue or something? |
everton1984
force-pushed
the
bazel_woa
branch
from
f816abd
to
756899e
Compare
|
I no longer maintain this project, defer to @malfet |
everton1984
force-pushed
the
bazel_woa
branch
from
756899e
to
7691807
Compare
Include detection for Volterra, Windows Dev Kit.
|
@malfet Any chance you could take a look at this please? |
malfet
approved these changes
Mar 15, 2024
|
@everton1984 please fix clang-formatting, otherwise LGTM |
@malfet Done. Hopefully it worked. Thanks a lot for reviewing! |
This was referenced Apr 11, 2024
|
@malfet , @everton1984 ; First of all, thanks for the contribution. However, we are considering that this PR is causing an issue for detection Thus, we created an issue with the details and a possible solution with PR. |
malfet
pushed a commit
that referenced
this pull request
Apr 17, 2024
**Summary:** Resolves #236 Also related to [PR 220](#220) change. ``` "Unknown chip model name 'Ampere(R) Altra(R) Processor'. Please add new Windows on Arm SoC/chip support to arm/windows/init.c!" ``` --- **Previous error details:** The error's reason was: `woa_chip_name` (`windows-arm-init.h`) enum had only 4 elements (stored in `woa_chip_name_last`) ```c enum woa_chip_name { woa_chip_name_microsoft_sq_1 = 0, woa_chip_name_microsoft_sq_2 = 1, woa_chip_name_microsoft_sq_3 = 2, woa_chip_name_ampere_altra = 3, woa_chip_name_unknown = 4, woa_chip_name_last = woa_chip_name_unknown }; ``` However, `woa_chips[]` (`init.c`) has a duplicated value for `woa_chip_name_microsoft_sq_3` due to different strings for same target after the [PR 220](#220) > Strings are `Snapdragon (TM) 8cx Gen 3` and `Snapdragon Compute Platform` And this was causing following `for loop` (`init.c`) is not checking for all elements in `woa_chips[]`. ```c for (uint32_t i = 0; i < (uint32_t)woa_chip_name_last; i++) { size_t compare_length = wcsnlen(woa_chips[i].chip_name_string, CPUINFO_PACKAGE_NAME_MAX); int compare_result = wcsncmp(text_buffer, woa_chips[i].chip_name_string, compare_length); if (compare_result == 0) { chip_info = woa_chips + i; break; } } ``` --- **Fix Details:** We added `woa_chip_name_microsoft_sq_3_devkit` to maintain **one to one** relationship between `woa_chip_name` (`windows-arm-init.h`) and `woa_chips[]` (`init.c`). Also, we especially specified indexes with `enums` to prevent future duplications and increase readability of the code and relationship.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This MR includes support for building with Bazel on cpu
arm64_windows, I also tried this on my Volterra Windows Dev Kit and noticed that the core string seems different from what the current source code defines. I don't know if this is because my hardware is a bit different or not.I ran the tests with the following results
with
cpu-info.exereturningand
isa-info.exereturning