RISCV: Add vector psabi checking. by yanzhang-dev · Pull Request #376 · riscvarchive/riscv-gcc

yanzhang-dev · 2023-04-20T08:25:41Z

This patch adds support to check function's argument or return is vector type and throw warning.

gcc/ChangeLog:

* config/riscv/riscv.cc (riscv_legitimize_move): (riscv_vector_psabi_warnning): (riscv_arg_has_vector): (riscv_pass_in_vector_p): (riscv_get_arg_info):

gcc/testsuite/ChangeLog:

* gcc.target/riscv/vector-abi-1.c: New test.
* gcc.target/riscv/vector-abi-2.c: New test.

gcc/testsuite/gcc.target/riscv/vector-abi-2.c

gcc/config/riscv/riscv.cc

This patch adds support to check function's argument or return is vector type and throw warning if yes. gcc/ChangeLog: * config/riscv/riscv.cc (riscv_vector_psabi_warnning): (riscv_arg_has_vector): (riscv_pass_in_vector_p): (riscv_get_arg_info): gcc/testsuite/ChangeLog: * gcc.target/riscv/vector-abi-1.c: New test. * gcc.target/riscv/vector-abi-2.c: New test. * gcc.target/riscv/vector-abi-3.c: New test. * gcc.target/riscv/vector-abi-4.c: New test. * gcc.target/riscv/vector-abi-5.c: New test. Signed-off-by: Yanzhang Wang <yanzhang.wang@intel.com> Co-authored-by: Kito Cheng <kito.cheng@sifive.com>

zhongjuzhe · 2023-04-20T14:28:26Z

Not familiar with ABI. I think it's all kito's call :)

Incarnation-p-lee · 2023-04-23T15:01:51Z

gcc/config/riscv/riscv.cc

+  if (type && riscv_arg_has_vector (type) && !warned)
+    {
+      warning (OPT_Wpsabi, "ABI for the vector type is currently in experimental"
+	       "stage and may changes in the upcoming version of GCC.");


will the message become something like in experimentalstage and ... instead of in experimental stage and ... after joining?

Yes. You're right. Will modify it in the fix commit.

Cool, thank you!

gcc/config/riscv/riscv.cc

kito-cheng · 2023-04-24T02:39:59Z

gcc/config/riscv/riscv.cc

+   intrinsic vector type.  Because we can't get the decl for the params.  */
+
+static bool
+riscv_arg_has_vector_size_attribute (const_tree type)


riscv_scalable_vector_type_p

kito-cheng · 2023-04-24T02:40:56Z

gcc/config/riscv/riscv.cc

+	    if (!TYPE_P (TREE_TYPE (f)))
+	      break;
+
+	    /* If there's vector_size attribute, ignore it.  */


Ignore it if it's fixed length vector.

kito-cheng · 2023-04-24T02:41:30Z

gcc/config/riscv/riscv.cc

+
+	    /* If there's vector_size attribute, ignore it.  */
+	    if (VECTOR_TYPE_P (TREE_TYPE (f)))
+	      is_vector = !riscv_arg_has_vector_size_attribute (type);


riscv_arg_has_vector_size_attribute (TREE_TYPE (f))?

Thanks, you're right. It should be the field type.

kito-cheng · 2023-04-24T07:17:50Z

gcc/config/riscv/riscv.cc

+  tree size = TYPE_SIZE (type);
+  if (size && TREE_CODE (size) == INTEGER_CST)
+    return true;
+
+  /* For the data type like vint32m1_t, the size code is POLY_INT_CST.  */
+  return false;


The return value should be inverted after function rename :)

Oh understand. Could I know why the intrinsic vector type is named as scalable vector ?

that's the term defined in AArch64 SVE actually, and we follow to use that term in our backend too , e.g. riscv.cc:riscv_v_adjust_scalable_frame

kito-cheng · 2023-04-24T07:19:49Z

gcc/config/riscv/riscv.cc

+
+  if (type && riscv_arg_has_vector (type) && !warned)
+    {
+      warning (OPT_Wpsabi, "ABI for the vector type is currently in "


vector -> scalable vector, that should more clear that it's exclude fixed length vector.

yanzhang-dev · 2023-05-09T01:40:03Z

Hi @kito-cheng , The commit 51fbc2e tries to fix the warning on intrinsic APIs. I'm not sure whether it's a right solution ?

I think the problem is we want to distinguish below 3 scenarios,

vint32m1_t fun(xxx) -> warning on return type.
vint32m1_t a = fun(xxx) -> no warning
xxx fun(vint32m1_t) -> warning on arg.

Both three will call riscv_pass_by_reference and then call riscv_get_arg_info. So we can't put the checking in get_arg_info. My solution moves the checking into function_arg and function_value. It can successfully distinguish the three.

But there's another new issue. When compiling with -flto, the warning will disappear and will show when do the linking. So I skip the the lto in tests.

kito-cheng

Only one minor comment, you can go to mailing list after you address that :)

kito-cheng · 2023-06-08T15:36:43Z

gcc/config/riscv/riscv.h

   for a call to a function whose data type is FNTYPE.
   For a library call, FNTYPE is 0.  */

+void init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree, int);


Rename to riscv_init_cumulative_args and put into riscv-protos.h.

kito-cheng reviewed Apr 20, 2023

View reviewed changes

gcc/testsuite/gcc.target/riscv/vector-abi-2.c Show resolved Hide resolved

gcc/config/riscv/riscv.cc Outdated Show resolved Hide resolved

gcc/config/riscv/riscv.cc Outdated Show resolved Hide resolved

gcc/config/riscv/riscv.cc Show resolved Hide resolved

yanzhang-dev force-pushed the yanzhang/vector-psabi branch from bc09437 to 2c68135 Compare April 20, 2023 14:12

Yanzhang Wang added 2 commits April 23, 2023 20:37

fix: disable the warning on attribute vector_size.

211f26b

fix: code style.

1004381

Incarnation-p-lee reviewed Apr 23, 2023

View reviewed changes

fix: need to split the words with space.

f9bafb8

kito-cheng reviewed Apr 24, 2023

View reviewed changes

fix: pass the right field type and other comments.

cb6ae98

kito-cheng reviewed Apr 24, 2023

View reviewed changes

fix: address comments.

33484c5

fix: disable intrinsic warning.

a6c96b8

yanzhang-dev force-pushed the yanzhang/vector-psabi branch from 51fbc2e to a6c96b8 Compare May 22, 2023 14:12

kito-cheng reviewed Jun 8, 2023

View reviewed changes

Comments

Conversation

yanzhang-dev commented Apr 20, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zhongjuzhe commented Apr 20, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yanzhang-dev commented May 9, 2023

Uh oh!

kito-cheng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants