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

updates for crypto scalar instructions #34

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from

Conversation

anku-anand
Copy link
Contributor

No description provided.

…in rs2 value

 for zext.h rs2 is always 0, if pack instruction is used with x0 as rs2
 then cannot distinguish from each other, hence using cover label to differentiate.

2) updated coverage.py for xperm4, xperm8, zip, unzip instructions
riscv_isac/plugins/internaldecoder.py Outdated Show resolved Hide resolved
riscv_isac/plugins/internaldecoder.py Outdated Show resolved Hide resolved
@anku-anand
Copy link
Contributor Author

In the Bitmanip or crypto scalar spec, zext.h is not mentioned as pseudo instruction.
Only zext.w is mentioned as pseudo instruction for add.uw, zext.w is not included in the spec.
zext.h is only in zbb spec. pack and packw are only in zbkb. In the instruction decoder we
can use the isa string to differentiate between zext.h and packw or pack with rs2 as x0.

@anku-anand anku-anand requested a review from pawks April 11, 2022 19:30
@pawks
Copy link
Collaborator

pawks commented Apr 12, 2022

In the Bitmanip or crypto scalar spec, zext.h is not mentioned as pseudo instruction. Only zext.w is mentioned as pseudo instruction for add.uw, zext.w is not included in the spec. zext.h is only in zbb spec. pack and packw are only in zbkb. In the instruction decoder we can use the isa string to differentiate between zext.h and packw or pack with rs2 as x0.

This causes the same problem as before. What if both the nodes are present in the cgf? Then the decoder will always give out the instruction as zext.h.

@anku-anand
Copy link
Contributor Author

anku-anand commented Apr 12, 2022 via email

@pawks
Copy link
Collaborator

pawks commented Apr 12, 2022

This might work out for the current use case. But it is not a complete solution. Even though the config nodes are different, if both are present in the same cgf, the problem persists. I am not inclined to go down this route as it is a hack for a single scenario. For example this will be extremely problematic to implement for the other decoder (for example the one based on rvopcodes). We need to follow the same philosophy as followed in the rvopcodes. The instructions are always decoded/expressed in terms of the base instruction only.

Added conditions to extract bs and rnum fields in rvopcodesdecoder.py
Removed zext.h from internaldecoder.py
@anku-anand
Copy link
Contributor Author

Updated cgf to use base_op and p_op_cond for pseudo instructions. Removed the condition to check for labels.

@@ -5,7 +5,7 @@

class DecoderSpec(object):
@decoderHookSpec
def setup(self,arch):
def setup(self,arch,isa):
Copy link
Collaborator

Choose a reason for hiding this comment

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

The setup function of the plugins does not need the isa anymore and all of the changes related need to be reverted back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed isa from the plugin setup function

pawks
pawks previously approved these changes Jun 1, 2022
@jamesbeyond jamesbeyond changed the base branch from master to dev May 28, 2024 17:20
@jamesbeyond
Copy link
Collaborator

Dear @anku-anand

The CTG and ISAC Repo had been moved to riscv-arch-test via PR
riscv-non-isa/riscv-arch-test#495

If you think this PR is important, and need to me merged on to risc-v CTG or ISAC, please re-submit your PR against riscv-arch-test

This repository shall be archived in about one week time .

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