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

Suppress pass-failed warnings/errors #2404

Merged
merged 1 commit into from
Mar 25, 2024
Merged

Conversation

SoniEx2
Copy link
Contributor

@SoniEx2 SoniEx2 commented Mar 25, 2024

Fixes #2403

simde obviously requests loop vectorization, but we shouldn't fail for missed vectorizations as we're more focused on testing for correctness.

@sbc100
Copy link
Member

sbc100 commented Mar 25, 2024

Can you give an example of the failure messages you are seeing?

Why would llvm be generating warning about loop vectorization failure unless we are explicitly asking to run that pass? (are we doing that?)

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Mar 25, 2024

see also simd-everywhere/simde#1151

@sbc100
Copy link
Member

sbc100 commented Mar 25, 2024

But if we aren't seeing these warning when we build/run our tests why suppress the warning here? If test/run-spec-wasm2c.py is generating warning why don't we see them in our CI?

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Mar 25, 2024

because our clang is too old (it's using clang 14)

(this is because we're stuck on ubuntu 22.04 until the next LTS comes out)

@sbc100
Copy link
Member

sbc100 commented Mar 25, 2024

because our clang is too old (it's using clang 14)

You mean the clang we use in the wabt CI?

(this is because we're stuck on ubuntu 22.04 until the next LTS comes out)

In that case I think we should combine this change some amount of CI test for recent versions of clang.

Do you know which version of clang is needed in order to reproduce this warning?

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Mar 25, 2024

the macos-14 runner (apple silicon, which we aren't using) reproduces the warning, but it also fails due to the other thing. (unimplemented simd read intrinsics or something?)

we could also wait a few months for CI to start failing.

we know it definitely reproduces on clang 17

@sbc100
Copy link
Member

sbc100 commented Mar 25, 2024

This sounds like maybe a bug in clang, no? Why would it tell the user about some internal pass failure, unless the user somehow opts into that pass specifically... are we somehow doing that in the generated code?

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Mar 25, 2024

are we somehow doing that in the generated code?

Yes. simde expands pragma clang loop vectorize(enable) which requests that pass specifically:

https://github.com/simd-everywhere/simde/blob/cf68aaf07105a71db9aa7dc076e9b75a701bf58c/simde/simde-common.h#L379

@sbc100
Copy link
Member

sbc100 commented Mar 25, 2024

are we somehow doing that in the generated code?

Yes. simde expands pragma clang loop vectorize(enable) which requests that pass specifically:

https://github.com/simd-everywhere/simde/blob/cf68aaf07105a71db9aa7dc076e9b75a701bf58c/simde/simde-common.h#L379

Thanks for context. I think that was what I was missing.

How about adding a comment here such as: # simde explicitly enables xxx via pragma yyy, which means clang can generate warnings when this pass fails.

@sbc100 sbc100 merged commit ff03168 into WebAssembly:main Mar 25, 2024
17 checks passed
@SoniEx2 SoniEx2 deleted the no-pass-failed branch March 26, 2024 12:12
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.

Error while running testsuite (simd_lane, simd_load) "loop not vectorized"
2 participants