Open AFOliveira opened 3 weeks ago
Sorry for the ammount of pushes, but this is a more clean structure with fewer LoC, reutilizing the riscv-opcode submodule
FYI, the auto-inst YAML files were very useful finding some errors in the hand-written instruction encodings.
Most have been fixed in the pr/AFOliveira/21 branch.
The remaining issues fall into two categories:
Details:
Variable mismatch for auipc:
auto: {"name"=>"imm", "location"=>"31-12"}
arch: {"name"=>"imm", "location"=>"31-12", "left_shift"=>12}
Variable mismatch for beq:
auto: {"name"=>"imm", "location"=>"31|7|30-25|11-8"}
arch: {"name"=>"imm", "location"=>"31|7|30-25|11-8", "left_shift"=>1}
Variable mismatch for bge:
auto: {"name"=>"imm", "location"=>"31|7|30-25|11-8"}
arch: {"name"=>"imm", "location"=>"31|7|30-25|11-8", "left_shift"=>1}
Variable mismatch for bgeu:
auto: {"name"=>"imm", "location"=>"31|7|30-25|11-8"}
arch: {"name"=>"imm", "location"=>"31|7|30-25|11-8", "left_shift"=>1}
Variable mismatch for blt:
auto: {"name"=>"imm", "location"=>"31|7|30-25|11-8"}
arch: {"name"=>"imm", "location"=>"31|7|30-25|11-8", "left_shift"=>1}
Variable mismatch for bltu:
auto: {"name"=>"imm", "location"=>"31|7|30-25|11-8"}
arch: {"name"=>"imm", "location"=>"31|7|30-25|11-8", "left_shift"=>1}
Variable mismatch for bne:
auto: {"name"=>"imm", "location"=>"31|7|30-25|11-8"}
arch: {"name"=>"imm", "location"=>"31|7|30-25|11-8", "left_shift"=>1}
Variable mismatch for jal:
auto: {"name"=>"imm", "location"=>"31|19-12|20|30-21"}
arch: {"name"=>"imm", "location"=>"31|19-12|20|30-21", "left_shift"=>1, "sign_extend"=>true}
Encoding mismatch for lui
auto: -------------------------0110111
arch: -------------------------0110011
Variable mismatch for sd:
auto: {"name"=>"imm", "location"=>"31-25|11-7"}
arch: {"name"=>"imm", "location"=>"31-25|11-7", "sign_extend"=>true}
Encoding mismatch for sll
auto: 0000000----------001-----0110011
arch: 0000000----------001-----0010011
Encoding mismatch for slli: Auto does not split by XLEN, but arch does
Variable mismatch for slliw:
auto: {"name"=>"shamt", "location"=>"24-20"}
arch: {"name"=>"shamt", "location"=>"25-20"}
Encoding mismatch for srai: Auto does not split by XLEN, but arch does
Variable mismatch for sraiw:
auto: {"name"=>"shamt", "location"=>"24-20"}
arch: {"name"=>"shamt", "location"=>"25-20"}
Encoding mismatch for srli: Auto does not split by XLEN, but arch does
Variable mismatch for srliw:
auto: {"name"=>"shamt", "location"=>"24-20"}
arch: {"name"=>"shamt", "location"=>"25-20"}
Encoding mismatch for slli.uw
auto: 000010-----------001-----0011011
arch: 0000010----------001-----0011011
Encoding mismatch for rori: Auto does not split by XLEN, but arch does
Encoding mismatch for rori: Auto does not split by XLEN, but arch does
Encoding mismatch for bclri: Auto does not split by XLEN, but arch does
Encoding mismatch for bexti: Auto does not split by XLEN, but arch does
Encoding mismatch for binvi: Auto does not split by XLEN, but arch does
Encoding mismatch for bseti: Auto does not split by XLEN, but arch does
Encoding mismatch for csrrs
auto: -----------------010-----1110011
arch: -----------------010-----0010011
Encoding mismatch for rori: Auto does not split by XLEN, but arch does
Encoding mismatch for rori: Auto does not split by XLEN, but arch does
Encoding mismatch for rori: Auto does not split by XLEN, but arch does
Instructions with a full (no variables) encoding have a YAML format error. The match field starts with a single quote, but isn't terminated. For example:
This should be solved on the new branch with my most recent commit.
Differences in how auto-inst and the handwritten handle encodings of shifts between RV32 and RV64
This was already solved it was only a bug when I converted the files to this structure, I will fix it and upload it
@dhower-qc
Getting Back on this, my latest commit, added the possiblity to add left_shift encapsulation to variables. I only added the ones present on the extensions already done on the arch folder. There are ambiguous results and therefores I will also be looking at riscv-opcodes to see if there is any change there that can enable this,
Missing left_shift on the auto-inst YAML. I believe I actually found a way to do this due to instructions name, I will try and then commit it
I think almost all the issues are now adressed on the PR branch with the exception of "sign_extend"=>true , which can be solved in the same way of the left_shift if you also agree so. I would like to know if there are any other instructions that were already hand-written on arch, so that I could add to the left_shift, which as of now is still pretty incomplete.
I think almost all the issues are now adressed on the PR branch with the exception of "sign_extend"=>true , which can be solved in the same way of the left_shift if you also agree so. I would like to know if there are any other instructions that were already hand-written on arch, so that I could add to the left_shift, which as of now is still pretty incomplete.
Great! Like we discussed, we can just forget about sign_extend; it isn't being used and I will just get rid of it in existing files.
@dhower-qc can I PR that separate branch or do you prefer to keep it on the side?
I'm still seeing three left shift discrepancies:
Variable mismatch for auipc:
auto: {"name"=>"imm", "location"=>"31-12"}
arch: {"name"=>"imm", "location"=>"31-12", "left_shift"=>12}
Variable mismatch for jal:
auto: {"name"=>"imm", "location"=>"31|19-12|20|30-21"}
arch: {"name"=>"imm", "location"=>"31|19-12|20|30-21", "left_shift"=>1}
Variable mismatch for lui:
auto: {"name"=>"imm", "location"=>"31-12"}
arch: {"name"=>"imm", "location"=>"31-12", "left_shift"=>12}
When there is a field that has a value restriction, e.g.:
c.addiw:
encoding:
match: ----------------001-----------01
variables:
- name: imm
location: 12|6-2
- name: rd/rs!=0
location: 11-7
riscv-unified-db is expecting a "not" field, like so:
c.addiw:
encoding:
match: ----------------001-----------01
variables:
- name: imm
location: 12|6-2
- name: rd_rs
location: 11-7
not: 0
Is that something we can do automatically?
I'm still seeing three left shift discrepancies:
Variable mismatch for auipc: auto: {"name"=>"imm", "location"=>"31-12"} arch: {"name"=>"imm", "location"=>"31-12", "left_shift"=>12} Variable mismatch for jal: auto: {"name"=>"imm", "location"=>"31|19-12|20|30-21"} arch: {"name"=>"imm", "location"=>"31|19-12|20|30-21", "left_shift"=>1} Variable mismatch for lui: auto: {"name"=>"imm", "location"=>"31-12"} arch: {"name"=>"imm", "location"=>"31-12", "left_shift"=>12}
This the case because there were only three left shifts in the .yaml and therefore I decided to wait for more instructions before properly handling all of them. The back-end is already done for them though.
When there is a field that has a value restriction, e.g.:
c.addiw: encoding: match: ----------------001-----------01 variables: - name: imm location: 12|6-2 - name: rd/rs!=0 location: 11-7
riscv-unified-db is expecting a "not" field, like so:
c.addiw: encoding: match: ----------------001-----------01 variables: - name: imm location: 12|6-2 - name: rd_rs location: 11-7 not: 0
Is that something we can do automatically?
Yes, it can. I forgot to mention it, my bad. What happened was that I could not find any examples on how this was being handled, but it can definitely be done automatically. I'll handle as soon as I can, thank you for pointing that out!
Is that something we can do automatically?
This is now done!
I believe that this type of file organization is a good workaround to not having everything in the upstream repo yet. This version still has some bugs namely on pseudoinstructions and on some assembly code. I will be working on those and I'll submit a new PR when they're solved.