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

Fix p4orch tests after SAI update #3337

Merged
merged 4 commits into from
Oct 22, 2024

Conversation

saiarcot895
Copy link
Contributor

What I did

Make sure that the p4orch tests are always run (not just when code coverage is enabled), and fix the p4orch tests following the SAI update.

Why I did it

sonic-net/sonic-sairedis#1431 updated the SAI submodule to the latest version, and #3321 fixed some of the compatibility issues, but there were still some that got through, because p4orch tests are enabled only when code coverage is enabled.

How I verified it

Details if related

@saiarcot895
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

2 test functions in acl_manager_test.cpp for p4orch are near 400 lines,
with macro expansion almost certainly adding a number of lines. On armhf
and arm64, GCC is complaining that it ran out of memory to do
optimizations here (specifically, keep track of const and copies).

```
error: const/copy propagation disabled: 9401 basic blocks and 114813 registers; increase '--param max-gcse-memory' above 134923152 [-Werror=disabled-optimization]
```

For now, turn the error into a warning for these two functions. Ideally,
these two test cases should be split up.

Signed-off-by: Saikrishna Arcot <[email protected]>
prsunny
prsunny previously approved these changes Oct 22, 2024
Instead of manually compiling separate versions of p4orch_tests for
ASAN/TSAN/UBSAN, have it instead be controlled by the top-level
--asan-enabled configuration flag. This matches the behavior for all
other tests in this repo.

This fixes an issue where armhf TSAN is not available, and arm64 TSAN is
not supported (it results in an error on startup).

Signed-off-by: Saikrishna Arcot <[email protected]>
Copy link
Collaborator

@prsunny prsunny left a comment

Choose a reason for hiding this comment

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

lgtm, @mint570 , let us know if you have any concerns

@prsunny prsunny merged commit e71eb2d into sonic-net:master Oct 22, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants