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

PPC LLVM 18 #2540

Merged
merged 24 commits into from
Dec 5, 2024
Merged

PPC LLVM 18 #2540

merged 24 commits into from
Dec 5, 2024

Conversation

Rot127
Copy link
Collaborator

@Rot127 Rot127 commented Nov 9, 2024

Your checklist for this pull request

  • I've documented or updated the documentation of every API function and struct this PR changes.
  • I've added tests that prove my fix is effective or that my feature works (if possible)

Detailed description

New

(According to LLVM changelog)

  • Added DFP instruction.
  • Added the SCV instruction.

Changes

  • The MCUpdater generates shorter and better readable file names.
  • Memory decoder were simplified by decoding disponent and base reg separately.
  • DFORM -> DFORM_BASE
  • Use inverted MCInstDesc table.
  • Replace the many declared printer in PPCInstPrinter with static inlines.
  • Renamed groups to upper case.
  • Switched to ARCH_add_cs_detail_X() function names.
  • Remove PPCInstPrinter.h because it is no longer used.
  • Absolute addresses are printed as unsigned immediate, not as signed.
  • Be way more strict about enabled CPU features and architectures.
  • Adds Power7-Power10 + Future ISA CPUs.
  • Adds and moves multiple features for PPC CPUs:
    • +aix Enables AIX OS assembly (only: PowerPC)
    • +booke Enables BOOKE extension (only: PowerPC)
    • +maix Enables Modern AIX assembly (only: PowerPC)
    • +msync Has only the msync instruction instead of sync. Implies BookE. (only: PowerPC)
    • +qpx Enables QPX extension (only: PowerPC)
    • +ps Enables PS extension (only: PowerPC)
    • +spe Enables SPE extension (only: PowerPC)

Test plan

Newly added and all green.

Closing issues

...

@Rot127 Rot127 marked this pull request as draft November 9, 2024 19:12
@github-actions github-actions bot added python bindings and removed ARM Arch python bindings LLVM-core-files auto-sync Documentation labels Nov 18, 2024
@Rot127 Rot127 marked this pull request as ready for review November 20, 2024 18:16
@Rot127 Rot127 requested a review from kabeor November 20, 2024 19:05
**New**

(According to LLVM changelog)

- Added DFP instruction.
- Added the SCV instruction.

**Changes**

- Memory decoder were simplified by decoding disponent and base reg separately.
- `DFORM` -> `DFORM_BASE`
- Use inverted `MCInstDesc` table.
- Replace the many declared printer in PPCInstPrinter with `static inlines`.
- Renamed groups to upper case.
- Switched to `ARCH_add_cs_detail_X()` function names.
- Remove `PPCInstPrinter.h` because it is no longer used.
Due to llvm/llvm-project@4b43ef3
the names of the operands were matched.
Because FRT dosn't exist in the XForm_1 class,
the generated tables didn't decoded them.
@XVilka
Copy link
Contributor

XVilka commented Nov 24, 2024

@kabeor take a look at this one please

@hainest
Copy link
Contributor

hainest commented Nov 25, 2024

Huge thanks for doing this. It looks like LLVM is choosing to only implement either an X-form or a D-form of some instructions[1]. It would be nice to have both forms, but that would require manual tracking. Is there a mechanism in the auto-sync importer to do this? I'd be happy to submit a PR for that.

[1] See dyninst/dyninst@64cec85 for a few examples based on v3.1C of the standard.

@Rot127
Copy link
Collaborator Author

Rot127 commented Nov 25, 2024

@hainest Do you have two example byte strings for testing. One in X one in D form? Then I can check how they decode.

@hainest
Copy link
Contributor

hainest commented Nov 25, 2024

Let me run them through an assembler. I just realized that it might actually be a difference in regular versus extended mnemonics.

Copy link
Member

@kabeor kabeor left a comment

Choose a reason for hiding this comment

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

Amazing!

@kabeor kabeor merged commit 93a104c into capstone-engine:next Dec 5, 2024
14 checks passed
@Rot127 Rot127 deleted the ppc-llvm-18 branch December 6, 2024 18:04
@Rot127
Copy link
Collaborator Author

Rot127 commented Dec 6, 2024

@hainest I currently fix some regressions of this one and checked one example from the link you provided. LLVM seemed to implement only the XFORM?

defm DENBCD: XForm_S1_FRTB5r<59, 834, (outs f8rc:$FRT),
                              (ins u1imm:$S, f8rc:$FRB),
                              "denbcd", "$S, $FRT, $FRB", []>;
defm DENBCDQ: XForm_S1_FRTB5r<63, 834, (outs fpairrc:$FRT),
                               (ins u1imm:$S, fpairrc:$FRB),

It would be nice if you open an issue about it, if you find some.
Maybe it is due to the definition of them in the td files. Making some isPseudo or CodeGenOnly or similar.

@Rot127
Copy link
Collaborator Author

Rot127 commented Dec 6, 2024

@hainest Off-topic. But I see your tool also seems to use Capstone quite extensively. If you have any work arounds for buggy instruction details, I would be really happy if you can open issues about it. One issue to collect all of them is fine.
I try to improve the test coverage.

@hainest
Copy link
Contributor

hainest commented Dec 6, 2024

@hainest I currently fix some regressions of this one and checked one example from the link you provided. LLVM seemed to impleemnt only the XFORM?

defm DENBCD: XForm_S1_FRTB5r<59, 834, (outs f8rc:$FRT),
                              (ins u1imm:$S, f8rc:$FRB),
                              "denbcd", "$S, $FRT, $FRB", []>;
defm DENBCDQ: XForm_S1_FRTB5r<63, 834, (outs fpairrc:$FRT),
                               (ins u1imm:$S, fpairrc:$FRB),

It would be nice if you open an issue about it, if you find some. Maybe it is due to the definition of them in the td files. Making some isPseudo or CodeGenOnly or similar.

So sorry, I've been inundated with fixing bugs. I will try to take a look at these over the weekend and get you some more details. I'll open an issue once I have something concrete.

@hainest
Copy link
Contributor

hainest commented Dec 6, 2024

@hainest Off-topic. But I see your tool also seems to use Capstone quite extensively. If you have any work arounds for buggy instruction details, I would be really happy if you can open issues about it. One issue to collect all of them is fine. I try to improve the test coverage.

Yes. I have some x86 instruction semantics that I'd like to add. I have three more instructions to analyze before I'm read to submit them. That said, I am very much in favor of integrating Zydis into Capstone. It's just too much work to keep track of the x86 instructions. I have confirmed that the missing semantics I found are present in Zydis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants