-
Notifications
You must be signed in to change notification settings - Fork 303
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
Added some V extension Pseudo-instructions #295
base: master
Are you sure you want to change the base?
Added some V extension Pseudo-instructions #295
Conversation
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Shouldn't the "gt" pseudos alias to "le", not "lt" ? |
@aamartin0000 I believe you're confusing this with negation, in which case it woukd be correct. However, if you take a closer look from an hardware standpoint, just switching the arguments wouldn't make it a negation since if they are equal(or the difference between them is 0), the branch is supposed to happen either way, |
Ok, I get it. If the two operands have the same value, flt will be false, and so will fgt, while fge would be true (unless the value is NaN). Next question: you have This is somewhat confusing because you are aliasing the .vv form with a .vf op, with rs1 instead of vs1. Should these not be using vmflt.vv/vmfle.vv ? |
rv_v
Outdated
@@ -138,6 +138,11 @@ vfwnmacc.vf 31..26=0x3d vm vs2 rs1 14..12=0x5 vd 6..0=0x57 | |||
vfwmsac.vf 31..26=0x3e vm vs2 rs1 14..12=0x5 vd 6..0=0x57 | |||
vfwnmsac.vf 31..26=0x3f vm vs2 rs1 14..12=0x5 vd 6..0=0x57 | |||
|
|||
#Pseudo OPFVF | |||
$pseudo_op rv_v::vmflt.vf vmfgt.vv 31..26=0x1b vm rs1 vs2 14..12=0x5 vd 6..0=0x57 | |||
$pseudo_op rv_v::vmfle.vf vmfge.vv 31..26=0x19 vm vs2 rs1 14..12=0x5 vd 6..0=0x57 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can a vv
variant be a pseudo of a vf
variant? I assume what you want here is something like
$pseudo_op rv_v::vmflt.vv vmfgt.vv 31..26=0x1b vm vs1 vs2 14..12=0x5 vd 6..0=0x57
$pseudo_op rv_v::vmfle.vv vmfge.vv 31..26=0x19 vm vs1 vs2 14..12=0x5 vd 6..0=0x57
but I don't see how this communicates to the downstream tooling that vs1 and vs2 are swapped. Are we missing some additional functionality that we need to implement before downstream tools can make use of these swapped-argument pseudos?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This are wrong, sure, they should be vv, my bad. I'm not quite sure yet how they are parsed differently, I'll take a look and answer back on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instruction descriptionts are fixed. I'll wait for #283 to do the rest of the required work.
$pseudo_op rv_v::vmslt.vv vmsgt.vv 31..26=0x1b vm vs1 vs2 14..12=0x0 vd 6..0=0x57 | ||
$pseudo_op rv_v::vmsltu.vv vmsgtu.vv 31..26=0x1a vm vs1 vs2 14..12=0x0 vd 6..0=0x57 | ||
$pseudo_op rv_v::vmsle.vv vmsge.vv 31..26=0x1d vm vs1 vs2 14..12=0x0 vd 6..0=0x57 | ||
$pseudo_op rv_v::vmsleu.vv vmsgeu.vv 31..26=0x1c vm vs1 vs2 14..12=0x0 vd 6..0=0x57 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same concern as for the FP comparisons.
@@ -411,6 +427,13 @@ vwmaccu.vv 31..26=0x3c vm vs2 vs1 14..12=0x2 vd 6..0=0x57 | |||
vwmacc.vv 31..26=0x3d vm vs2 vs1 14..12=0x2 vd 6..0=0x57 | |||
vwmaccsu.vv 31..26=0x3f vm vs2 vs1 14..12=0x2 vd 6..0=0x57 | |||
|
|||
#Pseudo-Instructions for Vector Integer Extension Instructions | |||
|
|||
$pseudo_op rv_v::vmxor.mm vmclr.m 31..26=0x1b 25=1 vs2=vd vs1=vd 14..12=0x2 vd 6..0=0x57 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More of a question than a comment: is there a reason for using vs2=vd rather than vs2=vs1 here? By transitivity, they're equivalent; I'm just curious if there should be a stylistic default and, if so, why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to ensure that the parsing assumes them as the same value. This is extremely useful when generating, per example, the yaml_dump since we can know what differs from the original instruction to the pseudo-instruction.
This can be parsed to ouput, per example, the following: https://github.com/riscv-software-src/riscv-unified-db/blob/00213d2b2703240dc4fbe90dcdd8749e5b5f7e35/arch/inst/B/add.uw.yaml#L25
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you missed my point. There are two equivalent ways of expressing this: we can say vs2=vd and vs1=vd, or we can say vs1=vd and vs2=vs1. The same information would be conveyed to parsing tools either way. I am just wondering why we are anchoring on vd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely missed it, but I believe the best way to go for it is to say that "original_field = other_field", since parsing wise this is more helpful
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
placing a hold on this PR until the swapped-argument pseudo issue is resolved.
I really apologize for my delay on this, but I was really busy to pick up this changes on the parsing and still ensure they would be 100% correct, I'll be looking into them now. Though I should probably wait for #283 before doing any changes, right? |
No problem!
Yeah, probably so. |
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Added some V extension Pseudo-instructions described on the ISA Manual and implemented on binutils. I'm working on adding the rest of them.