the if branches seem to have been forgotten https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=openpower/isafunctions/double2single.mdwn;hb=5f111b1fb9285c3605d0062bb41b50ac7fea550c#l42
(In reply to Jacob Lifshay from comment #0) > the if branches seem to have been forgotten > > https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=openpower/ > isafunctions/double2single.mdwn; > hb=5f111b1fb9285c3605d0062bb41b50ac7fea550c#l42 no they haven't, look at it more closely. goto does not exist in the syntax (it was not a keyword in section 1.3(?). i therefore created an intermediate variable "mode" as a substitute.
(In reply to Luke Kenneth Casson Leighton from comment #1) > (In reply to Jacob Lifshay from comment #0) > > the if branches seem to have been forgotten > > > > https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=openpower/ > > isafunctions/double2single.mdwn; > > hb=5f111b1fb9285c3605d0062bb41b50ac7fea550c#l42 > > no they haven't, look at it more closely. goto does not > exist in the syntax (it was not a keyword in section 1.3(?). > > i therefore created an intermediate variable "mode" as a > substitute. yeah I guessed that. you missed my point: when `mode` is set to 'inf_operand', nothing later checks if `mode` is 'inf_operand' and changes `result`, so `result` is left set to 0, which is the wrong answer.
ok good catch. yes as i explained on irc this was done extremely quickly, no comprehensive unit tests at all, just barely enough to get lauri up and running as quickly as possible. the mp3 unit tests all worked perfectly so no further investigation was done. yes, a hell of a lot more unit tests are needed, at some point porting those of the ieee754fp library to work with ISACaller would probably do it. ain't duplicating 2^5 unit tests that's for sure.
I re-converted the code from the PowerISA v3.1B spec. it took me quite a bit more time than anticipated, since I had to improve the pseudocode parser to let me figure out where the code was it didn't like -- even then I had to resort to commenting everything in the pseudo-code and uncommenting stuff until it broke again. https://git.libre-soc.org/?p=openpower-isa.git;a=commit;h=d648bc9ed1fc7403e41011ed2d06ba1d08b0939a I also fixed line number counting, so the parser's parse errors should have the correct line numbers. I renamed DOUBLE2SINGLE to FRSP since that's the instruction name, and since I had to add the FPSCR argument and return it. I added a helper wrapper DOUBLE2SINGLE that just calls FRSP, so we don't have to migrate everything right now. I ran all the test_caller*.py files that mention DOUBLE2SINGLE, they all pass except for test_caller_svp64_fft.py, which fails in a way that afaict is unrelated (I also checked on commit 5f111b1fb9 before I made these changes). Luke, can you add some funding to this bug? test_caller_svp64_fft.py backtrace: File "src/openpower/decoder/isa/test_caller_svp64_fft.py", line 369, in test_sv_remap_fpmadds_fft_svstep_scalar_temp sim = self.run_tst_program(program, svstate=svstate, File "src/openpower/decoder/isa/test_caller_svp64_fft.py", line 769, in run_tst_program simulator = run_tst(prog, initial_regs, mem=initial_mem, File "/home/jacob/projects/libreriscv/openpower-isa/src/openpower/decoder/isa/test_runner.py", line 166, in run_tst sim.run() File "/home/jacob/projects/libreriscv/nmigen/nmigen/sim/core.py", line 177, in run while self.advance(): File "/home/jacob/projects/libreriscv/nmigen/nmigen/sim/core.py", line 168, in advance return self._engine.advance() File "/home/jacob/projects/libreriscv/nmigen/nmigen/sim/pysim.py", line 319, in advance self._step() File "/home/jacob/projects/libreriscv/nmigen/nmigen/sim/pysim.py", line 308, in _step process.run() File "/home/jacob/projects/libreriscv/nmigen/nmigen/sim/_pycoro.py", line 125, in run self.coroutine.throw(exn) File "/home/jacob/projects/libreriscv/nmigen/nmigen/sim/_pycoro.py", line 64, in run command = self.coroutine.send(response) File "/home/jacob/projects/libreriscv/nmigen/nmigen/sim/core.py", line 82, in wrapper yield from process() File "/home/jacob/projects/libreriscv/openpower-isa/src/openpower/decoder/isa/test_runner.py", line 154, in process yield from simulator.execute_one() File "/home/jacob/projects/libreriscv/openpower-isa/src/openpower/decoder/isa/caller.py", line 1044, in execute_one yield from self.call(opname) # execute the instruction File "/home/jacob/projects/libreriscv/openpower-isa/src/openpower/decoder/isa/caller.py", line 1447, in call nia_update = (yield from self.check_step_increment(results, rc_en, File "/home/jacob/projects/libreriscv/openpower-isa/src/openpower/decoder/isa/caller.py", line 1691, in check_step_increment return nia_update UnboundLocalError: local variable 'nia_update' referenced before assignment
(In reply to Jacob Lifshay from comment #4) > I re-converted the code from the PowerISA v3.1B spec. it took me quite a bit > more time than anticipated, since I had to improve the pseudocode parser to > let me figure out where the code was it didn't like -- even then I had to > resort to commenting everything in the pseudo-code and uncommenting stuff > until it broke again. yep that sounds like a normal debugging technique. good to see it occurred to you. > https://git.libre-soc.org/?p=openpower-isa.git;a=commit; > h=d648bc9ed1fc7403e41011ed2d06ba1d08b0939a > > I also fixed line number counting, so the parser's parse errors should have > the correct line numbers. ah nice > I renamed DOUBLE2SINGLE to FRSP since that's the instruction name, and since > I had to add the FPSCR argument and return it. ahh very nice. i would have done it as a global. in fact that may have to be done that way, as FPSCR should be expected to be in the namespace of ISACaller. > I added a helper wrapper DOUBLE2SINGLE that just calls FRSP, so we don't > have to migrate everything right now. briilliant > I ran all the test_caller*.py files that mention DOUBLE2SINGLE, they all > pass except for test_caller_svp64_fft.py, which fails in a way that afaict > is unrelated (I also checked on commit 5f111b1fb9 before I made these > changes). > > Luke, can you add some funding to this bug? unfortunately there isn't any (for this topic), which is one of the reasons i didn't raise it or tackle it. it might be possible to sneak in under another bugreport that *needs* this. which will need some searching. > caller.py", line 1691, in check_step_increment > return nia_update > UnboundLocalError: local variable 'nia_update' referenced before assignment i know what that is. part of splitting ISACaller into functions. bug #728. i know what to do.