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

Immediate value in hexadecimal when MSB is 1 gives error for Immediate instructions #139

Closed
Ty-Greve opened this issue Nov 23, 2021 · 5 comments

Comments

@Ty-Greve
Copy link

Ty-Greve commented Nov 23, 2021

When using hexadecimal form of an immediate value with a MSB bit of 1 for immediate instructions, an error is thrown, even though both immediate values are 12-bits as the instruction specifies.
For example when attempting to assemble with an instruction such as: andi t0, a0, 0xF00

The error message is:

"0xF00": operand is out of range

However attempting to assemble this instruction andi t0, a0, -256 instead, is successful.

Screenshot (238)

I'm not sure if this issue is being worked on yet. I could try to work on this issue if no one else is working on this.

@giancarlopernudisegura
Copy link
Contributor

It looks like it automatically sign extends the immediate into 32-bits before considering the encoding based on the context of the instruction. Therefore, it sign extends 0xf00 into 0x00000f00 which of course is out of boundaries. If you try 0xffffff00 (which does not need to be sign-extended), it works without problem.

It's probably worthwhile to change the way RARS does immediate extension to take into consideration the format of the instruction being used.

@TheThirdOne
Copy link
Owner

Sorry, I didn't see this issue until now. No notification for it popped up.

This is a dup of #134, #103, and #5.

It's probably worthwhile to change the way RARS does immediate extension to take into consideration the format of the instruction being used.

I would like to provide a better error message, but otherwise I think RARS is doing everything correctly. If you consider and t0, t0, 0xF00 as the C code t0 = t0 &0xF00, that is simply not representable in RISC-V assembly. GCC supports the idea that such immediates are not valid.

Since this is a recurring issue, I will give some time to try to make a patch to give better error messages for immediates. I am a little surprised that people do not find the old closed issues though.

@TheThirdOne
Copy link
Owner

I came up with a solution that I am happy enough with.

I am somewhat at a loss for what the best error message would be. My first thought was something like: "Unsigned value is too large to fit into a sign-extended immediate". I am not in a great position to understand what would be most helpful to students though.

What would be the best error message for code like addi t0, t0, 0xFFF? I would appreciate suggestions. @giancarlopernudisegura @Ty-Greve @nickybangs @abdullah-zaiter @ziul123

@ziul123
Copy link
Contributor

ziul123 commented Dec 1, 2021

I believe the C code example is wrong. What would be expected from addi t0, t0, 0xFFF is t0 = t0 + sign_extended(0xFFF), which would be t0 = t0 + (-1), because type I instructions extend the sign of the immediate. I believe the reason this issue keeps popping up is because this expected behaviour is not implemented.

If changing this behaviour is too complicated, I think the most helpful error message would be similar to your own, but with some explanation that RARS is not just taking the immediate given and sign extending it, which was what I myself expected when this happened to me.

@TheThirdOne
Copy link
Owner

I believe the reason this issue keeps popping up is because this expected behaviour is not implemented.

There are 3 possible ways to choose to handle an issue like this: support unsigned immediates, support them with a warning, or disallow them. Supporting unsigned immediates without a warning is clearly bad to me; if you expect addi t0, t0, 0xFFF to be the same as t0 += 0xFFF, it will be really hard to find out why your code as a whole is not working without an error. Supporting them with a warning is a reasonable decision, but I don't know why it is desirable to write unsigned immediates other than saving a few keystrokes. The biggest reason about why I am pretty firm on staying with not supporting it is for consistency with other assemblers. Venus, Jupiter, and Ripes agree such values are invalid. MARS does not support the analogous 16-bit immediates, but allows it through psuedo-ops (which it can do because of the assembler temporary). I am not aware of any other related assemblers that DO allow it.

If changing this behaviour is too complicated, I think the most helpful error message would be similar to your own, but with some explanation that RARS is not just taking the immediate given and sign extending it, which was what I myself expected when this happened to me.

Changing the behavior is not hard, but I do not believe that would be an improvement as explained above.

I agree that some explanation of what RARS is doing is necessary, but I do not know how to convey it well. Having a super long message would be enough, but not ideal and I cannot easily link to a longer explanation. The main reason I was asking for input is the get an actual string to include in the code as the error.

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

No branches or pull requests

4 participants