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.

Sign up for GitHub

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

Jump to bottom

[RISC-V] disambiguation error of rvv intrinsic vector-scalar calls with fixed-length vector type #64404

Closed
garthlei opened this issue Aug 4, 2023 · 3 comments
Closed

[RISC-V] disambiguation error of rvv intrinsic vector-scalar calls with fixed-length vector type #64404

garthlei opened this issue Aug 4, 2023 · 3 comments
Labels
backend:RISC-V clang:frontend Language frontend issues, e.g. anything involving "Sema"

Comments

@garthlei
Copy link

garthlei commented Aug 4, 2023

Code:

#include <riscv_vector.h>

typedef vint64m1_t __attribute__((riscv_rvv_vector_bits(128))) vint64m1_128_t;

int main() {
  vint64m1_t a;
  vint64m1_128_t b;
  vint64m1_t c = __riscv_vand(a, 0, 0);
  vint64m1_t d = __riscv_vand(b, 0, 0);
}

Compiler message (clang -march=rv64gcv -mrvv-vector-bits=128 -S test.c):

test.c:9:18: error: call to '__riscv_vand' is ambiguous
    9 |   vint64m1_t d = __riscv_vand(b, 0, 0);
      |                  ^~~~~~~~~~~~
test.c:9:18: note: candidate function
test.c:9:18: note: candidate function
test.c:9:18: note: candidate function
test.c:9:18: note: candidate function
test.c:9:18: note: candidate function
test.c:9:18: note: candidate function
1 error generated.
@llvmbot
Copy link
Collaborator

llvmbot commented Aug 4, 2023

@llvm/issue-subscribers-backend-risc-v

@topperc
Copy link
Collaborator

topperc commented Aug 4, 2023

It does work with -fno-lax-vector-conversions. I need to investigate more.

topperc added a commit that referenced this issue Aug 21, 2023
@topperc
[RISCV] Check type size for lax conversions between RVV builtin types…
33af2f1 
… and VectorType::RVVFixedLengthDataVector.

This code was copied from SVE and modified for RVV. For SVE, there
is only one size for builtin types so they didn't need to check
the size. For RVV, due to LMUL there are 7 different sizes of builtin
types so we do need to check the size.

I'm not sure we should have lax vector conversions at all for RVV.
That appears to be contributing to #64404

This patch at least fixes the obvious correctness issue.
This should be backported to LLVM 17.

Reviewed By: jacquesguan

Differential Revision: https://reviews.llvm.org/D157130
llvmbot pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 21, 2023
@topperc @llvmbot
[RISCV] Check type size for lax conversions between RVV builtin types…
a7ad924 
… and VectorType::RVVFixedLengthDataVector.

This code was copied from SVE and modified for RVV. For SVE, there
is only one size for builtin types so they didn't need to check
the size. For RVV, due to LMUL there are 7 different sizes of builtin
types so we do need to check the size.

I'm not sure we should have lax vector conversions at all for RVV.
That appears to be contributing to llvm/llvm-project#64404

This patch at least fixes the obvious correctness issue.
This should be backported to LLVM 17.

Reviewed By: jacquesguan

Differential Revision: https://reviews.llvm.org/D157130

(cherry picked from commit 33af2f131db71a18aefc5469129540e2097a537f)
tru pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 25, 2023
@topperc @tru
[RISCV] Check type size for lax conversions between RVV builtin types…
2e17e9e 
… and VectorType::RVVFixedLengthDataVector.

This code was copied from SVE and modified for RVV. For SVE, there
is only one size for builtin types so they didn't need to check
the size. For RVV, due to LMUL there are 7 different sizes of builtin
types so we do need to check the size.

I'm not sure we should have lax vector conversions at all for RVV.
That appears to be contributing to llvm/llvm-project#64404

This patch at least fixes the obvious correctness issue.
This should be backported to LLVM 17.

Reviewed By: jacquesguan

Differential Revision: https://reviews.llvm.org/D157130

(cherry picked from commit 33af2f131db71a18aefc5469129540e2097a537f)
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 2, 2023
@topperc @razmser
[RISCV] Check type size for lax conversions between RVV builtin types…
a9d72bc 
… and VectorType::RVVFixedLengthDataVector.

This code was copied from SVE and modified for RVV. For SVE, there
is only one size for builtin types so they didn't need to check
the size. For RVV, due to LMUL there are 7 different sizes of builtin
types so we do need to check the size.

I'm not sure we should have lax vector conversions at all for RVV.
That appears to be contributing to llvm#64404

This patch at least fixes the obvious correctness issue.
This should be backported to LLVM 17.

Reviewed By: jacquesguan

Differential Revision: https://reviews.llvm.org/D157130
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 2, 2023
@topperc @razmser
[RISCV] Check type size for lax conversions between RVV builtin types…
4e414ef 
… and VectorType::RVVFixedLengthDataVector.

This code was copied from SVE and modified for RVV. For SVE, there
is only one size for builtin types so they didn't need to check
the size. For RVV, due to LMUL there are 7 different sizes of builtin
types so we do need to check the size.

I'm not sure we should have lax vector conversions at all for RVV.
That appears to be contributing to llvm#64404

This patch at least fixes the obvious correctness issue.
This should be backported to LLVM 17.

Reviewed By: jacquesguan

Differential Revision: https://reviews.llvm.org/D157130
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 2, 2023
@topperc @razmser
[RISCV] Check type size for lax conversions between RVV builtin types…
3494efe 
… and VectorType::RVVFixedLengthDataVector.

This code was copied from SVE and modified for RVV. For SVE, there
is only one size for builtin types so they didn't need to check
the size. For RVV, due to LMUL there are 7 different sizes of builtin
types so we do need to check the size.

I'm not sure we should have lax vector conversions at all for RVV.
That appears to be contributing to llvm#64404

This patch at least fixes the obvious correctness issue.
This should be backported to LLVM 17.

Reviewed By: jacquesguan

Differential Revision: https://reviews.llvm.org/D157130
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 3, 2023
@topperc @razmser
[RISCV] Check type size for lax conversions between RVV builtin types…
22bb6bf 
… and VectorType::RVVFixedLengthDataVector.

This code was copied from SVE and modified for RVV. For SVE, there
is only one size for builtin types so they didn't need to check
the size. For RVV, due to LMUL there are 7 different sizes of builtin
types so we do need to check the size.

I'm not sure we should have lax vector conversions at all for RVV.
That appears to be contributing to llvm#64404

This patch at least fixes the obvious correctness issue.
This should be backported to LLVM 17.

Reviewed By: jacquesguan

Differential Revision: https://reviews.llvm.org/D157130
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 3, 2023
@topperc @razmser
[RISCV] Check type size for lax conversions between RVV builtin types…
6e6b826 
… and VectorType::RVVFixedLengthDataVector.

This code was copied from SVE and modified for RVV. For SVE, there
is only one size for builtin types so they didn't need to check
the size. For RVV, due to LMUL there are 7 different sizes of builtin
types so we do need to check the size.

I'm not sure we should have lax vector conversions at all for RVV.
That appears to be contributing to llvm#64404

This patch at least fixes the obvious correctness issue.
This should be backported to LLVM 17.

Reviewed By: jacquesguan

Differential Revision: https://reviews.llvm.org/D157130
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 6, 2023
@topperc @razmser
[RISCV] Check type size for lax conversions between RVV builtin types…
8ef60d7 
… and VectorType::RVVFixedLengthDataVector.

This code was copied from SVE and modified for RVV. For SVE, there
is only one size for builtin types so they didn't need to check
the size. For RVV, due to LMUL there are 7 different sizes of builtin
types so we do need to check the size.

I'm not sure we should have lax vector conversions at all for RVV.
That appears to be contributing to llvm#64404

This patch at least fixes the obvious correctness issue.
This should be backported to LLVM 17.

Reviewed By: jacquesguan

Differential Revision: https://reviews.llvm.org/D157130
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 11, 2023
@topperc @razmser
[RISCV] Check type size for lax conversions between RVV builtin types…
8f67c84 
… and VectorType::RVVFixedLengthDataVector.

This code was copied from SVE and modified for RVV. For SVE, there
is only one size for builtin types so they didn't need to check
the size. For RVV, due to LMUL there are 7 different sizes of builtin
types so we do need to check the size.

I'm not sure we should have lax vector conversions at all for RVV.
That appears to be contributing to llvm#64404

This patch at least fixes the obvious correctness issue.
This should be backported to LLVM 17.

Reviewed By: jacquesguan

Differential Revision: https://reviews.llvm.org/D157130
@EugeneZelenko EugeneZelenko added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Oct 26, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 26, 2023

@llvm/issue-subscribers-clang-frontend

Author: Garth Lei (garthlei)

Code: ``` c #include <riscv_vector.h>

typedef vint64m1_t attribute((riscv_rvv_vector_bits(128))) vint64m1_128_t;

int main() {
vint64m1_t a;
vint64m1_128_t b;
vint64m1_t c = __riscv_vand(a, 0, 0);
vint64m1_t d = __riscv_vand(b, 0, 0);
}


Compiler message (`clang -march=rv64gcv -mrvv-vector-bits=128 -S test.c`):

test.c:9:18: error: call to '__riscv_vand' is ambiguous
9 | vint64m1_t d = __riscv_vand(b, 0, 0);
| ^~~~~~~~~~~~
test.c:9:18: note: candidate function
test.c:9:18: note: candidate function
test.c:9:18: note: candidate function
test.c:9:18: note: candidate function
test.c:9:18: note: candidate function
test.c:9:18: note: candidate function
1 error generated.

</details>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
backend:RISC-V clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@EugeneZelenko @topperc @garthlei @llvmbot