Math Lib in Monitor: Add SHL32 and SHR32

[sy2002]

If one of the Math PhDs @bernd-ulmann and @MJoergen would like to do me a favor, then you could implement these SHL32 and SHR32 stubs in monitor/math_library.asm (and maybe even add a test program for it to the test_programs folder). I already added the SYSCALLs to the jump table in monitor/qmon.asm.

I probably will need these functions next "nerd Saturday" for "tile_ed" - so in case you did not do it until then, then I will do it.

;
;******************************************************************************
;*
;* MTH$SHL32 performs 32-bit shift-left with the same semantics as SHL:
;*           fills with X and shifts to C
;*           R8 = low word, R9 = high word, R10 = SHL amount
;*
;******************************************************************************
;
MTH$SHL32       HALT
;
;******************************************************************************
;*
;* MTH$SHR32 performs 32-bit shift-right with the same semantics as SHR:
;*           fills with C and shifts to X
;*           R8 = low word, R9 = high word, R10 = SHR amount
;*
;******************************************************************************
;
MTH$SHR32       HALT

[bernd-ulmann]

Let us brainstorm a moment:

• A loop would be inelegant and slow.

• Just a crude idea which needs to be refined a lot. Let's assume we want to do a left shift for R10 < 16 bit positions. Could this be done like this?

1. Shift R8 to the right (!) by 16 - R10 and apply an AND mask of the form 0b0...01...1 with 16 - R16 one bits on the right. Store this value (eg. in R0).
2. Now shift R8 to the left by R10 positions.
3. Shift R9 to the left by R10 positions
4. Apply the inverse AND mask from above, ie. 0b1...10...0 with 16 - R16 zero bits in the lower positions, to R9.
5. OR R0 and R9.

That does not work for shifts of >= 16 bit positions but might be a conversation starter. It could also be complete balderdash... ;-)

Good night. :-)

[sy2002]

As my approach would be the inelegant and slow loop, I am truly impressed by this idea, though I do not understand how it works. I could perfectly live with not being able to shift more than 16 bit positions at a time, because we could put this algorithm in a loop to make arbitrary bit positions > 16 possible 😈 @MJoergen, thoughts?

[MJoergen]

I like @bernd-ulmann 's idea very much. Actually, I have seen a similar algorithm used in VHDL :-)

I expect to implement it later today or tomorrow.

[MJoergen]

I've implemented both routines now.

The only problem is that I've (so far) changed the semantics of shr32 so that the X flag is undefined upon return. The reason is that it is very difficult to control the value of the X flag, because every instruction modifies that flag.

If any of you see a way to preserve/manipulate the X flag in the shr32 routine then please go ahead.

[MJoergen]

Ah, my last comment was not entirely correct.

The real problem is that the RET instruction itself modifies the X flag (and N and Z), because it really is just a MOVE instruction in disguise. So the only flags not modified by RET are C and V.

We could consider introducing a special RET instruction that does not modify any flags at all, much like we did with INCRB and DECRB ?

[MJoergen]

One more thing: I just realized the C library has 32-bit shifts built-in. Look in e.g. c/qnice/vclib/machines/qnice/libsrc/_llsl.c.

[bernd-ulmann]

Hmmmm... Just a quick question - did we ever use the X-flag in any context except shift instructions? The reason for asking is that I (personally) would prefer changing the behaviour of X slightly instead of introducing a "real" RET instruction. Let us brainstorm for a moment... :-)

[MJoergen]

@bernd-ulmann : I'm not sure what you mean by "changing the behaviour of X slightly".

[bernd-ulmann]

Me neither... :-) That's what we might brainstorm about. We could eg. say that X is no longer implicitly set by any instruction other than shift instructions. The only way to set/reset it otherwise would be an OR/AND with SR. Just a possibility and maybe a stupid idea but if we never used the X bit until now we might very well think along this line, what do you think?

[MJoergen]

I don't know if the X bit has been used in other programs already. @sy2002 should be the one to answer that.

It is an "unusual" status flag (indicating a value = -1), meaning I have not seen anything like it in other systems/processors.

Yes, we do need to brainstorm on this!

[sy2002]

I never understood the use-case of the X flag outside SHR and SHL and therefore avoided using it in code that I wrote. Also, I did a quick grep -ir ", X" . in the folders test_programs and monitor. The only program that uses X a lot is Michael's cpu_test.asm 😄

Therefore, here is my suggestion (which is basically just based on what Michael and Bernd already wrote above):

1. We will change the ISA: The semantics of the X flag would be restricted as Bernd wrote here: Math Lib in Monitor: Add SHL32 and SHR32 #160 (comment)

2. I would de-couple this ISA change from the task of writing SHL32 and SHR32 and open a new issue, assign it to the three of us and collect and write down (in form of a checklist) all the TODOs there that need to happen for this change

Please indicate with "Thumbs Up", if you want to proceed like that, because then we do not need a call :-)

So this issue could be completed and closed as soon as Michael finished the SHL32 and SHR32 functions and wrote a test program in test_programs for them and the test program sucessfully passed.

---

@MJoergen Regarding your comment #160 (comment) about the SHR32/SHL32 implementation in the VBCC library: As soon as your monitor functions SHL32 and SHR32 are ready and tested: Wouldn't it make a ton of sense and replace the slow C code implementation in libsrc by your brand-new and super-fast monitor implementation? Would it make sense to open a separate issue for that? If you had questions of how-to, we would meet in a quick GoToMeeting call and I showed you.

[bernd-ulmann]

I just changed the emulator so that the X-bit is only read/changed by shift instructions and adapted the documentation accordingly. (In case we decide against this path we can just roll back these small changes. :-) )

[MJoergen]

I would instead suggest a different solution, which is to keep everything as it is, except for one small change:

• The MOVE instruction will not change X.

All the other instructions should keep their existing behaviour. The reason is that somehow I think it is useful to be able to test e.g. whether an OR instruction gives a result 0xFFFF. In other words, I would like to keep the possibility of writing:

OR   R0, R1
RBRA label,X

This is not super important to me, but more a nice-to-have.

---

@sy2002 : You wrote

as soon as Michael finished the SHL32 and SHR32 functions and wrote a test program in test_programs for them and the test program sucessfully passed

I'm not sure what is missing? The latest version on the develop branch has complete and working implementations and test programs for both operations. The only thing is that I changed the semantics of the SHR32 function call, so that the X flag is undefined after return. This is because a RET instruction currently destroys the X register, hence this whole discussion about an ISA change.

---

Regarding the C library, I've opened a new Issue #164 .

[sy2002]

@MJoergen About your X flag behavior sugestion: I like it - but Bernd as our master of the ISA should decide.

@MJoergen Sorry, I did not see your latest commit today, when I wrote that. Thank you for being so fast: This will allow me to continue with tile_ed.asm at my next Nerd Saturday :-) I suggest to close this one (and did it).

I am opening a new issue for the architectural change of the X flag.

[sy2002]

@MJoergen reopened this one due to this result in most recent emulator:

HALT instruction executed at address 801D.

[bernd-ulmann]

Dear Michael - to decide about the behaviour of the X-flag is not easy. I though about it for the last about 2 hours. I, personally, would opt to have the X bit only modified by explicit AND/OR with SR or due to a shift instruction.

I can fully understand your desire to have this flag modified by all other "normal" instructions except MOVE but to be honest, it does not feel right. It would a rather arbitrary special behaviour which applies only to MOVE. Having no instruction implicitly modify X except SHL and SHR is "cleaner" and I think, I could sleep better if we could decide to go down that route.

Did we ever use the X bit at all in our existing code? If not, then there was (at least until now) no real reason for any instruction (except SHx) to implicitly modify it, what do you think?

[MJoergen]

Ok. I don't have strong feelings either way, so I support @bernd-ulmann's suggestion.

[MJoergen]

There was a bug in the latest emulator, where the X flag was cleared after every instruction. That was why the test_shift.asm suddenly stopped working again. I've fixed the emulator now, and the test program works again,

Closing again :-)

[sy2002]

@MJoergen I did a retest of test_shift.asm and can confirm, that it works.

[sy2002]

@MJoergen As written in #160 (comment) I did run your test program in the emu and it worked. Will work with it at the week-end on hardware, too.

Question about the semantics: Is it fair to assume, that SHR32 fills with C and shifts to X? So same semantics as SHR? If yes, then I did it right (please double check my commit): I updated the documentation in math_library.asm and test_shift.asm where this was not yet mentioned and re-rendered the Monitor documentation PDF using monitor/create_documentation.pl (auto-generated from the comments of the Monitor's functions' source code).

[sy2002]

@MJoergen Stumbled over a piece of documentation that potentially needs update and therefore reopened this issue and assigned to you: Please check, if your SHL32/SHR32 are safe to be used inside ISRs (as by far not all SYSCALLs are safe) and update the list of safe math routines here, if they are:

https://github.com/sy2002/QNICE-FPGA/blob/develop/doc/best-practices.md#interrupt-service-routines-isrs

Having had a quick look at your code, I would assume: Yes, they are. But please you decide. Then close this issue again.