Review HBIOS Utility Functions

[wwarthen]

But I noticed room for further improvement in HBIOS space, in unrelated code. If you look at UTIL.ASM it has several functions which dont appear to be in use in normal code ( e.g. DUMP_BUFFER: line 255 -310 ), I tested by commenting the code, and build works fine. I believe this is called in test code which is surrounded by #if statements.

I think if this sort of code is removed from production code it could be a reasonable saving. Further Suggest the routines themselves are moved to a seperate file e.g. debug.asm or trace.asm which can be included wrapped in an appropriate #if statement. @wwarthen If you agree with this you could raise a seperate issue.

I agree and will create an issue to track it.

Thanks, Wayne

Originally posted by @wwarthen in #579

[kiwisincebirth]

... or trace.asm which can be included wrapped in an appropriate #if statement.

Also suggest if possible the code blocks (e.g. DUMP_BUFFER) themselves be wrapped in conditional logic based on the same logic as the calling debug code itself. The issue I saw was the use of #if FALSE wrapping debug code, making this maybe a much bigger job, worthwhile of consistent approach for enabling debug code.

[wwarthen]

Yeah, it is a bit of a mess right now. I will try to come up with something more consistent.

Thanks, Wayne

[kiwisincebirth]

I did some more review of UTIL.asm (see minor changes in #582) - however it is hard to know if code (referencing util.asm) is actually a real reference (the are copies of the code in other places) or the code itself is used at all. (e.g. PIO_PS.asm makes references but cannot tell if this is Legacy backup code)

I also found some duplication e.g.

• WRITESTR and PRTSTR perform the same basic function
• BCD2BYTE,BYTE2BCD in util.asm seem to share commonality with BCD.asm which has similar capability.

In short I agree it is a bit of a mess right now - my only other suggestion (and then I will shut up), would be to partition into smaller more manageable .inc files. e.g. util_cio.inc, util_delay.inc, util_math.inc, util_bcd.inc etc. (UTIL.ASM is a bit of dumping ground), then at least the duplication might be easier to manage, and includes into other code will not bring in as large of an overhead.

My 2c.

[wwarthen]

Hi @kiwisincebirth,

I did some more review of UTIL.asm (see minor changes in #582) - however it is hard to know if code (referencing util.asm) is actually a real reference (the are copies of the code in other places) or the code itself is used at all. (e.g. PIO_PS.asm makes references but cannot tell if this is Legacy backup code)

Thank you. Yes, it is hard to ensure things are not being used. Since they are just commented out, we can resolve as needed.

I also found some duplication e.g.

• WRITESTR and PRTSTR perform the same basic function

Yeah, they have the same basic goal, but their semantics are different enough that it is hard to consolidate them. They use different register conventions and different levels of register preservation. Yes, someday it should be rationalized, but it requires very careful work to avoid subtle failures at the points of change.

• BCD2BYTE,BYTE2BCD in util.asm seem to share commonality with BCD.asm which has similar capability.

The routines in util.asm are single byte conversions. The routines in bcd.asm are 32-bit.

In short I agree it is a bit of a mess right now - my only other suggestion (and then I will shut up), would be to partition into smaller more manageable .inc files. e.g. util_cio.inc, util_delay.inc, util_math.inc, util_bcd.inc etc. (UTIL.ASM is a bit of dumping ground), then at least the duplication might be easier to manage, and includes into other code will not bring in as large of an overhead.

I agree. Just a matter of getting to it... 🙂

Thanks, Wayne

[kiwisincebirth]

Have a look at functions BIT*: in util.asm (4 functions) - from my searching they are only used in ANSI.ASM - moving them there could save some space - and is simple win.

[wwarthen]

I have been working on approaches to make the inclusion of utility code automatic as needed. The right way to do this is with a linker and a library of object files. Unfortunately, that is not something that TASM does (neither does uz80as).

I found a cute way to automatically include routines whenever the label is referenced. However, this only works in TASM and not uz80as. Sigh.

I am feeling a little frustrated with this currently. Sadly, we have really outgrown the toolset being used. I should switch, but it is hard to gear up for the massive work that would require.

Thanks, Wayne

[feilipu]

Within z88dk the z88dk-z80asm tool is designed to do what you need to do. It would be worth a look imho.

See tool guide here.

Library code is used extensively in z88dk to support multiple targets across multiple architectures (from 8080 to 24-bit ez80, and Rabbit too).

It is cross-platform (windows, mac, linux) and cross-architecture (arm, x86, apple, etc).

I think the only difference from TASM is the syntax for defining constants and space. A one time regex fix.

Plenty of help available on the z88dk forum too.

[wwarthen]

Thanks for the suggestion @feilipu. Indeed, the conversion may not be quite a bad as I thought, but I'm not convinced yet. I think what I am going to do is create a minimal RomWBW build (virtually no drivers) and start a background task of attempting the conversion. This will not happen fast.

Thanks, Wayne

[kiwisincebirth]

FYI : Another thing I noticed but forgot to mention - the routine WRITESTR increments a character position PRTIDXCNT on the current line. This is also found in code in DMAMON/UTIL.ASM - However only the INVNTDEV.ASM (device inventory I created) ever references this variable for its character column alignments.

Not sure if this matters, may be useful to know if any attempt to rationalise its capability with PRTSTR

[wwarthen]

FYI : Another thing I noticed but forgot to mention - the routine WRITESTR increments a character position PRTIDXCNT on the current line. This is also found in code in DMAMON/UTIL.ASM - However only the INVNTDEV.ASM (device inventory I created) ever references this variable for its character column alignments.

Not sure if this matters, may be useful to know if any attempt to rationalise its capability with PRTSTR

Yup, it was created specifically to support the column alignment requirements of PRTSUM. Since PRTSUM is now neatly isolated into invntdev.asm, PRTIDXCNT is not needed in HBIOS. Can be refactored once something is done about the utility functions in general.

[wwarthen]

Hi @feilipu,

Within z88dk the z88dk-z80asm tool is designed to do what you need to do. It would be worth a look imho.

I did spend a little time playing with z88dk-z80asm. I got stuck pretty quickly because it appears to have no ability to modify the value of constants. In other words, once you set an equate, it may not be modified later. RomWBW uses a cascading configuration system that requires the ability to override constants. It would be a fundamental change to RomWBW to accommodate this.

I realize some folks will say this is a fundamental flaw in RomWBW and I wouldn't disagree. There are many things like this in RomWBW that exist as a result of the current toolset it uses.

Thanks, Wayne

[feilipu]

RomWBW uses a cascading configuration system that requires the ability to override constants. It would be a fundamental change to RomWBW to accommodate this.

Understood.

Paulo is very receptive to improving and extending z88dk-z80asm, so I don't see this as a blocker. If you would like, l can open an enhancement issue, and you can specify what you need. At least it would be worth the discussion I feel.