Re: [RFC PATCH] RISC-V: Remove f{r,s}flags builtins

2023-11-29 Thread Philipp Tomsich 
These build-ins are used internally for the
TARGET_ATOMIC_ASSIGN_EXPAND_FENV expansion (and therefore can not be
removed):

/* Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV.  */

void
riscv_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
{
  if (!(TARGET_HARD_FLOAT || TARGET_ZFINX))
return;

  tree frflags = GET_BUILTIN_DECL (CODE_FOR_riscv_frflags);
  tree fsflags = GET_BUILTIN_DECL (CODE_FOR_riscv_fsflags);
  tree old_flags = create_tmp_var_raw (RISCV_ATYPE_USI);

  *hold = build4 (TARGET_EXPR, RISCV_ATYPE_USI, old_flags,
  build_call_expr (frflags, 0), NULL_TREE, NULL_TREE);
  *clear = build_call_expr (fsflags, 1, old_flags);
  *update = NULL_TREE;
}


On Wed, 29 Nov 2023 at 20:58, Christoph Müllner
 wrote:
>
> On Wed, Nov 29, 2023 at 8:24 PM Patrick O'Neill  wrote:
> >
> > Hi Christoph,
> >
> > The precommit-ci is seeing a large number of ICE segmentation faults as a 
> > result of this patch:
> > https://github.com/ewlu/gcc-precommit-ci/issues/796#issuecomment-1831853523
> >
> > The failures aren't in riscv.exp testsuite files so that's likely why you 
> > didn't run into them in your testing.
>
> Oh, I see.
> Then keeping things like they are is probably the best idea.
> Sorry for the noise!
>
> BR
> Christoph
>
> >
> > Debug log:
> >
> > /home/runner/work/gcc-precommit-ci/gcc-precommit-ci/riscv-gnu-toolchain/gcc/gcc/testsuite/gcc.dg/c11-atomic-2.c:110:3:
> >  internal compiler error: Segmentation fault
> > 0x133afb3 crash_signal
> > ../../../gcc/gcc/toplev.cc:316
> > 0x1678d1f contains_struct_check(tree_node*, tree_node_structure_enum, char 
> > const*, int, char const*)
> > ../../../gcc/gcc/tree.h:3747
> > 0x1678d1f build_call_expr_loc_array(unsigned int, tree_node*, int, 
> > tree_node**)
> > ../../../gcc/gcc/tree.cc:10815
> > 0x1679043 build_call_expr(tree_node*, int, ...)
> > ../../../gcc/gcc/tree.cc:10865
> > 0x17f816e riscv_atomic_assign_expand_fenv(tree_node**, tree_node**, 
> > tree_node**)
> > ../../../gcc/gcc/config/riscv/riscv-builtins.cc:420
> > 0xc5209b build_atomic_assign
> > ../../../gcc/gcc/c/c-typeck.cc:4289
> > 0xc60a47 build_modify_expr(unsigned int, tree_node*, tree_node*, tree_code, 
> > unsigned int, tree_node*, tree_node*)
> > ../../../gcc/gcc/c/c-typeck.cc:6406
> > 0xc85a61 c_parser_expr_no_commas
> > ../../../gcc/gcc/c/c-parser.cc:9112
> > 0xc85db1 c_parser_expression
> > ../../../gcc/gcc/c/c-parser.cc:12725
> > 0xc862bb c_parser_expression_conv
> > ../../../gcc/gcc/c/c-parser.cc:12765
> > 0xca3607 c_parser_statement_after_labels
> > ../../../gcc/gcc/c/c-parser.cc:7755
> > 0xc9f27e c_parser_compound_statement_nostart
> > ../../../gcc/gcc/c/c-parser.cc:7242
> > 0xc9f804 c_parser_compound_statement
> > ../../../gcc/gcc/c/c-parser.cc:6527
> > 0xca359c c_parser_statement_after_labels
> > ../../../gcc/gcc/c/c-parser.cc:7590
> > 0xca5713 c_parser_statement
> > ../../../gcc/gcc/c/c-parser.cc:7561
> > 0xca5713 c_parser_c99_block_statement
> > ../../../gcc/gcc/c/c-parser.cc:7820
> > 0xca6a2c c_parser_do_statement
> > ../../../gcc/gcc/c/c-parser.cc:8194
> > 0xca3d51 c_parser_statement_after_labels
> > ../../../gcc/gcc/c/c-parser.cc:7605
> > 0xc9f27e c_parser_compound_statement_nostart
> > ../../../gcc/gcc/c/c-parser.cc:7242
> > 0xc9f804 c_parser_compound_statement
> > ../../../gcc/gcc/c/c-parser.cc:6527
> > Please submit a full bug report, with preprocessed source (by using 
> > -freport-bug).
> > Please include the complete backtrace with any bug report.
> > See  for instructions.
> > compiler exited with status 1
> > FAIL: gcc.dg/c11-atomic-2.c (internal compiler error: Segmentation fault)
> >
> > Let me know if you need any additional info/investigation from me.
> >
> > Thanks,
> > Patrick
> >
> > On 11/29/23 03:49, Christoph Muellner wrote:
> >
> > From: Christoph Müllner 
> >
> > We have two builtins which are undocumented and have no known users.
> > Further, they don't exist in LLVM (so are no portable).
> > This means they are in an unclear state of being supported or not.
> > Let's remove them get them out of this undecided state.
> >
> > A discussion about making these builtins available in all
> > compilers was held many years ago with the decision to
> > not document them in the RISC-V C API documentation:
> >   https://github.com/riscv-non-isa/riscv-c-api-doc/pull/3
> >
> > This is an RFC patch as this breaks existing code that uses
> > these builtins, even if we don't know if such code exists.
> >
> > An alternative to this patch would be to document them
> > in gcc/doc/extend.texi (like has been done with __builtin_riscv_pause)
> > and put them into a supported state.
> >
> > This patch removes two tests for these builtins.
> > A test of this patch did not trigger any regressions in riscv.exp.
> >
> > Signed-off-by: Christoph Müllner 
> >
> > gcc/ChangeLog:
> >
> > * config/riscv/riscv-builtins.cc: Remove the builtins
> > __builtin_riscv_frflags and __builtin_riscv_fsflags.
> >
> > gcc/testsuite/ChangeLog:
>

Re: [RFC PATCH] RISC-V: Remove f{r,s}flags builtins

2023-11-29 Thread Christoph Müllner 
On Wed, Nov 29, 2023 at 8:24 PM Patrick O'Neill  wrote:
>
> Hi Christoph,
>
> The precommit-ci is seeing a large number of ICE segmentation faults as a 
> result of this patch:
> https://github.com/ewlu/gcc-precommit-ci/issues/796#issuecomment-1831853523
>
> The failures aren't in riscv.exp testsuite files so that's likely why you 
> didn't run into them in your testing.

Oh, I see.
Then keeping things like they are is probably the best idea.
Sorry for the noise!

BR
Christoph

>
> Debug log:
>
> /home/runner/work/gcc-precommit-ci/gcc-precommit-ci/riscv-gnu-toolchain/gcc/gcc/testsuite/gcc.dg/c11-atomic-2.c:110:3:
>  internal compiler error: Segmentation fault
> 0x133afb3 crash_signal
> ../../../gcc/gcc/toplev.cc:316
> 0x1678d1f contains_struct_check(tree_node*, tree_node_structure_enum, char 
> const*, int, char const*)
> ../../../gcc/gcc/tree.h:3747
> 0x1678d1f build_call_expr_loc_array(unsigned int, tree_node*, int, 
> tree_node**)
> ../../../gcc/gcc/tree.cc:10815
> 0x1679043 build_call_expr(tree_node*, int, ...)
> ../../../gcc/gcc/tree.cc:10865
> 0x17f816e riscv_atomic_assign_expand_fenv(tree_node**, tree_node**, 
> tree_node**)
> ../../../gcc/gcc/config/riscv/riscv-builtins.cc:420
> 0xc5209b build_atomic_assign
> ../../../gcc/gcc/c/c-typeck.cc:4289
> 0xc60a47 build_modify_expr(unsigned int, tree_node*, tree_node*, tree_code, 
> unsigned int, tree_node*, tree_node*)
> ../../../gcc/gcc/c/c-typeck.cc:6406
> 0xc85a61 c_parser_expr_no_commas
> ../../../gcc/gcc/c/c-parser.cc:9112
> 0xc85db1 c_parser_expression
> ../../../gcc/gcc/c/c-parser.cc:12725
> 0xc862bb c_parser_expression_conv
> ../../../gcc/gcc/c/c-parser.cc:12765
> 0xca3607 c_parser_statement_after_labels
> ../../../gcc/gcc/c/c-parser.cc:7755
> 0xc9f27e c_parser_compound_statement_nostart
> ../../../gcc/gcc/c/c-parser.cc:7242
> 0xc9f804 c_parser_compound_statement
> ../../../gcc/gcc/c/c-parser.cc:6527
> 0xca359c c_parser_statement_after_labels
> ../../../gcc/gcc/c/c-parser.cc:7590
> 0xca5713 c_parser_statement
> ../../../gcc/gcc/c/c-parser.cc:7561
> 0xca5713 c_parser_c99_block_statement
> ../../../gcc/gcc/c/c-parser.cc:7820
> 0xca6a2c c_parser_do_statement
> ../../../gcc/gcc/c/c-parser.cc:8194
> 0xca3d51 c_parser_statement_after_labels
> ../../../gcc/gcc/c/c-parser.cc:7605
> 0xc9f27e c_parser_compound_statement_nostart
> ../../../gcc/gcc/c/c-parser.cc:7242
> 0xc9f804 c_parser_compound_statement
> ../../../gcc/gcc/c/c-parser.cc:6527
> Please submit a full bug report, with preprocessed source (by using 
> -freport-bug).
> Please include the complete backtrace with any bug report.
> See  for instructions.
> compiler exited with status 1
> FAIL: gcc.dg/c11-atomic-2.c (internal compiler error: Segmentation fault)
>
> Let me know if you need any additional info/investigation from me.
>
> Thanks,
> Patrick
>
> On 11/29/23 03:49, Christoph Muellner wrote:
>
> From: Christoph Müllner 
>
> We have two builtins which are undocumented and have no known users.
> Further, they don't exist in LLVM (so are no portable).
> This means they are in an unclear state of being supported or not.
> Let's remove them get them out of this undecided state.
>
> A discussion about making these builtins available in all
> compilers was held many years ago with the decision to
> not document them in the RISC-V C API documentation:
>   https://github.com/riscv-non-isa/riscv-c-api-doc/pull/3
>
> This is an RFC patch as this breaks existing code that uses
> these builtins, even if we don't know if such code exists.
>
> An alternative to this patch would be to document them
> in gcc/doc/extend.texi (like has been done with __builtin_riscv_pause)
> and put them into a supported state.
>
> This patch removes two tests for these builtins.
> A test of this patch did not trigger any regressions in riscv.exp.
>
> Signed-off-by: Christoph Müllner 
>
> gcc/ChangeLog:
>
> * config/riscv/riscv-builtins.cc: Remove the builtins
> __builtin_riscv_frflags and __builtin_riscv_fsflags.
>
> gcc/testsuite/ChangeLog:
>
> * g++.target/riscv/frflags.C: Removed.
> * gcc.target/riscv/fsflags.c: Removed.
> ---
>  gcc/config/riscv/riscv-builtins.cc   |  2 --
>  gcc/testsuite/g++.target/riscv/frflags.C |  7 ---
>  gcc/testsuite/gcc.target/riscv/fsflags.c | 16 
>  3 files changed, 25 deletions(-)
>  delete mode 100644 gcc/testsuite/g++.target/riscv/frflags.C
>  delete mode 100644 gcc/testsuite/gcc.target/riscv/fsflags.c
>
> diff --git a/gcc/config/riscv/riscv-builtins.cc 
> b/gcc/config/riscv/riscv-builtins.cc
> index fc3976f3ba1..1655492b246 100644
> --- a/gcc/config/riscv/riscv-builtins.cc
> +++ b/gcc/config/riscv/riscv-builtins.cc
> @@ -188,8 +188,6 @@ static const struct riscv_builtin_description 
> riscv_builtins[] = {
>#include "riscv-scalar-crypto.def"
>#include "corev.def"
>
> -  DIRECT_BUILTIN (frflags, RISCV_USI_FTYPE, hard_float),
> -  DIRECT_NO_TARGET_BUILTIN (fsflags, RISCV_VOID_FTYPE_USI, hard_float),
>RISCV_BUILTIN (pause, "pause",

Re: [RFC PATCH] RISC-V: Remove f{r,s}flags builtins

2023-11-29 Thread Patrick O'Neill 

Hi Christoph,

The precommit-ci is seeing a large number of ICE segmentation faults as 
a result of this patch:

https://github.com/ewlu/gcc-precommit-ci/issues/796#issuecomment-1831853523

The failures aren't in riscv.exp testsuite files so that's likely why 
you didn't run into them in your testing.


Debug log:

/home/runner/work/gcc-precommit-ci/gcc-precommit-ci/riscv-gnu-toolchain/gcc/gcc/testsuite/gcc.dg/c11-atomic-2.c:110:3:
 internal compiler error: Segmentation fault
0x133afb3 crash_signal
../../../gcc/gcc/toplev.cc:316
0x1678d1f contains_struct_check(tree_node*, tree_node_structure_enum, char 
const*, int, char const*)
../../../gcc/gcc/tree.h:3747
0x1678d1f build_call_expr_loc_array(unsigned int, tree_node*, int, tree_node**)
../../../gcc/gcc/tree.cc:10815
0x1679043 build_call_expr(tree_node*, int, ...)
../../../gcc/gcc/tree.cc:10865
0x17f816e riscv_atomic_assign_expand_fenv(tree_node**, tree_node**, tree_node**)
../../../gcc/gcc/config/riscv/riscv-builtins.cc:420
0xc5209b build_atomic_assign
../../../gcc/gcc/c/c-typeck.cc:4289
0xc60a47 build_modify_expr(unsigned int, tree_node*, tree_node*, tree_code, 
unsigned int, tree_node*, tree_node*)
../../../gcc/gcc/c/c-typeck.cc:6406
0xc85a61 c_parser_expr_no_commas
../../../gcc/gcc/c/c-parser.cc:9112
0xc85db1 c_parser_expression
../../../gcc/gcc/c/c-parser.cc:12725
0xc862bb c_parser_expression_conv
../../../gcc/gcc/c/c-parser.cc:12765
0xca3607 c_parser_statement_after_labels
../../../gcc/gcc/c/c-parser.cc:7755
0xc9f27e c_parser_compound_statement_nostart
../../../gcc/gcc/c/c-parser.cc:7242
0xc9f804 c_parser_compound_statement
../../../gcc/gcc/c/c-parser.cc:6527
0xca359c c_parser_statement_after_labels
../../../gcc/gcc/c/c-parser.cc:7590
0xca5713 c_parser_statement
../../../gcc/gcc/c/c-parser.cc:7561
0xca5713 c_parser_c99_block_statement
../../../gcc/gcc/c/c-parser.cc:7820
0xca6a2c c_parser_do_statement
../../../gcc/gcc/c/c-parser.cc:8194
0xca3d51 c_parser_statement_after_labels
../../../gcc/gcc/c/c-parser.cc:7605
0xc9f27e c_parser_compound_statement_nostart
../../../gcc/gcc/c/c-parser.cc:7242
0xc9f804 c_parser_compound_statement
../../../gcc/gcc/c/c-parser.cc:6527
Please submit a full bug report, with preprocessed source (by using 
-freport-bug).
Please include the complete backtrace with any bug report.
See  for instructions.
compiler exited with status 1
FAIL: gcc.dg/c11-atomic-2.c (internal compiler error: Segmentation fault)

Let me know if you need any additional info/investigation from me.

Thanks,
Patrick

On 11/29/23 03:49, Christoph Muellner wrote:

From: Christoph Müllner

We have two builtins which are undocumented and have no known users.
Further, they don't exist in LLVM (so are no portable).
This means they are in an unclear state of being supported or not.
Let's remove them get them out of this undecided state.

A discussion about making these builtins available in all
compilers was held many years ago with the decision to
not document them in the RISC-V C API documentation:
   https://github.com/riscv-non-isa/riscv-c-api-doc/pull/3

This is an RFC patch as this breaks existing code that uses
these builtins, even if we don't know if such code exists.

An alternative to this patch would be to document them
in gcc/doc/extend.texi (like has been done with __builtin_riscv_pause)
and put them into a supported state.

This patch removes two tests for these builtins.
A test of this patch did not trigger any regressions in riscv.exp.

Signed-off-by: Christoph Müllner

gcc/ChangeLog:

* config/riscv/riscv-builtins.cc: Remove the builtins
__builtin_riscv_frflags and __builtin_riscv_fsflags.

gcc/testsuite/ChangeLog:

* g++.target/riscv/frflags.C: Removed.
* gcc.target/riscv/fsflags.c: Removed.
---
  gcc/config/riscv/riscv-builtins.cc   |  2 --
  gcc/testsuite/g++.target/riscv/frflags.C |  7 ---
  gcc/testsuite/gcc.target/riscv/fsflags.c | 16 
  3 files changed, 25 deletions(-)
  delete mode 100644 gcc/testsuite/g++.target/riscv/frflags.C
  delete mode 100644 gcc/testsuite/gcc.target/riscv/fsflags.c

diff --git a/gcc/config/riscv/riscv-builtins.cc 
b/gcc/config/riscv/riscv-builtins.cc
index fc3976f3ba1..1655492b246 100644
--- a/gcc/config/riscv/riscv-builtins.cc
+++ b/gcc/config/riscv/riscv-builtins.cc
@@ -188,8 +188,6 @@ static const struct riscv_builtin_description 
riscv_builtins[] = {
#include "riscv-scalar-crypto.def"
#include "corev.def"
  
-  DIRECT_BUILTIN (frflags, RISCV_USI_FTYPE, hard_float),

-  DIRECT_NO_TARGET_BUILTIN (fsflags, RISCV_VOID_FTYPE_USI, hard_float),
RISCV_BUILTIN (pause, "pause", RISCV_BUILTIN_DIRECT_NO_TARGET, 
RISCV_VOID_FTYPE, hint_pause),
  };
  
diff --git a/gcc/testsuite/g++.target/riscv/frflags.C b/gcc/testsuite/g++.target/riscv/frflags.C

deleted file mode 100644

[RFC PATCH] RISC-V: Remove f{r,s}flags builtins

2023-11-29 Thread Christoph Muellner 
From: Christoph Müllner 

We have two builtins which are undocumented and have no known users.
Further, they don't exist in LLVM (so are no portable).
This means they are in an unclear state of being supported or not.
Let's remove them get them out of this undecided state.

A discussion about making these builtins available in all
compilers was held many years ago with the decision to
not document them in the RISC-V C API documentation:
  https://github.com/riscv-non-isa/riscv-c-api-doc/pull/3

This is an RFC patch as this breaks existing code that uses
these builtins, even if we don't know if such code exists.

An alternative to this patch would be to document them
in gcc/doc/extend.texi (like has been done with __builtin_riscv_pause)
and put them into a supported state.

This patch removes two tests for these builtins.
A test of this patch did not trigger any regressions in riscv.exp.

Signed-off-by: Christoph Müllner 

gcc/ChangeLog:

* config/riscv/riscv-builtins.cc: Remove the builtins
__builtin_riscv_frflags and __builtin_riscv_fsflags.

gcc/testsuite/ChangeLog:

* g++.target/riscv/frflags.C: Removed.
* gcc.target/riscv/fsflags.c: Removed.
---
 gcc/config/riscv/riscv-builtins.cc   |  2 --
 gcc/testsuite/g++.target/riscv/frflags.C |  7 ---
 gcc/testsuite/gcc.target/riscv/fsflags.c | 16 
 3 files changed, 25 deletions(-)
 delete mode 100644 gcc/testsuite/g++.target/riscv/frflags.C
 delete mode 100644 gcc/testsuite/gcc.target/riscv/fsflags.c

diff --git a/gcc/config/riscv/riscv-builtins.cc 
b/gcc/config/riscv/riscv-builtins.cc
index fc3976f3ba1..1655492b246 100644
--- a/gcc/config/riscv/riscv-builtins.cc
+++ b/gcc/config/riscv/riscv-builtins.cc
@@ -188,8 +188,6 @@ static const struct riscv_builtin_description 
riscv_builtins[] = {
   #include "riscv-scalar-crypto.def"
   #include "corev.def"
 
-  DIRECT_BUILTIN (frflags, RISCV_USI_FTYPE, hard_float),
-  DIRECT_NO_TARGET_BUILTIN (fsflags, RISCV_VOID_FTYPE_USI, hard_float),
   RISCV_BUILTIN (pause, "pause", RISCV_BUILTIN_DIRECT_NO_TARGET, 
RISCV_VOID_FTYPE, hint_pause),
 };
 
diff --git a/gcc/testsuite/g++.target/riscv/frflags.C 
b/gcc/testsuite/g++.target/riscv/frflags.C
deleted file mode 100644
index 6353044dcf7..000
--- a/gcc/testsuite/g++.target/riscv/frflags.C
+++ /dev/null
@@ -1,7 +0,0 @@
-/* { dg-options "-O2 -march=rv32if -mabi=ilp32f" } */
-/* { dg-do compile } */
-
-int f()
-{
-  return __builtin_riscv_frflags();
-}
diff --git a/gcc/testsuite/gcc.target/riscv/fsflags.c 
b/gcc/testsuite/gcc.target/riscv/fsflags.c
deleted file mode 100644
index 74a97b8a7c7..000
--- a/gcc/testsuite/gcc.target/riscv/fsflags.c
+++ /dev/null
@@ -1,16 +0,0 @@
-/* Verify that fsflags is using the correct register or immediate.  */
-/* { dg-do compile } */
-/* { dg-require-effective-target hard_float } */
-/* { dg-options "-O" } */
-
-void foo1 (int a)
-{
-   __builtin_riscv_fsflags(a);
-}
-void foo2 ()
-{
-   __builtin_riscv_fsflags(4);
-}
-
-/* { dg-final { scan-assembler-times "fsflags\t" 1 } } */
-/* { dg-final { scan-assembler-times "fsflagsi\t" 1 } } */
-- 
2.41.0