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

Reverting 'stat.c' to original. #99

Merged
merged 27 commits into from
Aug 21, 2024
Merged

Conversation

iss000
Copy link
Contributor

@iss000 iss000 commented Dec 22, 2023

Removing the 'ugly' hack. 'stat.c' and print() function are OK.
The same issue (with printing strings passed as param) occurs also in 'objdump.c'.
My guess is that virtual registers are messing up with (some) zp variables.
Here: printed lines are overlapped and random wrong strings are displayed (i.e. 'kl' instead 'kB'):
bad

The problem disappears when I comment out '-mlto-zp=32' line in 'mos-cpm65.cfg' file from the llvm-mos-sdk.
set

As result everything works fine:
good

@davidgiven
Copy link
Owner

davidgiven commented Dec 22, 2023

This looks suspiciously like a llvm-mos bug that got fixed a while back --- certain relocations where being made incorrectly which resulted in the zp static data being copied from the wrong place, so any short string which was being cached in zero page was getting corrupted. Like \r\n. I can't find a reference, unfortunately. Try upgrading llvm-mos and trying again?

FWIW, I've just tried it on the Oric in MAME and stat works fine on my system without modification:

image

@iss000
Copy link
Contributor Author

iss000 commented Dec 23, 2023

The issue is there - check closely the second column in your picture.
It's printed '2kri' instead of '2kB '.
Else I'm using always the last release of llvm-mos-sdk (now 9.0.1).
Screenshot_20231223_134843

@davidgiven
Copy link
Owner

Something's broken in llvm-mos --- asm.com doesn't work any more. It's not at my end; I tried redoing a previously successful build on github, and it now fails: https://github.com/davidgiven/cpm65/actions/runs/7532135164

I wonder if this is related?

@iss000
Copy link
Contributor Author

iss000 commented Feb 9, 2024

Something's broken in llvm-mos --- asm.com doesn't work any more. It's not at my end; I tried redoing a previously successful build on github, and it now fails: https://github.com/davidgiven/cpm65/actions/runs/7532135164

I wonder if this is related?

In my latest tests it seams that ASM.COM works.
The github auto-build reports an issue:
ALSA lib seq_hw.c:466:(snd_seq_hw_open) open /dev/snd/seq failed: No such file or directory
but I'm not sure if this is the real problem.
Else on my local make +mametest I found that there is (somehow) a stored old mame-session in
~/.mame/sta/oric1/ which is loaded during test. I removed it and now asm test.asm... works fine.
0000

EDIT: I've repeated the test with enabled option -mlto-zp=32 and indeed, mametest fails:
0001
and the log is:

MAMETEST src/arch/oricon+mametest
fail
GENPC   userdata        0000FC7C
CURPC   userdata        0000FC7C
CURFLAGS        userdata        N.....
PC      userdata        FC7C
A       userdata        DD
X       userdata        01
Y       userdata        04
P       userdata        B0
SP      userdata        01F1
IR      userdata        8D

BTW, what is the point to use -mlto-zp option in mos-cpm65.cfg?

@davidgiven
Copy link
Owner

I've just found a nasty LLVM bug: llvm-mos/llvm-mos-sdk#306 It was causing the start of the heap to not be relocated, meaning that if the binary was loaded above 0x0200 (which it usually is) then it would overwrite some of the program. This was causing the garbage out of ASM.COM in the mame tests. Unfortunately it'll take a while before the change reaches the release version of llvm-mos.

@iss000
Copy link
Contributor Author

iss000 commented Feb 17, 2024

Great news! I have some improvements for Oric and will open PR after I test all on real hardware.

@davidgiven
Copy link
Owner

That bug hasn't gone away. I finally found out what's going on this time: llvm-mos/llvm-mos-sdk#320

The good news is that your suggestion of turning off ZP LTO should work around this for now. I'll send them a PR.

@ivop
Copy link
Contributor

ivop commented Aug 21, 2024

Could this please be merged? Only LF is not how it's supposed to work.

@davidgiven
Copy link
Owner

Sorry, yes. Merging. This was left open because of the underlying linker issue. (It's not a problem with ZP LTO, but it's provoked by ZP LTO...)

@davidgiven davidgiven merged commit 5b88b0e into davidgiven:master Aug 21, 2024
1 check 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.

None yet

3 participants