[Libre-soc-dev] mulhw / mulhwu in microwatt updates SO

[Luke Kenneth Casson Leighton]

On Friday, August 28, 2020, Paul Mackerras <paulus at ozlabs.org> wrote:

> On Thu, Aug 27, 2020 at 04:39:21PM +0100, Luke Kenneth Casson Leighton
> wrote:
> > in section v3.0B p73 it says that mulhw, mulhw. and hwu do not have an
> > overflow version.  CR0 is also intended to be undefined in bits 0:2 in
> > 64-bit mode.  nothing is mentioned about SO however we might
> > reasonably assume that because there is no OE version of mulhw/mulhwu,
> > it must be ignored.
>
> What's the "it" in that last sentence?


OE, sorry.


>
> The ISA shows bit 11 (IBM bit 21) of mulhw and mulhwu as a reserved
> field, whereas that bit is the OE bit for mullw and other
> instructions.


yes.


>  So according to ISA Book 1 section 1.3.3, which says
> "Reserved fields in instructions are ignored by the processor", we
> should ignore the OE field for mulhw/mulhwu, and we do (and that is in
> fact what POWER9 does also).


ah.  ok.  that's the thing: if my reading is correct, microwatt doesn't
ignore it, i checked the source, there's no special-case to ignore OE for
OP_MULH32, and the decode1.vhdl tree, by having case patterns that run
OP_MULH32 regardless of how OE is set, leaves an execution path where
decode2.vhdl (decode_oe) and execute1.vhdl follows the standard generic
behaviour for OE.

i _did_ have a special case in the libresoc equivalent of decode2.vhdl to
ignore OE (set it to zero for OP_MULH32 regardless of whether the bit is
set) however this was what got the "wrong" behaviour when it came to
creating CR0.

if you examine running core_tb on 1.bin with the patch i sent you can see
that CR is written as if OE is respected (i.e. as if there was a mulhwo)
and that XER is also updated as if mulhwo was being run.

later i will find the relevant sections of the logs (i am not at my laptop
at the moment)

i assumed this was intentional where from your response i see that this may
instead be a possible bug in microwatt.


> > $ powerpc64le-linux-gnu-objdump -D  -b binary -m powerpc:common64 -z
> > /home/lkcl/src/libresoc/microwatt/tests/1.bin  > 1.dump
>
> Yes, objdump refuses to interpret instructions with non-zero values in
> reserved fields.


ahh... this makes sense in the context of the program being randomly
generated.  i did wonder.


>
> > manual decode of 0x7fc12c97:
> >
> > major 31
> > minor 75
> > Oe 1
> > rt 30
> > ra 1
> > rb 5
> > Rc 1
> >
> > it's "mulhw." but actually it's not, it's "mulhwo." because both OE=1
> > *and* Rc=1 which is an instruction that doesn't exist in v3.0B.  (so
> > why is it in microwatt unit test 1.bin, and where did that unit test
> > come from?)
>
> I believe it was produced by Anton's simple_random program.


ahh a very good way to explore odd cases, i like it.


>
> > put simply: if we "obey" the spec, XER gets corrupted.  microwatt is
>
> How does XER get corrupted?  mulhw/mulhwu with bit 11 set to 1 should
> behave the same as with bit 11 set to 0, i.e., it won't update XER[SO]
> or XER[OV] -- and that is how both microwatt and POWER9 behave, and it
> is in conformance with Book I section 1.3.3.


should, but my excruciatingly close line by line examination trying to get
dead accurate identical behaviour would suggest otherwise.

(if it wasn't for the comparative analysis i would never have noticed).

i will double-check.


> > also not "obeying" the spec, and this is cause for some concern /
> > alarm, because, strictly speaking, we are violating the OpenPOWER EULA
> > by not following - to the letter - the v3.0B specification.
>
> The compliance requirements in the licence are considerably more
> relaxed for soft cores than for hard cores, fortunately, otherwise
> open-source development of a soft core wouldn't be possible.


wheww :)

and here it seems that the spec is correct.

we should confirm this by running 0x7fc12c97 on IBM POWER9 hardware (LE).




>
> > some... clarity on this would be greatly appreciated.  are we
> > permitted to violate the v3.0B spec (and the OpenPOWER EULA) on the
> > basis that if we don't we will end up fragmenting interoperability
> > because IBM's POWER9 implementation does something *different* from
> > what the v3.0B spec says?
>
> You haven't convinced me that POWER9 does something different from
> v3.0B yet...


 no, i see it may be a possible microwatt bug instead.

l.



-- 
---
crowd-funded eco-conscious hardware: https://www.crowdsupply.com/eoma68