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

dialects: (x86) PR10 - RRI Operations #2524

Merged
merged 4 commits into from
May 2, 2024
Merged

Conversation

KGrykiel
Copy link
Collaborator

@KGrykiel KGrykiel commented May 2, 2024

Added support for instructions with 1 destination register, one source register and one immediate.

@KGrykiel KGrykiel added the dialects Changes on the dialects label May 2, 2024
@KGrykiel KGrykiel requested review from superlopuh, compor and tarinduj May 2, 2024 13:30
@KGrykiel KGrykiel self-assigned this May 2, 2024
Copy link

codecov bot commented May 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.85%. Comparing base (d588ab7) to head (03aebb8).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2524      +/-   ##
==========================================
- Coverage   89.85%   89.85%   -0.01%     
==========================================
  Files         351      351              
  Lines       43938    43963      +25     
  Branches     6559     6562       +3     
==========================================
+ Hits        39481    39501      +20     
- Misses       3500     3505       +5     
  Partials      957      957              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

KGrykiel and others added 2 commits May 2, 2024 14:37
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
@KGrykiel KGrykiel requested a review from superlopuh May 2, 2024 13:41
xdsl/dialects/x86/ops.py Outdated Show resolved Hide resolved
"""

r2 = operand_def(R2InvT)
immediate: AnyIntegerAttr = attr_def(AnyIntegerAttr)
Copy link
Member

Choose a reason for hiding this comment

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

Does x86 specify the bitcount of the immediate values?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image
According to the x86 manual immediates can only have 8, 16 or 32 bits (with 32 being the default I believe) with 64 bit immediates only appearing for rare instructions afaik.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image
The configuration im targetting also implies a 32 bit immediate

Copy link
Member

Choose a reason for hiding this comment

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

Ok, feels like we might want to encode that at some point, but not urgently

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good to merge then?

Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
@KGrykiel KGrykiel merged commit 167d54c into main May 2, 2024
10 checks passed
@KGrykiel KGrykiel deleted the KGrykiel/x86dialect-PRs10 branch May 2, 2024 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialects Changes on the dialects
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants