| Summary: | sv.bc branching to incorrect address in Vertical-First mode | ||
|---|---|---|---|
| Product: | Libre-SOC's first SoC | Reporter: | Andrey Miroshnikov <andy.miroshnikov> |
| Component: | Source Code | Assignee: | Jacob Lifshay <programmerjake> |
| Status: | RESOLVED FIXED | ||
| Severity: | enhancement | CC: | libre-soc-bugs, lkcl, programmerjake |
| Priority: | --- | ||
| Version: | unspecified | ||
| Hardware: | PC | ||
| OS: | Windows | ||
| See Also: | https://bugs.libre-soc.org/show_bug.cgi?id=994 | ||
| 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: | ||
| Bug Depends on: | |||
| Bug Blocks: | 961 | ||
|
Description
Andrey Miroshnikov
2023-11-17 15:08:19 GMT
(In reply to Andrey Miroshnikov from comment #0) > Example test case created in a branch: > https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff; > h=5a174ba4c2d7c19733167933eee71a2761e38695 Note I committed this test on a separate branch, so I don't affect master in any way. (In reply to Andrey Miroshnikov from comment #1) > Note I committed this test on a separate branch, so I don't affect master in > any way. additions are leaf-nodes and have zero impact. think it through: how can the addition of a *new independent* unitvtest have any possible adverse affect on any other code? move it to master. https://libre-soc.org/openpower/isa/branch/ if AA then NIA <-iea EXTS(LI || 0b00) else NIA <-iea CIA + EXTS(LI || 0b00) if LK then LR <-iea CIA + 4 (In reply to Andrey Miroshnikov from comment #0) > While experimenting with sv.bc in Vertical-First, I discovered an issue > where a branch is not take (although the condition for the branch is met). > > Example test case created in a branch: > https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff; > h=5a174ba4c2d7c19733167933eee71a2761e38695 > > Code (With PC/NIA): > 00 "setvl 0, 0, %d, 1, 1, 1" % maxvl, # VL = MAXVL = 2, vf=1 > 04 "sv.cmpi *cr0, 1, *10, 0x10", # compare reg val with immediate > 0c "sv.bc 0, *2, 0x10", # jmp if CTR!=0 AND reg not equal to imm > 14 "svstep. 27, 1, 0", this will occur if the jump does NOT take place because the conditions (CTR!=0 AND reg not eq immed) are not met. you can check if the conditions are or are not met by adding *another* nop and increasing the jump from 0x10 to 0x14. if the address jumped to is 0x18 then your hypothesis is correct. if the address of the next ecexuted instruction is still 0x14 then the branch is *not taking place* and the simulator is simply moving to the next instruction. > BUT instead NIA is set to 0x14 (svstep., the next insn) probably because the conditions for *conditional* branching are not met. (In reply to Luke Kenneth Casson Leighton from comment #3) > this will occur if the jump does NOT take place because the > conditions (CTR!=0 AND reg not eq immed) are not met. I checked it in a debugger on similar assembly, the pseudocode *is* writing to NIA, but then that written value doesn't properly propagate to the fetch of the next instruction. (In reply to Jacob Lifshay from comment #4) > I checked it in a debugger on similar assembly, the pseudocode *is* writing > to NIA, but then that written value doesn't properly propagate to the fetch > of the next instruction. interesting. there's an update function that is supposed to handle that. wait... that's really obscure because NIA is definitely a variable that is taken care of somewhere around here https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/decoder/isa/caller.py;hb=HEAD#l2833 (In reply to Luke Kenneth Casson Leighton from comment #2) > move it to master. I rebased and pushed to master: https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=089e6d352ec57be4ab645d18ad9e95df3af0d365 (In reply to Jacob Lifshay from comment #6) > I rebased and pushed to master: > https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff; > h=089e6d352ec57be4ab645d18ad9e95df3af0d365 thx jacob. good find andrey. I attempted to fix sv.bc, all tests now pass (except the ones that failed before anyway). I pushed to the branch: https://git.libre-soc.org/?p=openpower-isa.git;a=shortlog;h=refs/heads/1210-attempt-to-fix-vf-sv.bc Luke, can you look at the change I made and see if it looks ok? Andrey, please test the attempted fix. https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=d9544764b1710f3807a9c0685d150a665f70b9a2 commit d9544764b1710f3807a9c0685d150a665f70b9a2 Author: Jacob Lifshay <programmerjake@gmail.com> Date: Mon Nov 20 17:45:23 2023 -0800 fix vertical-first sv.bc https://bugs.libre-soc.org/show_bug.cgi?id=1210 (In reply to Jacob Lifshay from comment #8) > I attempted to fix sv.bc, all tests now pass (except the ones that failed > before anyway). I pushed to the branch: > https://git.libre-soc.org/?p=openpower-isa.git;a=shortlog;h=refs/heads/1210- > attempt-to-fix-vf-sv.bc awesome. > Luke, can you look at the change I made and see if it looks ok? yyyeah it looks like the same "guess" i was considering :) i don't think anything more sophisticated is needed, at this point: if anything else shows up (with extra tests) it can be dealt with. nicely done. (In reply to Jacob Lifshay from comment #8) > I attempted to fix sv.bc, all tests now pass (except the ones that failed > before anyway). I pushed to the branch: > https://git.libre-soc.org/?p=openpower-isa.git;a=shortlog;h=refs/heads/1210- > attempt-to-fix-vf-sv.bc Thanks for doing that Jacob. > > Andrey, please test the attempted fix. Can confirm the test is now passing (the branch is taken correctly). pushed to master fantastic work everybody, that was quite important to have working. |