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

llvm-mos divmod.cc ported from C++ to C incorrectly #7

Open
asiekierka opened this issue Sep 27, 2023 · 7 comments
Open

llvm-mos divmod.cc ported from C++ to C incorrectly #7

asiekierka opened this issue Sep 27, 2023 · 7 comments

Comments

@asiekierka
Copy link

asiekierka commented Sep 27, 2023

static __inlinefunc unsigned udiv(unsigned a, unsigned b)
{

This implementation of udiv (and umod, udivmod) will only work on numbers <= 16 bits. As the pi benchmark utilizes division of long (32 bit) numbers, this will not work correctly and lead to a result error.

Using divmod.cc and family directly from llvm-mos-sdk, as opposed to this patched version, allows the pi benchmark to give the correct result; as verified by compiling pi.c directly with llvm-mos-sdk (after patching _putc and _puts calls).

The same applies to mul.c, and potentially other files, I have not checked them all.

I'm getting similar results with merge-sort completing in reasonable time compiled directly with llvm-mos-sdk's standard library on the atari800 emulator, but failing when compiled through oricCompilerBenchmark's suite, but I haven't been able to track those down yet.

@iss000
Copy link
Owner

iss000 commented Sep 29, 2023

Noted. I'll check asap.

@johnwbyrd
Copy link

Poke.

@iss000
Copy link
Owner

iss000 commented Jul 19, 2024

Poke.

Thanks for the reminder :)
Now it's fixed and all tests work.

@iss000 iss000 closed this as completed Jul 19, 2024
@johnwbyrd
Copy link

Yes, and now all llvm-mos tests, including the ones that were working fine before, now take 3 times the previous amount of time and memory as your previous benchmark. This result is not sane.

@asiekierka
Copy link
Author

asiekierka commented Jul 20, 2024

613f95a#diff-70f4dc41f1a432a0131e9a2230954d6fad0063199aacd7aac423ec1d987d79afR15

LTO was commented out, which LLVM-MOS relies on strongly for its optimization strategies. A lot of the best tricks in the compiler are not possible without whole-program analysis, which is not available if link-time optimization is not being used.

@iss000
Copy link
Owner

iss000 commented Jul 20, 2024

Indeed. With '-flto' some tests fail. It needs more deep analyzes what exactly happens.

@johnwbyrd
Copy link

johnwbyrd commented Jul 21, 2024

Please reopen this issue, or create another corresponding issue.

@iss000 iss000 reopened this Jul 23, 2024
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

No branches or pull requests

3 participants