-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Add AArch64 support to the premerge tests #155274
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
base: main
Are you sure you want to change the base?
Conversation
Did we get quota from AWS for this? |
Yes, we did. Right now, I'm just trying to get a prototype working. |
The test TestVariableAnnotationsDisassembler.py relies on an .s input file generated for x86_64 with assembly directives that are not portable (e.g., DWARF sections and comments using #). This causes build failures on non-x86 targets such as AArch64. This change skips the test unless the architecture is x86_64, ensuring it only runs where the assembly input is valid.
✅ With the latest revision this PR passed the Python code formatter. |
There is still one failing test, but the implementation is complete (including sccache setup). |
Due to a fallback in GDBRemoteCommunicationClient.cpp, on Linux we will assume a PID of 1 if the remote does not respond in some way that tells us the real PID. So if PID 1 happened to be a process that the current user could read, we would try to debug that instead. On my current machine, PID 1 was sshd run by root so we would ignore it. If I changed the fallback to some process ID run by my user, the test would fail. To prevent this, return something to LLDB that tells it there is no remote PID, preventing the host lookup. Fixes llvm#155895
This is ready for review. I set the continue-on-error property for the aarch64 tests for now to allow for a stabilization period that doesn't impact users. |
Also, because I had to change the name of the tests to include the arch, we will need to update the rulset which requires the precommit tests to pass. |
) | ||
args = parser.parse_args() | ||
|
||
logo = ":window:" if platform.system() == "Windows" else ":penguin:" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: It might be slightly cleaner to refactor this into a compute_platform_title
function.
# why we do not hardcode it. | ||
export SCCACHE_GCS_BUCKET=$CACHE_GCS_BUCKET | ||
export SCCACHE_GCS_RW_MODE=READ_WRITE | ||
if [[ "${{ matrix.runs-on }}" = "llvm-premerge-linux-runners" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make a note that this is conditional due to the different caching setup between GCP and AWS/depot?
No description provided.