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

Questions about extending to 128bits floating point arithmetic #160

Closed
LiuTaowen-Tony opened this issue Apr 11, 2022 · 13 comments
Closed

Questions about extending to 128bits floating point arithmetic #160

LiuTaowen-Tony opened this issue Apr 11, 2022 · 13 comments
Labels
question Further information is requested

Comments

@LiuTaowen-Tony
Copy link

Hi, I am new to this project, and I want to contribute to this project by implementing Q extension.

I found several difficulties.

  1. load and store
    image
    For storing and loading 128bits data, alignment checking and virtual address translation can only support up to 64bits. Changing these to 128 bits would affect other extensions. I tried to use 2 separate memory reads to load the data. But I think there will be some problem when there is an invalid read.

  2. return value from C softfloat lib
    For the C softfloat lib, in the existing code, to return a value from a C function, it uses a 64-bits-vector. I worked around this : I defined two separate extern functions to do the calculation: One returns the upper 64 bits, and the other returns the lower 64bits. I am not sure how to transfer structs between sail and C, and I want to ask where to find documentations about this.
    image
    image

@martinberger
Copy link
Collaborator

martinberger commented Apr 11, 2022

I wonder what the official status of RV128 is and the Q extension. Is any company working actively on implementing RV128? It there somebody in charge of the Q extension right now?

@aswaterman @LiuTaowen-Tony

@billmcspadden-riscv
Copy link
Collaborator

billmcspadden-riscv commented Apr 11, 2022 via email

@martinberger
Copy link
Collaborator

martinberger commented Apr 11, 2022

@billmcspadden-riscv Do you know where I can find the official Q-extension document that was ratified?

@billmcspadden-riscv
Copy link
Collaborator

billmcspadden-riscv commented Apr 11, 2022 via email

@LiuTaowen-Tony
Copy link
Author

Hi, is it possible to provide a more detailed version of this?

For example, here, it doesn't give a 3bit encoding for the width "Q".

image

And I wonder where to find more detailed description of how to load and store 128bits from/to memory. (Split into 2 8byte read/write or something)

@jrtc27
Copy link
Collaborator

jrtc27 commented Apr 11, 2022

For storing and loading 128bits data, alignment checking and virtual address translation can only support up to 64bits. Changing these to 128 bits would affect other extensions. I tried to use 2 separate memory reads to load the data. But I think there will be some problem when there is an invalid read.

In what sense? mem_read is polymorphic, and max_mem_access is 16. We use 16-byte/128-bit loads and stores (ish) downstream in sail-cheri-riscv just fine. All you're missing is a QUAD in word_width for decoding purposes.

For the C softfloat lib, in the existing code, to return a value from a C function, it uses a 64-bits-vector. I worked around this : I defined two separate extern functions to do the calculation: One returns the upper 64 bits, and the other returns the lower 64bits. I am not sure how to transfer structs between sail and C, and I want to ask where to find documentations about this.

It can get messy. If you want a 128-bit integer, use a sail_int (mpz_t) rather than a mach_int (int64_t). You shouldn't need a struct.

Hi, is it possible to provide a more detailed version of this?

This isn't the place for questions or feedback on the ISA manual. But you probably want the "RV32/64G Instruction Set Listings" chapter which, despite its name, includes things other than G (namely Q and Zfh in the version I have).

@LiuTaowen-Tony
Copy link
Author

For storing and loading 128bits data, alignment checking and virtual address translation can only support up to 64bits. Changing these to 128 bits would affect other extensions. I tried to use 2 separate memory reads to load the data. But I think there will be some problem when there is an invalid read.

In what sense? mem_read is polymorphic, and max_mem_access is 16. We use 16-byte/128-bit loads and stores (ish) downstream in sail-cheri-riscv just fine. All you're missing is a QUAD in word_width for decoding purposes.

For the C softfloat lib, in the existing code, to return a value from a C function, it uses a 64-bits-vector. I worked around this : I defined two separate extern functions to do the calculation: One returns the upper 64 bits, and the other returns the lower 64bits. I am not sure how to transfer structs between sail and C, and I want to ask where to find documentations about this.

It can get messy. If you want a 128-bit integer, use a sail_int (mpz_t) rather than a mach_int (int64_t). You shouldn't need a struct.

Hi, is it possible to provide a more detailed version of this?

This isn't the place for questions or feedback on the ISA manual. But you probably want the "RV32/64G Instruction Set Listings" chapter which, despite its name, includes things other than G (namely Q and Zfh in the version I have).

Thank you for your guide!

I will look at cheri extension to see if I can solve it.

@jrtc27
Copy link
Collaborator

jrtc27 commented Apr 11, 2022

For storing and loading 128bits data, alignment checking and virtual address translation can only support up to 64bits. Changing these to 128 bits would affect other extensions. I tried to use 2 separate memory reads to load the data. But I think there will be some problem when there is an invalid read.

In what sense? mem_read is polymorphic, and max_mem_access is 16. We use 16-byte/128-bit loads and stores (ish) downstream in sail-cheri-riscv just fine. All you're missing is a QUAD in word_width for decoding purposes.

For the C softfloat lib, in the existing code, to return a value from a C function, it uses a 64-bits-vector. I worked around this : I defined two separate extern functions to do the calculation: One returns the upper 64 bits, and the other returns the lower 64bits. I am not sure how to transfer structs between sail and C, and I want to ask where to find documentations about this.

It can get messy. If you want a 128-bit integer, use a sail_int (mpz_t) rather than a mach_int (int64_t). You shouldn't need a struct.

Hi, is it possible to provide a more detailed version of this?

This isn't the place for questions or feedback on the ISA manual. But you probably want the "RV32/64G Instruction Set Listings" chapter which, despite its name, includes things other than G (namely Q and Zfh in the version I have).

Thank you for your guide!

I will look at cheri extension to see if I can solve it.

I would not suggest that; it's all complicated by having to support tagged memory. The only relevant bit is that it passes 16 to mem_read and similar functions in some cases.

@aswaterman
Copy link
Member

As @billmcspadden-riscv said, RV128 is not ratified (or even anywhere close to frozen), but Q was ratified a few years ago.

@LiuTaowen-Tony see page 135 of https://github.com/riscv/riscv-isa-manual/releases/download/Ratified-IMAFDQC/riscv-spec-20191213.pdf for the instruction encodings.

@LiuTaowen-Tony
Copy link
Author

I think it is only ext_data_get_addr and check_misaligned requires the width parameter (mem_read supports 16 byte). Adding QUAD to word_width would involve in lots of changes with the existing code. Can I write new functions like ext_data_get_addr_Q and check_misaligned_Q to work around?

@jrtc27
Copy link
Collaborator

jrtc27 commented Apr 11, 2022

Just add to word_width, it's not a lot of code, and anything else is a gross hack.

@LiuTaowen-Tony
Copy link
Author

@jrtc27 Yeah, changing word_width worked. It is just that I did this to size_bits to keep the backward compatibility. Is this acceptable?

Screen Shot 2022-04-16 at 4 43 12 PM

@Timmmm Timmmm added the question Further information is requested label May 7, 2024
@Timmmm
Copy link
Collaborator

Timmmm commented May 7, 2024

There's a PR for this now, so I think we can close this:

#445

@Timmmm Timmmm closed this as completed May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

6 participants