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

How to support fixed-size rvv intrinsic type in gcc ? #373

Open
zengdage opened this issue Apr 11, 2024 · 10 comments
Open

How to support fixed-size rvv intrinsic type in gcc ? #373

zengdage opened this issue Apr 11, 2024 · 10 comments

Comments

@zengdage
Copy link

Hi @howjmay ,I have a question about the neon intrinsic types convert into rvv intrinsic types. Can you help me with that ?

As I know,the neon intrinsic types are fixed-size,but rvv intrinsic types are sizeless. For example, the size of uint8x16_t (neon type) is 128 bits,and the size of vuint8m1_t (rvv type) is unkown, but you bind them together in neon2rvv.h.

I think it will make some errors in such scenario because the compiler need to allocate fixed-size stack memory space for vtmp.

uint8x16_t vtmp;
.......
vtmp = vld1q_u8(rp);
@howjmay
Copy link
Owner

howjmay commented Apr 11, 2024

Sorry I can't understand your question totally.
I guess you may mention this case
https://github.com/howjmay/neon2rvv/blob/main/tests/impl.cpp#L574
It should be covered in the test case

@zengdage
Copy link
Author

Sorry, maybe I didn't explain it clearly and give a incomplete example. May be you can test the following code,

#include <stdlib.h>

#if defined(__arm__) || defined(__aarch64__)
#include <arm_neon.h>
#else

#include <riscv_vector.h>

typedef vuint8m1_t uint8x16_t;

__attribute__((always_inline)) uint8x16_t vld1q_u8(const uint8_t *ptr) { return __riscv_vle8_v_u8m1(ptr, 16); }
__attribute__((always_inline)) void vst1q_u8(uint8_t *a, uint8x16_t b) { return __riscv_vse8_v_u8m1(a, b, 16); }
#endif

struct test_struct {
  uint8x16_t x;
};

int main(int argc, char **argv) {
  unsigned char *data;

  data = malloc(1024);
  if (!data) {
    return -1;
  }
  struct test_struct temp;
  temp.x = vld1q_u8(data);
  vst1q_u8(data + 512, temp.x);
  return 0;
}

Because the uint8x16_t is fixed-size in neon type system,so it can be a member in union or member, but vuint8x16_t can't. I found that it have been supported in gcc and clang in riscv-non-isa/rvv-intrinsic-doc#176.

@OMaghiarIMG
Copy link
Contributor

Hello @howjmay
Yes @zengdage is correct, if you want to use Neon2RVV in a project that uses Neon types in structs/global variables it needs fixed-size RVV types.
I could contribute the switch.

@mr-c
Copy link

mr-c commented Apr 11, 2024

The optimized RISCV64 implementations for Neon in SIMDe use a fixed size

We have included clang-qemu-rvv testing for the following RISC-V V Extension architectures, both with and without ZVFH enabled:

  • vlen = 128 & elen = 64
  • vlen = 256 & elen = 64
  • vlen = 512 & elen = 64

To compile SIMDe with support for the conversion from NEON to the RISC-V Vector Extension, please use Clang-17 and include the flag -mrvv-vector-bits=<vector_length_of_vector_machine> during compilation. Replace <vector_length_of_vector_machine> with the actual vector length of RISC-V vector machine.

simd-everywhere/simde#1130

To use SIMDe in a similar manner as neon2rvv:

#define SIMDE_ENABLE_NATIVE_ALIASES
#include <simde/arm/neon.h>

And it is recommended to add -DSIMDE_ENABLE_OPENMP -fopenmp-simd -O3 to your CFLAGS and/or CXXFLAGS when compiling with GCC/Clang.

Very recent GCC can also use fixed size rvv:
https://gcc.gnu.org/pipermail/gcc-patches/2024-March/648204.html

@howjmay
Copy link
Owner

howjmay commented Apr 11, 2024

If fixed size rvv, has been supported in the recent GCC/Clang, then I temporarily can't see the potential need for this feature.

@zengdage @mr-c Do you guys know any case that we should support this feature. I think rvv still hasn't not reached the stable version yet.
If there is such a need exists, I am more than happy for the offer that @OMaghiarIMG's provides

In the meantime, I am ok for a patch to support fixed size rvv now. Just I hope it could be a build option which we can turn it off in default with macro

@OMaghiarIMG
Copy link
Contributor

In the meantime, I am ok for a patch to support fixed size rvv now. Just I hope it could be a build option which we can turn it off in default with macro

That's right, if the built option is detected the Neon types will be defined using the fixed RVV types, otherwise it will use the current sizeless types, no other changes. Just one setback, seems like the tuple types can't have a fixed size yet, opened topic at LLVM: llvm/llvm-project#88369

@howjmay
Copy link
Owner

howjmay commented Apr 12, 2024

@OMaghiarIMG Thank you so much for the help!

@howjmay
Copy link
Owner

howjmay commented Apr 18, 2024

@OMaghiarIMG Will you do the fix? No rush, just curious.

@OMaghiarIMG
Copy link
Contributor

@OMaghiarIMG Will you do the fix? No rush, just curious.

Yeah, worked on it a bit, the problem with the RVV tuple types, beside not supporting fixed-size, is that they are incompatible with Neon tuple type direct access - i.e. you can access a Neon int32x4x4_t using x.var[3], but this doesn't work for vint32m1x4_t.
I've tried to use Neon2RVV as a translation layers for an existing Neon library but the only solution is to redefine the Neon tuples as a struct and refactor the implementations:

typedef struct  int32x4x4_t {
    int32x4_t val[4];
} int32x4x4_t;

FORCE_INLINE void vst4q_s32(int32_t *a, int32x4x4_t b) {
  vint32m1x4_t x = __riscv_vcreate_v_i32m1x4(b.val[0], b.val[1], b.val[2], b.val[3]);
  return __riscv_vsseg4e32_v_i32m1x4(a, x, 4);
}

The disadvantage is that now Neon2RVV must always be built with specifying fixed-RVV size, but I don't see any other way.
What do you think about merging such a change?

@howjmay
Copy link
Owner

howjmay commented Apr 24, 2024

I think only the tuple types is good. This way aligns to the neon way to implement tuples. It should be easy for other developers to understand the codebase.

Thank you!

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