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

clang floating point handling fix #19852

Merged
merged 2 commits into from
Aug 1, 2023
Merged

Conversation

MrKevinWeiss
Copy link
Contributor

Contribution description

As promised (with a delay for vacation), here is the nightlies fix. It is very easy...

I guess previously native had different handling of floating points (similar to cross-compiled gcc) when using clang. thanks to @maribu great work on fixing the llvm toolchain issues, that seems to not be an issue anymore (though I don't know the commit that fixed it).

Thus we remove clang conditions and enjoy a green CI.

Testing procedure

It seems like this wasn't tested or doesn't get tested in the normal murdock/bors procedure (still don't know why) but can be easily verified with a/b testing (master vs this PR)

TOOLCHAIN=llvm make all test -C tests/pkg/cayenne-lpp/

and

TOOLCHAIN=llvm make all test -C tests/pkg/lora-serialization/

I also tested with cross compiling on the samr21 and they all work...

Issues/PRs references

Look at nightlies...

It seems like the differences between float handling in clang vs gnu are resolved.
This condition now makes tests fail so it should be removed...
As in the previous commit, the floats are handled the same clang or not.
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Aug 1, 2023
Copy link
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

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

ACK

@dylad dylad added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: ready for merge train 🚃 PR is ready to be merged and awaiting the next merge train labels Aug 1, 2023
@riot-ci
Copy link

riot-ci commented Aug 1, 2023

Murdock results

✔️ PASSED

63f89b0 tests/pkg/cayenne-lpp: Remove clang condition

Success Failures Total Runtime
33 0 33 01m:23s

Artifacts

@MrKevinWeiss
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Aug 1, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 876a8b1 into RIOT-OS:master Aug 1, 2023
26 checks passed
@MrKevinWeiss MrKevinWeiss deleted the pr/clangfix branch August 1, 2023 12:24
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.10 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: ready for merge train 🚃 PR is ready to be merged and awaiting the next merge train
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants