Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LLVM replaces Manual #4

Merged
merged 1 commit into from
Jul 20, 2021
Merged

LLVM replaces Manual #4

merged 1 commit into from
Jul 20, 2021

Conversation

Rot127
Copy link
Member

@Rot127 Rot127 commented Jul 10, 2021

Hi,

so this is basically a complete rewrite of the generation code. I used LLVM as the source of information and no longer the Programmers Reference Manual. The reason is mainly that the parsing had to be done for each Manual again. This was error prone and could not be tested easily (and there are a lot more reasons).

There are still some non functional things missing in this version (list below), but I won't have that much time in the next two weeks to work on it further, so I just wanted to put it out for you to review.

I am very happy about any remarks, requests or improvement ideas!

Also I really hope the code is easy to understand, maintainable and extendable. If you have any remarks in this regard, please let me know as well!

Features comparison

Old New Notes
v5 - v62 instr set LLVM instr. set (v5-v68 + HVX) n-o-o-n added not public instruction to its plugin. We do not have those.
Incorrect sub register parsing Has been fixed
Only GPRs in register profile All registers in register profile + argument and return register
Hard coded calling convention (arg regs/return regs) CC derived from LLVM CC for HVX is not added yet. Rizin can not handle more than 10 arg registers.
Hard to update (needs parsing code for each Manual ver.) Easy to update (as long as LLVM does not change too much.)
Does not distinguish between different Hexagon versions. same This could be problematic because after v66 vector registers were 1024bit wide instead of 512bit. This plugin sets the sizeof vector registers to 1024bit.
No difference between 32bit immediate values and non 32bit values in the syntax. 32bit prefixed with ##. Non 32bit with #
Neg. immediate values printed as unsigned int Signed immediate values! ##0xffffffff -> ##-0x1 - My favorite :D
None Hardware loop support
None Instr. packet highlighting

TODO

  • HVX register (vector registers) have no special type in the register protocol yet. They are treated as GPRs at the moment. See: Add vector and control register type rizin#1287
  • Double registers are listed as independent registers in the register profile. This should be changed.
  • Some asm tests fail on purpose (because of the previous points).
  • Rizin analysis tests missing -> Tested only by manually checking it in rizin (which works fine).
  • No UTF-8 free assembly code at the moment (Instr. packets are prefixed with Unicode chars). -> UTF-8 chars are removed.
  • Missing Control double registers (C21:20 - C29:28) encodings. This is a problem in the LLVM source. Probably have to create a pull request there.
  • HVX instructions and register types are not tested yet (didn't get the IDE to compile HVX code in a reasonable time).
  • For consistency: All not generated files should be placed in handwritten and then moved.
  • Set all rizin instruction types for suitable instructions in hexagon_analyses.c. Not just jmp instructions.
  • Adapt syntax highlighting.
  • The method for parsing a Duplex and normal instruction are almost the same. This needs some refactoring.
  • (some more tiny things. See comments in code.)

@XVilka
Copy link
Member

XVilka commented Jul 12, 2021

Overall it's a very good approach, I like it. Will check more thoroughly soon

@XVilka
Copy link
Member

XVilka commented Jul 12, 2021

Please add SPDX with license and your copyright as well.

Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three more things:

rizin/librz/analysis/arch/hexagon/hexagon_analysis.h Outdated Show resolved Hide resolved
rizin/librz/analysis/arch/hexagon/hexagon_analysis.h Outdated Show resolved Hide resolved
rizin/librz/asm/arch/hexagon/hexagon.c Outdated Show resolved Hide resolved
rizin/librz/asm/arch/hexagon/hexagon.c Outdated Show resolved Hide resolved
Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good enough. But having able to operate without UTF-8 is crucial before merging and synchronizing it in Rizin.

@Rot127
Copy link
Member Author

Rot127 commented Jul 16, 2021 via email

.reuse/dep5 Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved

Verified

This commit was signed with the committer’s verified signature.
@XVilka XVilka merged commit 7df09f5 into rizinorg:master Jul 20, 2021
@Rot127 Rot127 mentioned this pull request Jul 20, 2021
4 tasks
@Rot127 Rot127 deleted the llvm-src branch June 3, 2022 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants