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

Error with the first bit of the immediate value in some instructions #5

Closed
zaitera opened this issue Apr 24, 2018 · 7 comments
Closed

Comments

@zaitera
Copy link

zaitera commented Apr 24, 2018

testing with andi instruction
capture

testing with ori instruction
capture1

the same happens with lui instruction, rars is not able to assemble these instructions whenever the first bit of the immediate number is equal to 1.

@TheThirdOne
Copy link
Owner

TheThirdOne commented Apr 24, 2018

This is really an issue with user interface rather than the assembler.

There are two possible ways to interpret 0xFFF as a 32 bit value. The sign extended 0xFFFF FFFF and the unsigned value 0x00000FFF. In RARS, every value represented with hex is interpreted as an unsigned value.

And 0x00000FFF is not actually representable as an immediate because the immediate field is sign extended. So you would need to use 0xFFFFFFFF or -1 instead of 0xFFF.

It would be possible to try the sign extended version if there was a error later down the line, but that is non-trivial to implement. I don't know an easy way to improve RARS to handle this more graciously (including even giving a more detailed error message), but I do recognize that it is a problem.

TLDR:
Use the value that you actually want to be anded rather than a truncated one.

@rmnattas
Copy link
Contributor

Whats the reason of the following warning when loading .byte 0x80 (or .byte 128) in the .data?
Warning in riscv1.asm line 2 column 7: "0x80" is out-of-range for a signed value and possibly truncated
Shouldn't the .data section is just setting the memory for the program, hence it shouldn't worry about the type of the data (signed or unsigned, int or char). It should just put the data (byte), as is, in the next memory location.

Also, for addi x0 x0 0x800, and other immediate instructions (addi, slti, andi, ori, xori, sltiu), I don't think the issue should be closed. I see how in MARS the immediate are not sign-extended hence maybe the structure of the program treat hex as an unsigned value, but at the end this should aim to simulate the real RISC-V architecture.

I'm willing to try and fix both of these issues, any tips or things I should keep in mind while doing so would be appreciated.

@TheThirdOne
Copy link
Owner

Whats the reason of the following warning when loading .byte 0x80 (or .byte 128) in the .data?

It would probably be good to widen the bounds for .byte and .half to allow for the maximum unsigned value the can store because there is no sign extension issue.

Do note though that it is just a warning. The code works as intended with 0x80 and 128.

Also, for addi x0 x0 0x800, and other immediate instructions (addi, slti, andi, ori, xori, sltiu), I don't think the issue should be closed.

If I had a good idea on how to provide a better error message, I would leave it open. However, as stated above, I don't know how to improve this and ultimately its a UI problem rather than a correctness one.

but at the end this should aim to simulate the real RISC-V architecture.

RARS is the closest you will get to a real RISC-V architecture among educational simulators.

Specifically for the issue at hand (an assembling rather than simulating issue), gcc agrees with RARS that 0xFFF is not an acceptable paramater for the xxxi instructions.

login$ riscv64-linux-gnu-gcc test.s -o out.o
test.s: Assembler messages:
test.s:6: Error: illegal operands `andi t0,t0,0xFFF'

I'm willing to try and fix both of these issues, any tips or things I should keep in mind while doing so would be appreciated.

For the issue with .byte and .half it should be a quick fix following from where they are parsed. Finding the spot in the code is most of the work so just leave it to me.

For the error around xxxi, the only way I can see to make it better is an overhaul of the error reporting with better messages.

If you want to help out with an error message overhaul, I would suggest opening up an issue showcasing existing error messages that are unclear. This would potentially help people that might google their error and it would help me figure out the best way to change the error system. I wouldn't spend the effort for just xxxi related issues, but if several other messages could be improved, I would consider it.

@rmnattas
Copy link
Contributor

rmnattas commented May 30, 2019

I was surprised that gcc also throws an error for that, as immediate are 12-bits and 0x800 is only 12-bits. Then I got to this issue:

The immediate is 12 bits, but that corresponds to the range [-0x800,0x7ff]. 0xfff is out of range.

Which is not intuitive to use a negative hex (-0x002 instead of 0xffe for -2) but if that's what they use in the assembler, and it works in RARS, then we'll have to follow it.

@TheThirdOne
Copy link
Owner

TheThirdOne commented May 30, 2019

So if 0x800 was a valid immediate, what would you expect t0 to after executing this

li t0, 0xFFFF0FFF
andi t0, t0, 0x800

If you are expecting 0x000000800 that cannot be done in a single instruction for andi.

And I would say that expecting 0xFFFF0800, then you might as well actually put the value you intend to and with in the immediate field rather than rely the internals of the representation and sign extension.

Which is not intuitive to use a negative hex (-0x002 instead of 0xffe for -2) but if that's what they use in the assembler, and it works in RARS, then we'll have to follow it.

I don't believe RARS supports negatives in front of hexadecimals, but using -2 or 0xfffffffe will work as intended.

@rmnattas
Copy link
Contributor

I would expect 0xFFFF0800, it may not be the most intuitive when using the immediate 0x800 but it's what the RISC-V architecture do (sign-extend the immediate).

I don't believe RARS supports negatives in front of hexadecimals, but using -2 or 0xfffffffe will work as intended.

I have tried it with couple of examples and it does works as the gcc assembler does.

@rmnattas
Copy link
Contributor

rmnattas commented May 30, 2019

Apparently, the rars.jar that I downloaded from the release page on May 1st supports andi t0 t0 -0x800, but the current release doesn't. So it looks like there's a change between May 1st and now that made them unsupported.

Also, the error is duplicated for some reason

Error in riscv7.asm line 3 column 12: andi x5 x5 -0x800
Invalid language element: -0x800
Error in riscv7.asm line 3 column 12: andi x5 x5 -0x800
Invalid language element: -0x800
Assemble: operation completed with errors.

TheThirdOne added a commit that referenced this issue Jun 4, 2019
Unterminated strings were mentioned in #29, I decided to make them an
error because it was slightly technically easier. Also I don't know of
any riscv assembler which allows unterminated strings.

The range warnings for .byte and .half were improved to not warn on
values the fit in an unsigned byte/half and to say exactly how the value
is truncated. Now it matches how gcc handles it. This was mentioned in #5
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

3 participants