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

Logical with Immediate meta-mnemonics and and bug fixes #103

Merged
merged 3 commits into from
Feb 1, 2025

Conversation

burhanr13
Copy link
Contributor

I have added meta mnemonics for logical operations with immediate (and_imm, ands_imm,orr_imm,eor_imm) and a function to check whether a value is a valid immediate for these kinds of instructions. I also fixed align() to not add padding if curr is already aligned, and reset() to reset the protection mode to RW so the code can be written to again.

@kawakami-k
Copy link
Collaborator

Hi, @burhanr13 !
Thank you very much for you contribution! I'll check the PR and merge soon!

@kawakami-k
Copy link
Collaborator

@herumi

I think this PR is fine to merge. Would you please check this PR too? If there are no problems, please merge it. After that, I will add test patterns for this PR for CI and update the version number.

@herumi
Copy link
Collaborator

herumi commented Feb 1, 2025

It's good for me, but if add_imm is changed to

void and_(const WReg &dst, const WReg &src, uint32_t imm, const WReg *tmp = 0) {
  if (isValidLogicalImm(imm, 32)) {
    // original and_(dst, src, imm);
  } else {
    mov(tmp, imm);
    // original and_(dst, src, tmp);
  }
}

then we can use the same function name. How do you think about it?

@kawakami-k
Copy link
Collaborator

@herumi

Although it is not a clear policy, the current xbyak_aarch64_mnemonic.h contains the implementations of the instructions that exist in Armv8-v9 instruction set, and the pseudo-instructions unique to xbyak_aarch64 are in xbyak_aarch64_meta_mnemonic.h. To maintain this consistency, it would be good to name the pseudo-instructions like and_imm and put them in `xbyak_aarch64_meta_mnemonic.h".

@herumi
Copy link
Collaborator

herumi commented Feb 1, 2025

@kawakami-k
I see, it's okay.

@kawakami-k kawakami-k merged commit 9db7da1 into fujitsu:main Feb 1, 2025
@kawakami-k
Copy link
Collaborator

@burhanr13
Thank you for the contribution!

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.

3 participants