"sv.addi/sw=8 *24,*8,0", => ERROR: test_sv_pospopcount (__main__.PosPopCountTestCase) positional popcount ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/lkcl/src/libresoc/openpower-isa/src/openpower/decoder/isa/test_caller_svp64_pospopcount.py", line 87, in test_sv_pospopcount sim = self.run_tst_program(program, initial_mem=initial_mem, File "/home/lkcl/src/libresoc/openpower-isa/src/openpower/decoder/isa/test_caller_svp64_pospopcount.py", line 109, in run_tst_program simulator = run_tst(prog, initial_regs, svstate=svstate, File "/home/lkcl/src/libresoc/openpower-isa/src/openpower/decoder/isa/test_runner.py", line 176, in run_tst sim.run() File "/home/lkcl/src/libresoc/nmigen/nmigen/sim/core.py", line 177, in run while self.advance(): File "/home/lkcl/src/libresoc/nmigen/nmigen/sim/core.py", line 168, in advance return self._engine.advance() File "/home/lkcl/src/libresoc/nmigen/nmigen/sim/pysim.py", line 319, in advance self._step() File "/home/lkcl/src/libresoc/nmigen/nmigen/sim/pysim.py", line 308, in _step process.run() File "/home/lkcl/src/libresoc/nmigen/nmigen/sim/_pycoro.py", line 125, in run self.coroutine.throw(exn) File "/home/lkcl/src/libresoc/nmigen/nmigen/sim/_pycoro.py", line 64, in run command = self.coroutine.send(response) File "/home/lkcl/src/libresoc/nmigen/nmigen/sim/core.py", line 82, in wrapper yield from process() File "/home/lkcl/src/libresoc/openpower-isa/src/openpower/decoder/isa/test_runner.py", line 164, in process yield from simulator.execute_one() File "/home/lkcl/src/libresoc/openpower-isa/src/openpower/decoder/isa/caller.py", line 1808, in execute_one yield from self.call(opname) # execute the instruction File "/home/lkcl/src/libresoc/openpower-isa/src/openpower/decoder/isa/caller.py", line 2269, in call results = info.func(self, *inputs) File "/home/lkcl/src/libresoc/openpower-isa/src/openpower/decoder/isa/caller.py", line 2962, in decorator result = func(*args, **kwargs) File "/home/lkcl/src/libresoc/openpower-isa/src/openpower/decoder/isa/generated/fixedarith.py", line 17, in op_addi RT = copy_assign_rhs(GPR.getz(_RA) + self.EXTS(SI)) NameError: name '_RA' is not defined
I'll have to work on this later, the power's out at my house.
from going through it with a debugger, it looks like _RA was just never implemented for svp64 instructions with elwid!=64: https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/decoder/isa/caller.py;h=96833c598b76aacbf9d8cacdbdc6a3ac1ba1928b;hb=2eb5abb845808c436797f3a5449f847fefdacca8#l2531 so, it looks like GPR.getz doesn't have the information necessary to pick the right register, so, luke, what do you think of changing (RA|0) to translate to GPR.getz(_RA, RA) so getz can just return the second input if the first is non-zero? we can then pass in the original register number (with the SVP64 extension bits) in _RA. This would *not* point to a sub-register (that can be added later if necessary). this seems like the simplest alternative.
(In reply to Jacob Lifshay from comment #2) > so, it looks like GPR.getz doesn't have the information necessary to pick > the right register, so, luke, what do you think of changing (RA|0) to > translate to GPR.getz(_RA, RA) so getz can just return the second input if > the first is non-zero? if the input *is* zero, then getz will return a SelectableInt(0, len(arg2))
(In reply to Jacob Lifshay from comment #2) > from going through it with a debugger, it looks like _RA was just never > implemented for svp64 instructions with elwid!=64: > https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/ > decoder/isa/caller.py;h=96833c598b76aacbf9d8cacdbdc6a3ac1ba1928b; > hb=2eb5abb845808c436797f3a5449f847fefdacca8#l2531 > > so, it looks like GPR.getz doesn't have the information necessary to pick > the right register, yep i remember now. elwidths were hack-started about 8-10 months ago. the context (offset, srcid, dstwd, xlen) aaaaalll need to be passed through to GPR.getz. manipulating the regfile directly is a pig, it really shouldn't be one. CR Field manipulation is going to be similar fun when extending ISACaller to 128 CR Fields (CR[BFA] etc) > so, luke, what do you think of changing (RA|0) to > translate to GPR.getz(_RA, RA) so getz can just return the second input if > the first is non-zero? it needs to be passed the full context... i like this hack though. it should solve the immediate problem, can you go ahead with it in the 672_pospopcount branch for me and add a v. quick unit test to src/openpower/test/alu/svp64_cases.py class SVP64ALUElwidthTestCase? https://git.libre-soc.org/?p=openpower-isa.git;a=shortlog;h=refs/heads/672_pospopcount > we can then pass in the original register number (with the SVP64 extension > bits) in _RA. This would *not* point to a sub-register (that can be added > later if necessary). an object that GPR.getz can interpret... it's going to need the exact same context/parameters/inputs as ISACaller.get_input(): https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=ade4c100a066e27dbbe1ccccf09fa2902bdefe0b this was always planned, i just never got round to it. > this seems like the simplest alternative. yes. if you've time (and power!) could you drop that in?
(In reply to Luke Kenneth Casson Leighton from comment #4) > yes. if you've time (and power!) could you drop that in? I decided to multiply the _RA value by 10000 since that preserves _RA == 0 but will likely give an error if we try to access any actual registers with _RA, since that would give incorrect results anyway. I added a FIXME so we can change that when we get a proper solution. done: https://git.libre-soc.org/?p=openpower-isa.git;a=commit;h=85332a97a87211fca375476912e7cfede5149681 commit 85332a97a87211fca375476912e7cfede5149681 Author: Jacob Lifshay <programmerjake@gmail.com> Date: Tue Nov 28 22:13:25 2023 -0800 ISACaller/parser: kludge: support (RA|0) when elwidth != 64 https://bugs.libre-soc.org/show_bug.cgi?id=1221 commit d031061f834df603910a030114240c366de124d9 Author: Jacob Lifshay <programmerjake@gmail.com> Date: Tue Nov 28 22:12:17 2023 -0800 isatables: update generated csvs
(In reply to Jacob Lifshay from comment #5) > (In reply to Luke Kenneth Casson Leighton from comment #4) > > yes. if you've time (and power!) could you drop that in? > > I decided to multiply the _RA value by 10000 since that preserves _RA == 0 > but will likely give an error if we try to access any actual registers with > _RA, since that would give incorrect results anyway. I added a FIXME so we > can change that when we get a proper solution. fantastic thank you so much. i will check it later, it will get pospopcount down by 1-2 instructions which is really good.