PDA

View Full Version : Keyboard buffering in HiTech C



JonB
July 27th, 2016, 08:17 AM
Hi all

Still working on the VI clone for CP/M, came across a problem.

I am porting various parts of it from C to pure assembler, and have noticed that the assembler version doesn't handle keyboard input (or screen output) very well when there are lots of things going on. It is almost as if a buffer was getting overrun, and characters are being dropped (either from the keyboard or screen i/o). This is a big problem, what with it being so slow on native hardware.

My question is about HiSoft C and its inplementation of putchar(), printf() and getchar(). Do they have some sort of interrupt driven buffering that prevents character dropout? If so, how can I prevent my assembly routines clashing with this?

Cheers
JonB

Randy McLaughlin
July 27th, 2016, 09:23 AM
Nothing about it is interrupt driven (unless you write it for a specific computer).

I would be surprised if there is any buffering at all.

CP/M is almost guaranteed to be non-interrupt driven and just polled.

You can send a character and it will not return until complete.

You can check if a character is ready and no wait above the time to check the port.

You can read a character and it will wait until a character is typed.

Unlike Unix/Linux CP/M usually never has interrupts enabled and other than N-key rollover implemented at the hardware level of a keyboard there is no keyboard/screen/printer buffer.


Randy

Chuck(G)
July 27th, 2016, 10:50 AM
Buffering in CP/M in some vendor BIOS implementations (I wrote one) does occur at the software level; however, most vendors didn't bother. An example of hardware buffering (well, okay, firmware buffering) is the IBM PC, where the keyboard controller has a FIFO queue of about 64 characters.

Absent buffering, your only choice is to include frequent keyboard checks during output. Indeed, some CP/M systems have no provisions for interrupt processing.

JonB
July 28th, 2016, 02:02 AM
Thank you both for your advice.. confirms what I thought about CP/M (after reading the source code I can't see anything interrupty going on, so if anything it'll be in the vendor BIOS code; certainly the P2000C uses interrupts, but far below the CP/M level).

So the conundrum here is that I have two versions of my editor and they both have the same input and output routines. In fact, the only difference is that the routine that inserts each character into the edit buffer is assembly in one version, and C in another. I am most perplexed, but I suspect that my embedded ASM code is not doing something that the C code does (something to do with the rest of the executable - functionally it is identical and works).

Any thoughts?

Chuck(G)
July 28th, 2016, 06:23 AM
Sure, got some code snippets to compare?

JonB
July 28th, 2016, 08:41 AM
Sure...



/* Insert a single character into the buffer */
/* ASM version */
inschar(c)
int c;
{
/* is there space? */
if(canincrease(1))
{

#asm

psect text
global _Curschar
global _Fileend

;Init
ld hl,(_Fileend) ;Fileend (last char in buff)
ld de,(_Curschar) ;Curschar (insert point)
or a ;clear carry flag
sbc hl,de ;calculate number of bytes to move
ld b,h
ld c,l ;copy result into bc

ld h,d ;set source to Curschar
ld l,e

inc de ;set dest to Curschar+1
ldir ; block copy

#endasm

*Curschar++ = c;
Fileend++;
CHANGED;
}
}

/* Insert a single character into the buffer */
/* Pure C version */
inschar(c)
int c;
{
register char *p;

/* Move everything in the file over to make */
/* room for the new char. */
if ( ! canincrease(1) )
return;

for ( p=Fileend; p>Curschar; p-- ) {
*p = *(p-1);
}

*Curschar++ = c;
Fileend++;
CHANGED;
}


As far as I can see (having not worked on it for some time), this is the only piece of code that is different.

Chuck(G)
July 28th, 2016, 10:44 AM
LDIR, moves from low-to-high addresses, incrementing each time. So your assembly code clobbers the buffer contents by essentially taking the first character at (hl) and propagating it to the end of the buffer. In more succinct terms,

(Buffer) contains c
LDIR on the first iteration moves c to (Buffer+1)
LDIR on the second iteration moves c from (Buffer+1) to (Buffer+2)....and so on.

Your C code does it right by starting at the buffer end and decrementing its way through addresses.

You should probably be using LDDR (recompute your pointers as well).

At least that's the way I read it.

JonB
July 28th, 2016, 10:52 AM
But it does work when run... I'll re-examine it tomorrow and re read LDIR info.

Might be overrunning or something.

Chuck(G)
July 28th, 2016, 12:56 PM
for ( p=Fileend; p>Curschar; p-- ) {
*p = *(p-1);

You're moving backwards in the buffer from the end. LDIR is moving forward from the current character. I'm also assuming that Curschar and Fileend are being treated by the compiler in accordance with K&R--that is, as unsigned values.

JonB
July 29th, 2016, 03:45 AM
Not sure they are unsigned. Maybe! But if this was a problem, the thing would fail spectacularly.

Meanwhile, this is the same code altered for LDDR. It does work - the only bug I found was not adding one to BC to copy the "zeroth" character.



psect text

global _Curschar
global _Fileend

;Init
ld hl,(_Fileend) ;Fileend (last char in buff)
ld de,(_Curschar) ;Curschar (insert point)
or a ;clear carry flag
sbc hl,de ;calculate number of bytes to move
ld b,h
ld c,l ;copy result into bc

inc bc ;add one for zeroth char
ld hl,(_Fileend) ;copy from end
ld d,h
ld e,l
inc de ;..to end+1 (shuffle up one)
lddr ; block copy decrement

#endasm


However, the buffering issue persists. It must be an artefact of the way the switch to ASM is handled in Hitech C... this is the only reason I can think of (and note, I'm not saving / restoring the registers before / after the call - not sure if it's needed in Hitech C).

Chuck(G)
July 29th, 2016, 07:49 AM
Okay, let's tackle that one now.

Most Cs, include Hitech, pass arguments and store local variables on the stack. For the Z80, most use IX and IY to access stack variables, but you could be running afoul of something--the Hitech manual is silent on register usage. Why not try saving all registers and then restoring them before and after your assembly code? Then start removing the PUSH and POPs until things quit working.

It could well be that the argument c is materialized into a register before your code is entered.

JonB
July 30th, 2016, 09:40 AM
Hi Chuck

I have seen the generated code for the function call, it is all IX & IY (which I'm not too familiar with) and stack ops.

However, by the time I am in the ASM section, the C function call logic has been executed, because my ASM is wrapped in the C function.

If the argument (c) was getting lost somewhere, nothing (or garbage) would be inserted into the buffer by the function, so I doubt this is the issue. Yet it works flawlessly, save for the loss of the keyboard buffer. I might also be experiencing crashes or data loss, but I see nothing. I begin to wonder if it is something else at play...

Chuck(G)
July 30th, 2016, 09:45 AM
The only way generic CP/M code is going to be able to poll the keyboard is via the CBIOS call to get keyboard status. Have you examined the C code to see where this might be occurring? I assume that if you're pushing AF, BC, DE and HL before your code and then popping them, that this doesn't change the non-behavior of your code. Or haven't you tried this yet?

JonB
July 30th, 2016, 09:52 AM
No, not yet. I guess that's my next port of call!

Meanwhile, I have been trying to emulate the effect on a z80pack setup in Linux. Running at 4mhz gives lots of weirdness. It's all caused by the editor's constant re-positioning of the cursor. It's really slow. And I get buffer drop too. Betting this is nothing to do with the editor, but then, how does the un-optimised version manage it?

Let me push & pop and try again (on the real machine, so will take a short while).

(edit: Wordstar to the rescue. Now compiling..)

JonB
July 30th, 2016, 11:06 AM
No, push/pop of the registers hl, bc, de made no difference.

There is something else going on. Somewhere!

Chuck(G)
July 30th, 2016, 12:35 PM
So, we know it isn't your code per se (I'll trust you on the verification of operation). I suppose that something is most likely clobbering memory somehow, but I'm not at all clear on how that could happen. It could also be that there's a position-dependent bug that your code jostles just enough.

On average, when you invoke this routine, how many bytes are you typically moving? Is this a typical "edit in the gap" type editor?

JonB
July 31st, 2016, 01:02 AM
It opens a single byte in a buffer prior to inserting a character, so the amount moved depends on the difference between the the current position (the insert point) and the current data (file) length. The buffer is allocated from contiguous memory and is always greater than or equal to the size of the data (file) being edited. That's what the canincrease() check does - ensures we do not attempt to insert into an already full buffer.

At this time I would like to say this code wasn't written by me (apart from the ASM optimisations). It's a copy of the "STEVIE" editor originally written for the Atari ST by Tim Thompson, and as such I am not responsible for any coding conventions the reader may not agree to (such as passing a char as an int in inschar()...). Interesting factoid: STEVIE is known as the grandfather of the vim editor and is extant on nearly every Linux implementation.

Chuck(G)
July 31st, 2016, 07:54 AM
Okay, let's tackle this problem the brute-force way.

Get assembly-code listings of the routine both ways and let's see how they differ. Most C compilers offer a "show me the code" option.

As noted before, it might not have anything to do with your code.

JonB
August 1st, 2016, 12:03 AM
OK, here is the Pure C version:


;MISCCMDS.C: 166: inschar(c)
;MISCCMDS.C: 167: int c;
;MISCCMDS.C: 168: {
_inschar:
call ncsv
defw f64
;MISCCMDS.C: 169: register char *p;
;MISCCMDS.C: 173: if ( ! canincrease(1) )
global _canincrease
ld hl,1
push hl
call _canincrease
exx
ld hl,2
add hl,sp
ld sp,hl
exx
ld l,l
ld h,h
ld a,l
or h
jp nz,l59
;MISCCMDS.C: 174: return;
jp l58
;MISCCMDS.C: 176: for ( p=Fileend; p>Curschar; p-- ) {
l59:
ld iy,(_Fileend)
jp l63
l60:
;MISCCMDS.C: 177: *p = *(p-1);
ld a,(iy+-1)
ld (iy+0),a
;MISCCMDS.C: 178: }
l62:
push iy
pop hl
dec hl
push hl
pop iy
l63:
push iy
pop de
ld hl,(_Curschar)
global wrelop
call wrelop
jp llt,l60
l61:
;MISCCMDS.C: 180: *Curschar++ = c;
ld a,(ix+6)
ld hl,(_Curschar)
inc hl
ld (_Curschar),hl
dec hl
ld (hl),a
;MISCCMDS.C: 181: Fileend++;
ld hl,(_Fileend)
inc hl
ld (_Fileend),hl
;MISCCMDS.C: 182: Changed=1;
ld a,.low.1
ld (_Changed),a
;MISCCMDS.C: 183: }
l58:
jp cret


..and this is the optimised version with embedded ASM:


;NTOSCR.C: 10: inschar(c)
;NTOSCR.C: 11: int c;
;NTOSCR.C: 12: {
psect text
global _inschar
_inschar:
global ncsv, cret, indir
call ncsv
defw f20
;NTOSCR.C: 14: if(canincrease(1))
global _canincrease
ld hl,1
push hl
call _canincrease
exx
ld hl,2
add hl,sp
ld sp,hl
exx
ld l,l
ld h,h
ld a,l
or h
jp z,l3
;NTOSCR.C: 15: {

psect text
;NTOSCR.C: 19: psect text
global _Curschar
;NTOSCR.C: 20: global _Curschar
global _Fileend
;NTOSCR.C: 21: global _Fileend

;Init
;NTOSCR.C: 23: ;Init
ld hl,(_Fileend) ;Fileend (last char in buff)
;NTOSCR.C: 24: ld hl,(_Fileend) ;Fileend (last char in buff)
ld de,(_Curschar) ;Curschar (insert point)
;NTOSCR.C: 25: ld de,(_Curschar) ;Curschar (insert point)
or a ;clear carry flag
;NTOSCR.C: 26: or a ;clear carry flag
sbc hl,de ;calculate number of bytes to move
;NTOSCR.C: 27: sbc hl,de ;calculate number of bytes to move
ld b,h
;NTOSCR.C: 28: ld b,h
ld c,l ;copy result into bc
;NTOSCR.C: 29: ld c,l ;copy result into bc

inc bc ;add one for zeroth char
;NTOSCR.C: 31: inc bc ;add one for zeroth char
ld hl,(_Fileend) ;copy from end
;NTOSCR.C: 32: ld hl,(_Fileend) ;copy from end
ld d,h
;NTOSCR.C: 33: ld d,h
ld e,l
;NTOSCR.C: 34: ld e,l
inc de ;..to end+1 (shuffle up one)
;NTOSCR.C: 35: inc de ;..to end+1 (shuffle up one)
lddr ; block copy decrement
;NTOSCR.C: 36: lddr ; block copy decrement

;NTOSCR.C: 40: *Curschar++ = c;
global _Curschar
ld a,(ix+6)
ld hl,(_Curschar)
inc hl
ld (_Curschar),hl
dec hl
ld (hl),a
;NTOSCR.C: 41: Fileend++;
global _Fileend
ld hl,(_Fileend)
inc hl
ld (_Fileend),hl
;NTOSCR.C: 42: Changed=1;
global _Changed
ld a,.low.1
ld (_Changed),a
;NTOSCR.C: 43: }
;NTOSCR.C: 44: }
l3:
l2:
jp cret


See what you make of them.. :)

JonB
August 1st, 2016, 07:36 AM
Oh, one more thing. I don't need to inc hl becasue it already points to the end of the file +1 (it's set as Fileend, which is the next free byte in the buffer after the actual data). So we get this:


#asm

psect text
global _Curschar
global _Fileend

;Init
ld hl,(_Fileend) ;Fileend (end of buff + 1)
ld de,(_Curschar) ;Curschar (insert point)
or a ;clear carry flag
sbc hl,de ;calculate number of bytes to move
ld b,h
ld c,l ;copy result into bc

ld hl,(_Fileend) ;copy from end
ld d,h
ld e,l
inc de ;..to end+1 (shuffle up one)
lddr ; block copy decrement

#endasm

Chuck(G)
August 1st, 2016, 09:17 AM
Other than the terrible generated code from C, I don't see a thing wrong, unless there's a call to a status check in the wrelop routine.

JonB
August 1st, 2016, 01:09 PM
Other than the terrible generated code from C

Yes, it does look awful!

GregK
September 12th, 2016, 12:33 AM
Try using

insloop:
ldd
jp pe,insloop
instead of
lddr

JonB
September 12th, 2016, 05:35 AM
By all means Greg, but why? Is it not functionally identical to lddr? And what if P/V is already set at the entry of the loop? And your code appears to be 5 T-states per iteration slower... according to z80 heaven, anyway.

:)

JonB
September 12th, 2016, 05:38 AM
Oh, one more thing. I don't need to inc hl becasue it already points to the end of the file +1 (it's set as Fileend, which is the next free byte in the buffer after the actual data). So we get this:


#asm

psect text
global _Curschar
global _Fileend

;Init
ld hl,(_Fileend) ;Fileend (end of buff + 1)
ld de,(_Curschar) ;Curschar (insert point)
or a ;clear carry flag
sbc hl,de ;calculate number of bytes to move
ld b,h
ld c,l ;copy result into bc

ld hl,(_Fileend) ;copy from end
ld d,h
ld e,l
inc de ;..to end+1 (shuffle up one)
lddr ; block copy decrement

#endasm


Actually, come to think of it, I need to check that BC is not zero before calling lddr or it will iterate all over the memory (not that that is actually happening, but it is good practice IMHO).

GregK
September 13th, 2016, 12:38 AM
By all means Greg, but why? Is it not functionally identical to lddr? And, what if P/V is already set at the entry of the loop?
lddr blocks interrupts for a relatively long time (especially if there's a lot of data in the buffer; and, you're editting near the beginning). That might be why you sometimes lose typed characters. Looping manually allows interrupts between each character. If you do have interrupt-driven key bufferring, then you will have a very responsive keyboard.
The block-copy instructions always change the P/V flag; they set it to 1 when BC is non-zero, and to 0 when BC is zero.


Actually, come to think of it, I need to check that BC is not zero before calling lddr; or, it will iterate over all the memory (not that that actually is happening; but, it is good practice, IMHO).BC will be zero when you "insert" at the end of the text.

Your pointer set-up looks to be backwards. Fileend already points to where you want to move the data; therefore, you should do it this way:
ld de,(_Fileend)
ld l,e
ld h,d
dec hl

JonB
September 13th, 2016, 12:00 PM
Your pointer set-up looks to be backwards. Fileend already points to where you want to move the data; therefore, you should do it this way:
ld de,(_Fileend)
ld l,e
ld h,d
dec hl

Good spot, dunno why I didn't see it... It is in there and working in the emulated CP/M box (under Linux). Now have added ldd loop as you suggest, but I am not sure the target machine (Philips P2000C) is really using interrupts. Maybe, to get stuff into a buffer from the serial port. The machine is implemented as a CPU, (internal) serial port and (internal) terminal, so the serial port may well be interrupt driven. Ah, it just finished compiling, let's see if you are right...

[long wait for compilation on the real hardware]

...nope, it doesn't work. It gets rather confused and drops characters.