Bug 753 - simulator somehow ends up with SO set even though nothing writes to it
Summary: simulator somehow ends up with SO set even though nothing writes to it
Status: RESOLVED FIXED
Alias: None
Product: Libre-SOC's second ASIC
Classification: Unclassified
Component: source code (show other bugs)
Version: unspecified
Hardware: PC Linux
: --- major
Assignee: Luke Kenneth Casson Leighton
URL:
Depends on:
Blocks: 745
  Show dependency treegraph
 
Reported: 2021-12-09 06:12 GMT by Jacob Lifshay
Modified: 2021-12-10 22:56 GMT (History)
1 user (show)

See Also:
NLnet milestone: ---
total budget (EUR) for completion of task and all subtasks: 0
budget (EUR) for this task, excluding subtasks' budget: 0
parent task for budget allocation:
child tasks for budget allocation:
The table of payments (in EUR) for this task; TOML format:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jacob Lifshay 2021-12-09 06:12:44 GMT
running src/soc/fu/shift_rot/test/test_pipe_caller.py
at soc.git:
commit 031bebcc32d72f2000f13a316dae1f1cb4a8874d (HEAD -> master, origin/master, origin/HEAD)
Author: Jacob Lifshay <programmerjake@gmail.com>
Date:   Wed Dec 8 22:05:15 2021 -0800

    add bitmanip tests

openpower-isa.git:
commit 950e54f10be461122e30fde1b2957b342bb9b0ba (HEAD -> master, origin/master, origin/HEAD)
Author: Jacob Lifshay <programmerjake@gmail.com>
Date:   Wed Dec 8 22:04:10 2021 -0800

    make ternlogi tests run

Traceback (most recent call last):
  File "src/soc/fu/shift_rot/test/test_pipe_caller.py", line 157, in process
    yield from self.execute(alu, instruction, pdecode2, test)
  File "src/soc/fu/shift_rot/test/test_pipe_caller.py", line 126, in execute
    yield from self.check_alu_outputs(alu, pdecode2,
  File "src/soc/fu/shift_rot/test/test_pipe_caller.py", line 188, in check_alu_outputs
    ALUHelpers.check_cr_a(self, res, sim_o, "CR%d %s" % (cridx, code))
  File ".../openpower-isa/src/openpower/test/common.py", line 581, in check_cr_a
    dut.assertEqual(cr_expected, cr_actual, msg)
AssertionError: 9 != 8 : CR0 .4byte 0x14642ff9 # ternlogi. 3, 4, 5, 255
Comment 1 Luke Kenneth Casson Leighton 2021-12-09 10:27:08 GMT
  1 opcode,unit,internal op,in1,in2,in3,out,CR in,CR out,inv A,inv out,cry in,cry out,ldst len,BR,sgn ext,upd,rsrv,32b,sgn,rc,lk,sgl pipe,comment,form,CONDITIONS,unofficial,comment2
   2 --------00-,SHIFT_ROT,OP_TERNLOG,RA,RB,RT,RT,NONE,CR0,0,0,ZERO,0,NONE,0,0,0,0,0,0,RC,0,0,ternlogi,TLI,,1,unofficial until submitted and approved/renumbered by the opf isa wg

"RC=1" is a request to PowerDecoder2 to decode both OE and Rc flags,
which will then (sigh) decode those bits manually (a hangover from
using microwatt decode1.vhdl and decode2.vhdl)

try this:

   0,0,RC,0
   ==>
   0,0,NONE,0

this tends to suggest that disallowing Rc=1 for ternlog is probably a good
idea [and would free up more bits]

   A,RB,RT,RT,NONE,CR0,0,0,ZERO
   ==>
   A,RB,RT,RT,NONE,NONE,0,0,ZERO
Comment 2 Luke Kenneth Casson Leighton 2021-12-09 10:29:42 GMT
" even though nothing writes to it"

something *always* writes to something.  this could have been tracked
down by examining the debug log output, and noting that handle_overflow
in ISACaller is responsible for creating SO.


        # arithmetic overflow can be done by analysing the input and output
        elif len(inputs) >= 2:
            output = outputs[0]

            # OV (64-bit)
Comment 3 Jacob Lifshay 2021-12-10 20:01:42 GMT
(In reply to Luke Kenneth Casson Leighton from comment #2)
> " even though nothing writes to it"
> 
> something *always* writes to something.

I meant nothing in the pseudo-code.

>  this could have been tracked
> down by examining the debug log output,

well, the debug log has gotten so verbose/noisy as to be nearly useless from my perspective, I tend to ignore most of it.
Comment 4 Jacob Lifshay 2021-12-10 20:11:04 GMT
(In reply to Luke Kenneth Casson Leighton from comment #1)
>   1 opcode,unit,internal op,in1,in2,in3,out,CR in,CR out,inv A,inv out,cry
> in,cry out,ldst len,BR,sgn ext,upd,rsrv,32b,sgn,rc,lk,sgl
> pipe,comment,form,CONDITIONS,unofficial,comment2
>    2
> --------00-,SHIFT_ROT,OP_TERNLOG,RA,RB,RT,RT,NONE,CR0,0,0,ZERO,0,NONE,0,0,0,
> 0,0,0,RC,0,0,ternlogi,TLI,,1,unofficial until submitted and
> approved/renumbered by the opf isa wg
> 
> "RC=1" is a request to PowerDecoder2 to decode both OE and Rc flags,
> which will then (sigh) decode those bits manually (a hangover from
> using microwatt decode1.vhdl and decode2.vhdl)

Ok, when we have time, we should separate Rc decoding from overflow decoding in the CSVs.

> this tends to suggest that disallowing Rc=1 for ternlog is probably a good
> idea [and would free up more bits]

Yes, I'm also biased against ternlogi having Rc anyway, though not for any reason I can express at the moment, hence why I haven't mentioned it before...

I'll remove "ternlogi." and just have "ternlogi".
Comment 5 Jacob Lifshay 2021-12-10 20:38:25 GMT
Fixed in:
openpower-isa.git:
commit 3d9d4a0864e4e5d4ab6dc13348ec3289748285d6 (HEAD -> master, origin/master, origin/HEAD)
Author: Jacob Lifshay <programmerjake@gmail.com>
Date:   Fri Dec 10 12:34:23 2021 -0800

    change ternlogi to not have Rc field
Comment 6 Luke Kenneth Casson Leighton 2021-12-10 22:41:07 GMT
(In reply to Jacob Lifshay from comment #3)
> (In reply to Luke Kenneth Casson Leighton from comment #2)
> > " even though nothing writes to it"
> > 
> > something *always* writes to something.
> 
> I meant nothing in the pseudo-code.

yes.  and there never will be.  it is a high level specification.

> >  this could have been tracked
> > down by examining the debug log output,
> 
> well, the debug log has gotten so verbose/noisy as to be nearly useless from
> my perspective, I tend to ignore most of it.

now you know why it is there.  you ignored the output and were thus unable to debug the Simulator.  clearly there is a correlation there.
Comment 7 Luke Kenneth Casson Leighton 2021-12-10 22:56:20 GMT
(In reply to Jacob Lifshay from comment #4)

> Ok, when we have time, we should separate Rc decoding from overflow decoding
> in the CSVs.

they're separated (different columns), it's the heavy interdependence
(OE cannot exist without Rc due to XER.SO *also writing to CR0) that
is the problem, and that's basically unfixable: it's *by design* as
part of the ISA. annoying as that may be.

> > this tends to suggest that disallowing Rc=1 for ternlog is probably a good
> > idea [and would free up more bits]
> 
> Yes, I'm also biased against ternlogi having Rc anyway, though not for any
> reason I can express at the moment, hence why I haven't mentioned it
> before...

i think, arithmetically, it has no meaning. CR0.EQ to zero, sure, but CR0.LT?
or CR0.SO? they make no sense, because this is a "Logical" operation, and there
are no Rc=1 Logical Power ISA operations (and then the bit is unused, sigh)

> I'll remove "ternlogi." and just have "ternlogi".

excellent.