[RFC PATCH] RISC-V: Initial RV64E and LP64E support

2023-10-21 Thread Tsukasa OI
From: Tsukasa OI 

Along with RV32E, RV64E is ratified.  Though ILP32E and LP64E ABIs are
still draft, it's worth supporting it.

This commit should not be merged until two proposals below are
going to proceed.

LP64E proposal (including suggested changes):


New "__riscv_64e" proposal by the author of this commit:


gcc/ChangeLog:

* common/config/riscv/riscv-common.cc
(riscv_subset_list::parse_std_ext): Allow RV64E.
* config.gcc: Parse base ISA RV64E and ABI LP64E.
* config/riscv/arch-canonicalize: Parse base ISA 'rv64e'.
* config/riscv/riscv-c.cc (riscv_cpu_cpp_builtins):
Build different macro per RV32E/RV64E.
Add handling for ABI_LP64E.
* config/riscv/riscv-d.cc (riscv_d_handle_target_float_abi):
Add handling for ABI_LP64E.
* config/riscv/riscv-opts.h (enum riscv_abi_type): Add ABI_LP64E.
* config/riscv/riscv.cc (riscv_option_override): Enhance error
handling to support RV64E and LP64E.
(riscv_conditional_register_usage): Change "RV32E" in a comment
to "RV32E/RV64E".
* config/riscv/riscv.h
(UNITS_PER_FP_ARG): Add handling for ABI_LP64E.
(STACK_BOUNDARY): Ditto.
(ABI_STACK_BOUNDARY): Ditto.
(MAX_ARGS_IN_REGISTERS): Ditto.
(ABI_SPEC): Add support for "lp64e".
* config/riscv/riscv.opt: Parse -mabi=lp64e as ABI_LP64E.
* doc/invoke.texi: Add documentation of the LP64E ABI.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/predef-1.c: Test for __riscv_64e.
* gcc.target/riscv/predef-2.c: Ditto.
* gcc.target/riscv/predef-3.c: Ditto.
* gcc.target/riscv/predef-4.c: Ditto.
* gcc.target/riscv/predef-5.c: Ditto.
* gcc.target/riscv/predef-6.c: Ditto.
* gcc.target/riscv/predef-7.c: Ditto.
* gcc.target/riscv/predef-8.c: Ditto.
* gcc.target/riscv/predef-9.c: New test for RV32E and LP64E,
based on predef-7.c.
---
 gcc/common/config/riscv/riscv-common.cc   |  2 +-
 gcc/config.gcc| 10 ++--
 gcc/config/riscv/arch-canonicalize|  2 +-
 gcc/config/riscv/riscv-c.cc   |  3 +-
 gcc/config/riscv/riscv-d.cc   |  1 +
 gcc/config/riscv/riscv-opts.h |  1 +
 gcc/config/riscv/riscv.cc | 19 ---
 gcc/config/riscv/riscv.h  | 17 --
 gcc/config/riscv/riscv.opt|  3 ++
 gcc/doc/invoke.texi   |  7 +--
 gcc/testsuite/gcc.target/riscv/predef-1.c |  3 ++
 gcc/testsuite/gcc.target/riscv/predef-2.c |  3 ++
 gcc/testsuite/gcc.target/riscv/predef-3.c |  3 ++
 gcc/testsuite/gcc.target/riscv/predef-4.c |  3 ++
 gcc/testsuite/gcc.target/riscv/predef-5.c |  3 ++
 gcc/testsuite/gcc.target/riscv/predef-6.c |  3 ++
 gcc/testsuite/gcc.target/riscv/predef-7.c |  3 ++
 gcc/testsuite/gcc.target/riscv/predef-8.c |  3 ++
 gcc/testsuite/gcc.target/riscv/predef-9.c | 66 +++
 19 files changed, 134 insertions(+), 21 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/predef-9.c

diff --git a/gcc/common/config/riscv/riscv-common.cc 
b/gcc/common/config/riscv/riscv-common.cc
index 526dbb7603be..14e0bcfc6b5e 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -963,7 +963,7 @@ riscv_subset_list::parse_std_ext (const char *p)
 
   add ("e", major_version, minor_version, explicit_version_p, false);
 
-  if (m_xlen > 32)
+  if (m_xlen > 64)
{
  error_at (m_loc, "%<-march=%s%>: rv%de is not a valid base ISA",
m_arch, m_xlen);
diff --git a/gcc/config.gcc b/gcc/config.gcc
index 0782cbc6e915..0f568654ad64 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -4684,7 +4684,7 @@ case "${target}" in
 
# Infer arch from --with-arch, --target, and --with-abi.
case "${with_arch}" in
-   rv32e* | rv32i* | rv32g* | rv64i* | rv64g*)
+   rv32e* | rv32i* | rv32g* | rv64e* | rv64i* | rv64g*)
# OK.
;;
"")
@@ -4693,11 +4693,12 @@ case "${target}" in
ilp32e) with_arch="rv32e" ;;
ilp32 | ilp32f | ilp32d) with_arch="rv32gc" ;;
lp64 | lp64f | lp64d) with_arch="rv64gc" ;;
+   lp64e) with_arch="rv64e" ;;
*) with_arch="rv${xlen}gc" ;;
esac
;;
*)
-   echo "--with-arch=${with_arch} is not supported.  The 
argument must begin with rv32e, rv32i, rv32g, rv64i, or rv64g." 1>&2
+   echo "--with-arch=${with_arch} is not supported.  The 
argument must begin with rv32e, rv32i, rv32g, rv64e, rv64i, or rv64g." 1>&2

Re: [PATCH] RISC-V: Prohibit combination of 'E' and 'H'

2023-10-21 Thread Tsukasa OI
On 2023/10/22 3:04, Jeff Law wrote:
> 
> 
> On 10/20/23 23:32, Tsukasa OI wrote:
>> From: Tsukasa OI 
>>
>> According to the ratified privileged specification (version 20211203),
>> it says:
>>
>>> The hypervisor extension depends on an "I" base integer ISA with 32 x
>>> registers (RV32I or RV64I), not RV32E, which has only 16 x registers.
>>
>> Also in the latest draft, it also prohibits RV64E with the 'H' extension.
>> This commit prohibits the combination of 'E' and 'H' extensions.
>>
>> gcc/ChangeLog:
>>
>> * common/config/riscv/riscv-common.cc (riscv_subset_list::parse):
>> Prohibit 'E' and 'H' combinations.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/riscv/arch-26.c: New test.
> In a similar vein, GCC doesn't really care about the privileged
> extensions.  So this won't really affect code generation.  So I'll ACK,
> but going forward let's start doing the regression test.  If you need
> help setting that up, I'm sure someone here can make suggestions.
> Personally I prefer a qemu+binfmt setup as it doesn't require setting up
> a board file and explicitly calling the simulator, ie, it looks a lot
> like native testing.
> 
> jeff
> 

Thanks for reviewing.  I'll commit two patches soon.

Yes, for GCC, privileged extensions (and version numbers) are not
important in general (unless toolchain conventions create privileged
built-in functions).

Intents of my two small patch sets are:

1. Allow inline assembly to use new/privileged extensions.
2. Allow/disallow same -march for both CC and AS (as possible).
3. (As long as no major compatibility breakage happens),
   make both GCC and Binutils faithful to the specification
   and the current status (that would also improve interoperability).

Hmm, I generally agree with your opinion and I made a board file for
DejaGnu (running qemu-riscv64) to run "make check-gcc
RUNTESTFLAGS='--target_board=riscv-sim riscv.exp'" because it already
contains many execute tests (and annoys me if I don't do that).

What I'm not sure is, what kind of regression tests we need?

(In my mind)
Level 1: Make nearly empty program with specific -march (and optionally
 -mabi?) and make sure that it works.
Level 2: Make a program with inline assembly and execute tests with
 specific configurations (with specific -march and -mabi)
 [I'm not sure how to write **and optionally execute tests**]

I would like to hear your thoughts.

Thanks,
Tsukasa


[PATCHv2] move the (a-b) CMP 0 ? (a-b) : (b-a) optimization from fold_cond_expr_with_comparison to match

2023-10-21 Thread Andrew Pinski
From: Andrew Pinski 

This patch moves the `(a-b) CMP 0 ? (a-b) : (b-a)` optimization
from fold_cond_expr_with_comparison to match.

Bootstrapped and tested on x86_64-linux-gnu.

Changes in:
v2: Removes `(a == b) ? 0 : (b - a)` handling since it was handled
via r14-3606-g3d86e7f4a8ae
Change zerop to integer_zerop for `(a - b) == 0 ? 0 : (b - a)`,
Add `(a - b) != 0 ? (a - b) : 0` handling.

gcc/ChangeLog:

* match.pd (`(A - B) CMP 0 ? (A - B) : (B - A)`):
New patterns.

gcc/testsuite/ChangeLog:

* gcc.dg/tree-ssa/phi-opt-38.c: New test.
---
 gcc/match.pd   | 46 --
 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-38.c | 45 +
 2 files changed, 88 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-38.c

diff --git a/gcc/match.pd b/gcc/match.pd
index a56838fb388..ce8d159d260 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -5650,9 +5650,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (cnd (logical_inverted_value truth_valued_p@0) @1 @2)
   (cnd @0 @2 @1)))
 
-/* abs/negative simplifications moved from fold_cond_expr_with_comparison,
-   Need to handle (A - B) case as fold_cond_expr_with_comparison does.
-   Need to handle UN* comparisons.
+/* abs/negative simplifications moved from fold_cond_expr_with_comparison.
 
None of these transformations work for modes with signed
zeros.  If A is +/-0, the first two transformations will
@@ -5717,6 +5715,48 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(convert (negate (absu:utype @0
(negate (abs @0)
  )
+
+ /* (A - B) == 0 ? (A - B) : (B - A)same as (B - A) */
+ (for cmp (eq uneq)
+  (simplify
+   (cnd (cmp (minus@0 @1 @2) zerop) @0 (minus@3 @2 @1))
+   (if (!HONOR_SIGNED_ZEROS (type))
+@3))
+  (simplify
+   (cnd (cmp (minus@0 @1 @2) integer_zerop) integer_zerop (minus@3 @2 @1))
+   @3)
+ )
+ /* (A - B) != 0 ? (A - B) : (B - A)same as (A - B) */
+ (for cmp (ne ltgt)
+  (simplify
+   (cnd (cmp (minus@0 @1 @2) zerop) @0 (minus @2 @1))
+   (if (!HONOR_SIGNED_ZEROS (type))
+@0))
+  (simplify
+   (cnd (cmp (minus@0 @1 @2) integer_zerop) @0 integer_zerop)
+   @0)
+ )
+ /* (A - B) >=/> 0 ? (A - B) : (B - A)same as abs (A - B) */
+ (for cmp (ge gt)
+  (simplify
+   (cnd (cmp (minus@0 @1 @2) zerop) @0 (minus @2 @1))
+   (if (!HONOR_SIGNED_ZEROS (type)
+   && !TYPE_UNSIGNED (type))
+(abs @0
+ /* (A - B) <=/< 0 ? (A - B) : (B - A)same as -abs (A - B) */
+ (for cmp (le lt)
+  (simplify
+   (cnd (cmp (minus@0 @1 @2) zerop) @0 (minus @2 @1))
+   (if (!HONOR_SIGNED_ZEROS (type)
+   && !TYPE_UNSIGNED (type))
+(if (ANY_INTEGRAL_TYPE_P (type)
+&& !TYPE_OVERFLOW_WRAPS (type))
+ (with {
+tree utype = unsigned_type_for (type);
+  }
+  (convert (negate (absu:utype @0
+  (negate (abs @0)
+ )
 )
 
 /* -(type)!A -> (type)A - 1.  */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-38.c 
b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-38.c
new file mode 100644
index 000..0f0e3170f8d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-38.c
@@ -0,0 +1,45 @@
+/* { dg-options "-O2 -fno-signed-zeros -fdump-tree-phiopt" } */
+int minus1(int a, int b)
+{
+  int c = a - b;
+  if (c == 0) c = b - a;
+  return c;
+}
+int minus2(int a, int b)
+{
+  int c = a - b;
+  if (c != 0) c = b - a;
+  return c;
+}
+int minus3(int a, int b)
+{
+  int c = a - b;
+  if (c == 0) c = 0;
+  else c = b - a;
+  return c;
+}
+int minus4(int a, int b)
+{
+  int c;
+  if (a == b) c = 0;
+  else
+c = b - a;
+  return c;
+}
+int abs0(int a, int b)
+{
+  int c = a - b;
+  if (c <= 0) c = b - a;
+  return c;
+}
+int negabs(int a, int b)
+{
+  int c = a - b;
+  if (c >= 0) c = b - a;
+  return c;
+}
+
+/* The above should be optimized at phiopt1 except for negabs which has to wait
+  until phiopt2 as -abs is not acceptable in early phiopt.  */
+/* { dg-final { scan-tree-dump-times "if" 1  "phiopt1"  } } */
+/* { dg-final { scan-tree-dump-not "if" "phiopt2" } } */
-- 
2.39.3



Re: [PATCH] move the (a-b) CMP 0 ? (a-b) : (b-a) optimization from fold_cond_expr_with_comparison to match

2023-10-21 Thread Andrew Pinski
On Thu, Oct 19, 2023 at 10:13 PM Andrew Pinski  wrote:
>
> On Mon, Jul 12, 2021 at 4:47 AM Richard Biener via Gcc-patches
>  wrote:
> >
> > On Sun, Jul 11, 2021 at 4:12 AM apinski--- via Gcc-patches
> >  wrote:
> > >
> > > From: Andrew Pinski 
> > >
> > > This patch moves the (a-b) CMP 0 ? (a-b) : (b-a) optimization
> > > from fold_cond_expr_with_comparison to match.
> >
> > So I searched and I guess these transforms are produced from
> >
> >   /* If we have A op 0 ? A : -A, consider applying the following
> >  transformations:
> >
> >  A == 0? A : -Asame as -A
> >  A != 0? A : -Asame as A
> >  A >= 0? A : -Asame as abs (A)
> >  A > 0?  A : -Asame as abs (A)
> >  A <= 0? A : -Asame as -abs (A)
> >  A < 0?  A : -Asame as -abs (A)
> >
> >  None of these transformations work for modes with signed
> >  zeros.  If A is +/-0, the first two transformations will
> >  change the sign of the result (from +0 to -0, or vice
> >  versa).  The last four will fix the sign of the result,
> >  even though the original expressions could be positive or
> >  negative, depending on the sign of A.
> >
> >  Note that all these transformations are correct if A is
> >  NaN, since the two alternatives (A and -A) are also NaNs.  */
> >   if (!HONOR_SIGNED_ZEROS (type)
> >   && (FLOAT_TYPE_P (TREE_TYPE (arg01))
> >   ? real_zerop (arg01)
> >   : integer_zerop (arg01))
> >   && ((TREE_CODE (arg2) == NEGATE_EXPR
> >&& operand_equal_p (TREE_OPERAND (arg2, 0), arg1, 0))
> >  /* In the case that A is of the form X-Y, '-A' (arg2) may
> > have already been folded to Y-X, check for that. */
> >   || (TREE_CODE (arg1) == MINUS_EXPR
> >   && TREE_CODE (arg2) == MINUS_EXPR
> >   && operand_equal_p (TREE_OPERAND (arg1, 0),
> >   TREE_OPERAND (arg2, 1), 0)
> >   && operand_equal_p (TREE_OPERAND (arg1, 1),
> >   TREE_OPERAND (arg2, 0), 0
> > ...
> >
> > I wonder at which point we can remove the code from fold-const.c?
>
> I have to double check if after an updated patch, if that code does
> anything that match does not do.
> I will do that before I submit an updated patch.

I looked and the main thing left is solving the stripping of sign nops
that happen at the beginning of fold_cond_expr_with_comparison.
I did solve part of that with the recent
r14-4662-gc7609acb8a8210188d21b2cd7 but not with this new patterns; I
will solve that in a separate patch.

Thanks,
Andrew Pinski

>
> >
> > Some comments inline below.
> >
> > > OK? Bootstrapped and tested on x86_64-linux-gnu.
> > >
> > > gcc/ChangeLog:
> > >
> > > * match.pd ((A-B) CMP 0 ? (A-B) : (B - A)):
> > > New patterns.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * gcc.dg/tree-ssa/phi-opt-25.c: New test.
> > > ---
> > >  gcc/match.pd   | 48 --
> > >  gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25.c | 45 
> > >  2 files changed, 90 insertions(+), 3 deletions(-)
> > >  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25.c
> > >
> > > diff --git a/gcc/match.pd b/gcc/match.pd
> > > index 30680d488ab..aa88381fdcb 100644
> > > --- a/gcc/match.pd
> > > +++ b/gcc/match.pd
> > > @@ -4040,9 +4040,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > >(cnd (logical_inverted_value truth_valued_p@0) @1 @2)
> > >(cnd @0 @2 @1)))
> > >
> > > -/* abs/negative simplifications moved from 
> > > fold_cond_expr_with_comparison,
> > > -   Need to handle (A - B) case as fold_cond_expr_with_comparison does.
> > > -   Need to handle UN* comparisons.
> > > +/* abs/negative simplifications moved from 
> > > fold_cond_expr_with_comparison.
> > >
> > > None of these transformations work for modes with signed
> > > zeros.  If A is +/-0, the first two transformations will
> > > @@ -4098,6 +4096,50 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > > (convert (negate (absu:utype @0
> > > (negate (abs @0)
> > >   )
> > > +
> > > + /* (A - B) == 0 ? (A - B) : (B - A)same as (B - A) */
> > > + (for cmp (eq uneq)
> > > +  (simplify
> > > +   (cnd (cmp (minus@0 @1 @2) zerop) @0 (minus@3 @2 @1))
> > > +(if (!HONOR_SIGNED_ZEROS (type))
> > > + @3))
> > > +  (simplify
> > > +   (cnd (cmp (minus@0 @1 @2) zerop) integer_zerop (minus@3 @2 @1))
> >
> > So that makes me think why integer_zerop?  'type' should then be
> > integer and thus never HONOR_SIGNED_ZEROS.
>
> yes that should be done.
>
> >
> > Don't we also need the inverted condition case for completeness?
>
> Yes we should. Though for phiopt we don't.
>
>
> >
> > > +(if (!HONOR_SIGNED_ZEROS (type))
> > > + @3))
> > > +  (simplify
> > > +   (cnd (cmp @1 @2) integer_zerop (minus@3 @2 @1))
> >
> > I think this needs to be (cmp:c @1 @2)
>
> This is now actually handled already by 

Re: [PATCH v9 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces

2023-10-21 Thread rep . dot . nop
On 21 October 2023 01:56:16 CEST, Vineet Gupta  wrote:
>On 10/19/23 23:50, Ajit Agarwal wrote:
>> Hello All:
>> 
>> This version 9 of the patch uses abi interfaces to remove zero and sign 
>> extension elimination.
>> Bootstrapped and regtested on powerpc-linux-gnu.
>> 
>> In this version (version 9) of the patch following review comments are 
>> incorporated.
>> 
>> a) Removal of hard code zero_extend and sign_extend  in abi interfaces.
>> b) Source and destination with different registers are considered.
>> c) Further enhancements.
>> d) Added sign extension elimination using abi interfaces.
>
>As has been trend in the past, I don't think all the review comments have been 
>addressed.

And apart from that, may I ask if this is just me, or does anybody else think 
that it might be worthwhile to actually read a patch before (re-)posting?

Seeing e.g. the proposed abi_extension_candidate_p as written in a first POC 
would deserve some manual CSE, if nothing more then for clarity and conciseness?

Just curious from a meta perspective..

And:

>> ree: Improve ree pass for rs6000 target using defined abi interfaces

mentioning powerpc like this, and then changing generic code could be 
interpreted as misleading, IMHO.

>> 
>> For rs6000 target we see redundant zero and sign extension and done
>> to improve ree pass to eliminate such redundant zero and sign extension
>> using defined ABI interfaces.

Mentioning powerpc in the body as one of the affected target(s) is of course 
fine.


>>   +/* Return TRUE if target mode is equal to source mode of zero_extend
>> +   or sign_extend otherwise false.  */

, false otherwise.

But I'm not a native speaker 


>> +/* Return TRUE if the candidate insn is zero extend and regno is
>> +   a return registers.  */
>> +
>> +static bool
>> +abi_extension_candidate_return_reg_p (/*rtx_insn *insn, */int regno)

Leftover debug comment.

>> +{
>> +  if (targetm.calls.function_value_regno_p (regno))
>> +return true;
>> +
>> +  return false;
>> +}
>> +

As said, I don't see why the below was not cleaned up before the V1 submission.
Iff it breaks when manually CSEing, I'm curious why?

>> +/* Return TRUE if reg source operand of zero_extend is argument registers
>> +   and not return registers and source and destination operand are same
>> +   and mode of source and destination operand are not same.  */
>> +
>> +static bool
>> +abi_extension_candidate_p (rtx_insn *insn)
>> +{
>> +  rtx set = single_set (insn);
>> +  machine_mode dst_mode = GET_MODE (SET_DEST (set));
>> +  rtx orig_src = XEXP (SET_SRC (set), 0);
>> +
>> +  if (!FUNCTION_ARG_REGNO_P (REGNO (orig_src))
>> +  || abi_extension_candidate_return_reg_p (/*insn,*/ REGNO (orig_src)))

On top, debug leftover.

>> +return false;
>> +
>> +  /* Mode of destination and source should be different.  */
>> +  if (dst_mode == GET_MODE (orig_src))
>> +return false;
>> +
>> +  machine_mode mode = GET_MODE (XEXP (SET_SRC (set), 0));
>> +  bool promote_p = abi_target_promote_function_mode (mode);
>> +
>> +  /* REGNO of source and destination should be same if not
>> +  promoted.  */
>> +  if (!promote_p && REGNO (SET_DEST (set)) != REGNO (orig_src))
>> +return false;
>> +
>> +  return true;
>> +}
>> +

As said, please also rephrase the above (and everything else if it obviously 
looks akin the above).

The rest, mentioned below,  should largely be covered by following the coding 
convention.

>> +/* Return TRUE if the candidate insn is zero extend and regno is
>> +   an argument registers.  */

Singular register.

>> +
>> +static bool
>> +abi_extension_candidate_argno_p (/*rtx_code code, */int regno)

Debug leftover.
I would probably have inlined this function manually, with a respective comment.
Did not look how often it is used, admittedly.

>> +{
>> +  if (FUNCTION_ARG_REGNO_P (regno))
>> +return true;
>> +
>> +  return false;
>> +}
[]
>> +
>>   /* This function goes through all reaching defs of the source

s/This function goes/Go/

>>  of the candidate for elimination (CAND) and tries to combine

(of, ?didn't look) candidate CAND for eliminating

>>  the extension with the definition instruction.  The changes

defining

Pre-existing, I know.
But you could fix those in a preparatory patch while you touch surrounding code.
This is not a requirement, of course, just good habit, IMHO.

>> @@ -770,6 +889,11 @@ combine_reaching_defs (ext_cand *cand, const_rtx 
>> set_pat, ext_state *state)
>>   state->defs_list.truncate (0);
>> state->copies_list.truncate (0);
>> +  rtx orig_src = XEXP (SET_SRC (cand->expr),0);
>> +
>> +  if (abi_extension_candidate_p (cand->insn)
>> +  && (!get_defs (cand->insn, orig_src, NULL)))

Excess braces.
Hopefully check_gnu_style would have complained.

>> +return abi_handle_regs (cand->insn);
>>   outcome = make_defs_and_copies_lists (cand->insn, set_pat, state);
>>   @@ -1036,6 +1160,15 @@ combine_reaching_defs (ext_cand *cand, const_rtx 
>> set_pat, 

Re: Darwin: Replace environment runpath with embedded [PR88590]

2023-10-21 Thread Iain Sandoe
Hi Jeff,

> On 21 Oct 2023, at 19:05, Jeff Law  wrote:

> On 10/8/23 16:14, Iain Sandoe wrote:
>> + Jeff
>>> On 8 Oct 2023, at 14:07, Nathanael Nerode  wrote:
>>> 
>>> I hope a global maintainer can step up.  I've been on hiatus from GCC work 
>>> for some years, and this was never my part of the build system anyway -- 
>>> and I don't use Darwin -- so I'm not qualified to review it.  It looks fine 
>>> but it should be reviewed by someone who knows what they're doing.
>> Thanks Nathanael for taking a look,
>> @Jeff as we discussed at the Cauldron, I suspected it might be difficult to 
>> get this review, so would really appreciate if could cast an eye over it at 
>> some point,
> If you could forward the remaining patches it'd be helpful.  I think Alex 
> just acked one or more of these that affected the build machinery.

Yes, that’s now fine - it was the build system review that was missing (which 
Alex did, thanks!) - the other parts are already approved.

Iain



[PATCH] convert_to_complex vs invalid_conversion [PR111903]

2023-10-21 Thread Andrew Pinski
convert_to_complex when creating a COMPLEX_EXPR does
not currently check if either the real or imag parts
was not error_mark_node. This later on confuses the gimpilfier
when there was a SAVE_EXPR wrapped around that COMPLEX_EXPR.
The simple fix is after calling convert inside convert_to_complex_1,
check that the either result was an error_operand and return
an error_mark_node in that case.

Bootstrapped and tested on x86_64-linux-gnu with no regressions.

PR c/111903

gcc/ChangeLog:

* convert.cc (convert_to_complex_1): Return
error_mark_node if either convert was an error
when converting from a scalar.

gcc/testsuite/ChangeLog:

* gcc.target/i386/float16-8.c: New test.
---
 gcc/convert.cc|  9 +++--
 gcc/testsuite/gcc.target/i386/float16-8.c | 12 
 2 files changed, 19 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/float16-8.c

diff --git a/gcc/convert.cc b/gcc/convert.cc
index 80d86fe3708..ac6af7026a7 100644
--- a/gcc/convert.cc
+++ b/gcc/convert.cc
@@ -1006,8 +1006,13 @@ convert_to_complex_1 (tree type, tree expr, bool fold_p)
 case ENUMERAL_TYPE:
 case BOOLEAN_TYPE:
 case BITINT_TYPE:
-  return build2 (COMPLEX_EXPR, type, convert (subtype, expr),
-convert (subtype, integer_zero_node));
+  {
+   tree real = convert (subtype, expr);
+   tree imag = convert (subtype, integer_zero_node);
+   if (error_operand_p (real) || error_operand_p (imag))
+ return error_mark_node;
+   return build2 (COMPLEX_EXPR, type, real, imag);
+  }
 
 case COMPLEX_TYPE:
   {
diff --git a/gcc/testsuite/gcc.target/i386/float16-8.c 
b/gcc/testsuite/gcc.target/i386/float16-8.c
new file mode 100644
index 000..003f82e7146
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/float16-8.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-mno-sse" } */
+/* PR c/111903 */
+
+int i;
+_Float16 f;
+int bar(...);
+void
+foo (void)
+{
+  i /= bar ((_Complex _Float16) f); /* { dg-error "" } */
+}
-- 
2.39.3



Re: Darwin: Replace environment runpath with embedded [PR88590]

2023-10-21 Thread Jeff Law




On 10/8/23 16:14, Iain Sandoe wrote:

+ Jeff


On 8 Oct 2023, at 14:07, Nathanael Nerode  wrote:

I hope a global maintainer can step up.  I've been on hiatus from GCC work for 
some years, and this was never my part of the build system anyway -- and I 
don't use Darwin -- so I'm not qualified to review it.  It looks fine but it 
should be reviewed by someone who knows what they're doing.


Thanks Nathanael for taking a look,

@Jeff as we discussed at the Cauldron, I suspected it might be difficult to get 
this review, so would really appreciate if could cast an eye over it at some 
point,
If you could forward the remaining patches it'd be helpful.  I think 
Alex just acked one or more of these that affected the build machinery.


jeff


Re: [PATCH] RISC-V: Prohibit combination of 'E' and 'H'

2023-10-21 Thread Jeff Law




On 10/20/23 23:32, Tsukasa OI wrote:

From: Tsukasa OI 

According to the ratified privileged specification (version 20211203),
it says:


The hypervisor extension depends on an "I" base integer ISA with 32 x
registers (RV32I or RV64I), not RV32E, which has only 16 x registers.


Also in the latest draft, it also prohibits RV64E with the 'H' extension.
This commit prohibits the combination of 'E' and 'H' extensions.

gcc/ChangeLog:

* common/config/riscv/riscv-common.cc (riscv_subset_list::parse):
Prohibit 'E' and 'H' combinations.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/arch-26.c: New test.
In a similar vein, GCC doesn't really care about the privileged 
extensions.  So this won't really affect code generation.  So I'll ACK, 
but going forward let's start doing the regression test.  If you need 
help setting that up, I'm sure someone here can make suggestions. 
Personally I prefer a qemu+binfmt setup as it doesn't require setting up 
a board file and explicitly calling the simulator, ie, it looks a lot 
like native testing.


jeff


Re: [PATCH] RISC-V: 'Zfa' extension is now ratified

2023-10-21 Thread Jeff Law




On 10/20/23 23:32, Tsukasa OI wrote:

From: Tsukasa OI 

Since this extension is ratified, it now has the version number 1.0.

Reference:


gcc/ChangeLog:

* common/config/riscv/riscv-common.cc (riscv_ext_version_table):
Change version number of the 'Zfa' extension to 1.0.
Note that it should be standard practice to run a build & regression 
test for every change.  However, I don't think anything in GCC actually 
cares about the version number of the specs.  So I'll ACK without the 
usual testing.


OK for the trunk.

jeff


Re: [PATCH] libstdc++: Ensure active union member is correctly set

2023-10-21 Thread Jonathan Wakely
On Fri, 29 Sept 2023 at 17:46, Jonathan Wakely  wrote:
>
> On Fri, 29 Sept 2023 at 17:29, Nathaniel Shead
>  wrote:
> >
> > On Fri, Sep 29, 2023 at 04:06:33PM +0100, Jonathan Wakely wrote:
> > > On Fri, 29 Sept 2023 at 10:32, Jonathan Wakely  wrote:
> > > > > Thanks for the comments, here's an updated version of the patch.
> > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu.
> > > >
> > > > Great, I'll get this committed today - thanks!
> > >
> > > That's done now.
> > >
> >
> > Thanks!
> >
> > > > >
> > > > > I'll note that there are some existing calls to `_M_use_local_data()`
> > > > > already used only for their side effects without a cast to void, e.g.
> > > > >
> > > > >   /**
> > > > >*  @brief  Default constructor creates an empty string.
> > > > >*/
> > > > >   _GLIBCXX20_CONSTEXPR
> > > > >   basic_string()
> > > > >   
> > > > > _GLIBCXX_NOEXCEPT_IF(is_nothrow_default_constructible<_Alloc>::value)
> > > > >   : _M_dataplus(_M_local_data())
> > > > >   {
> > > > > _M_use_local_data();
> > > > > _M_set_length(0);
> > > > >   }
> > > > >
> > > > > I haven't updated these, but should this be changed for consistency?
> > > >
> > > > Yes, good idea. I can do that.
> > >
> > > I started to do that, and decided it made more sense to split out the
> > > constexpr loop from _M_use_local_data() into a separate function,
> > > _M_init_local_buf(). Then we can use that for all the places where we
> > > don't care about the return value. That avoids the overhead of using
> > > pointer_traits::pointer_to when we don't need the return value (which
> > > is admittedly only going to be an issue for -O0 code, but I think it's
> > > cleaner this way anyway).
> > >
> > > Please see the attached patch and let me know what you think.
> >
> > I agree, and it also looks clearer to me what is happening.
>
> Good, I'll make this change next week then.
>
>
> >
> > >
> > > > Thanks again for fixing these. I think this might fix some bug reports
> > > > about clang rejecting our std::string in constant expressions, so I'll
> > > > check those.
> > >
> > > Your patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110900
> > > (so we should backport it to gcc-13 and gcc-12 too).
> >
> > > commit 2668979d3206ff6c039ac0165aae29377a15666c
> > > Author: Jonathan Wakely 
> > > Date:   Fri Sep 29 12:12:22 2023
> > >
> > > libstdc++: Split std::basic_string::_M_use_local_data into two 
> > > functions
> > >
> > > This splits out the activate-the-union-member-for-constexpr logic from
> > > _M_use_local_data, so that it can be used separately in cases that 
> > > don't
> > > need to use std::pointer_traits::pointer_to to obtain the
> > > return value.
> > >
> > > This leaves only three uses of _M_use_local_data() which are all the
> > > same form:
> > >
> > >   __s._M_data(_M_use_local_data());
> > >   __s._M_set_length(0);
> > >
> > > We could remove _M_use_local_data() and change those three places to 
> > > use
> > > a new _M_reset() function that does:
> > >
> > >   _M_init_local_buf();
> > >   _M_data(_M_local_data());
> > >   _M_set_length(0);
> > >
> > > This is left for a future change.
> > >
> > > libstdc++-v3/ChangeLog:
> > >
> > > * include/bits/basic_string.h (_M_init_local_buf()): New
> > > function.
> > > (_M_use_local_data()): Use _M_init_local_buf.
> > > (basic_string(), basic_string(const Alloc&))
> > > (basic_string(basic_string&&))
> > > (basic_string(basic_string&&, const Alloc&)): Use
> > > _M_init_local_buf instead of _M_use_local_data().
> > > * include/bits/basic_string.tcc (swap(basic_string&))
> > > (_M_construct(InIter, InIter, forward_iterator_tag))
> > > (_M_construct(size_type, CharT), reserve()): Likewise.
> > > (_M_construct(InIter, InIter, input_iterator_tag)): Remove 
> > > call
> > > to _M_use_local_data() and initialize the local buffer 
> > > directly.
> > >
> > > diff --git a/libstdc++-v3/include/bits/basic_string.h 
> > > b/libstdc++-v3/include/bits/basic_string.h
> > > index 4f94cd967cf..18a19b8dcbc 100644
> > > --- a/libstdc++-v3/include/bits/basic_string.h
> > > +++ b/libstdc++-v3/include/bits/basic_string.h
> > > @@ -353,13 +353,23 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> > >// Ensure that _M_local_buf is the active member of the union.
> > >__attribute__((__always_inline__))
> > >_GLIBCXX14_CONSTEXPR
> > > -  pointer
> > > -  _M_use_local_data() _GLIBCXX_NOEXCEPT
> > > +  void
> > > +  _M_init_local_buf() _GLIBCXX_NOEXCEPT
> > >{
> > >  #if __cpp_lib_is_constant_evaluated
> > >   if (std::is_constant_evaluated())
> > > for (size_type __i = 0; __i <= _S_local_capacity; ++__i)
> > >   _M_local_buf[__i] = _CharT();
> > > +#endif
> > > +  }
> > > +
> > > 

Re: [PATCH 01/22] Add condition coverage profiling

2023-10-21 Thread Jørgen Kvalsvik

On 05/10/2023 22:39, Jørgen Kvalsvik wrote:

On 05/10/2023 21:59, Jan Hubicka wrote:


Like Wahlen et al this implementation records coverage in fixed-size
bitsets which gcov knows how to interpret. This is very fast, but
introduces a limit on the number of terms in a single boolean
expression, the number of bits in a gcov_unsigned_type (which is
typedef'd to uint64_t), so for most practical purposes this would be
acceptable. This limitation is in the implementation and not the
algorithm, so support for more conditions can be added by also
introducing arbitrary-sized bitsets.


This should not be too hard to do - if conditionalis more complex you
simply introduce more than one counter for it, right?
How many times this trigers on GCC sources?


It shouldn't be, no. But when dynamic bitsets are on the table it would 
be much better to length-encode in smaller multiples than the 64-bit 
counters. Most expressions are small (<4 terms), so the savings would be 
substantial. I opted for the simpler fixed-size to start with because it 
is much simpler and would not introduce any branching or decisions in 
the instrumentation.


I just posted v6 of this patch, and the bitsets are still fixed size. I 
consider dynamic bitsets out of scope for this particular effort, 
although I think it might be worth pursuing later.






For space overhead, the instrumentation needs two accumulators
(gcov_unsigned_type) per condition in the program which will be written
to the gcov file. In addition, every function gets a pair of local
accumulators, but these accmulators are reused between conditions in the
same function.

For time overhead, there is a zeroing of the local accumulators for
every condition and one or two bitwise operation on every edge taken in
the an expression.

In action it looks pretty similar to the branch coverage. The -g short
opt carries no significance, but was chosen because it was an available
option with the upper-case free too.

gcov --conditions:

 3:   17:void fn (int a, int b, int c, int d) {
 3:   18:    if ((a && (b || c)) && d)
conditions covered 3/8
condition  0 not covered (true)
condition  0 not covered (false)
condition  1 not covered (true)
condition  2 not covered (true)
condition  3 not covered (true)

It seems understandable, but for bigger conditionals I guess it will be
bit hard to make sense between condition numbers and the actual source
code.  We could probably also show the conditions as ranges in the
conditional?  I am adding David Malcolm to CC, he may have some ideas.

I wonder how much this information is confused by early optimizations
happening before coverage profiling?


Yes, but I could not figure out strong gcov mechanisms to make a truly 
better one, and the json + extra tooling is more in line with what you 
want for large conditionals, I think. I also specifically did one unit 
of information per line to play nicer with grep, so it currently looks like:


conditions covered 3/8
condition  0 not covered (true false)
...

I think improving gcov's printing abilities would do wonders, but I 
couldn't find anything that would support it currently. Did I miss 
something?




Some expressions, mostly those without else-blocks, are effectively
"rewritten" in the CFG construction making the algorithm unable to
distinguish them:

and.c:

 if (a && b && c)
 x = 1;

ifs.c:

 if (a)
 if (b)
 if (c)
 x = 1;

gcc will build the same graph for both these programs, and gcov will
report boths as 3-term expressions. It is vital that it is not
interpreted the other way around (which is consistent with the shape of
the graph) because otherwise the masking would be wrong for the and.c
program which is a more severe error. While surprising, users would
probably expect some minor rewriting of semantically-identical
expressions.

and.c.gcov:
 #:    2:    if (a && b && c)
conditions covered 6/6
 #:    3:    x = 1;

ifs.c.gcov:
 #:    2:    if (a)
 #:    3:    if (b)
 #:    4:    if (c)
 #:    5:    x = 1;
conditions covered 6/6


Maybe one can use location information to distinguish those cases?
Don't we store discriminator info about individual statements that is 
also used for

auto-FDO?


That is one possibility, which I tried for a bit, but abandoned to focus 
on getting the rest of the algorithm right. I am sure it can be 
revisited (possibly as a future improvement) and weighted against always 
emitting an else block (see 
https://gcc.gnu.org/pipermail/gcc-patches/2023-September/631254.html)


This has not been changed in v6. I think the always-else approach is nicer.





gcc/ChangeLog:

* builtins.cc (expand_builtin_fork_or_exec): Check
profile_condition_flag.
 * collect2.cc (main): Add -fno-profile-conditions to OBSTACK.
* common.opt: Add new options -fprofile-conditions and
* doc/gcov.texi: Add --conditions 

[PATCH v6] Add condition coverage profiling

2023-10-21 Thread Jørgen Kvalsvik
This patch adds support in gcc+gcov for modified condition/decision
coverage (MC/DC) with the -fprofile-conditions flag. MC/DC is a type of
test/code coverage and it is particularly important in the avation and
automotive industries for safety-critical applications. MC/DC it is
required for or recommended by:

* DO-178C for the most critical software (Level A) in avionics
* IEC 61508 for SIL 4
* ISO 26262-6 for ASIL D

>From the SQLite webpage:

Two methods of measuring test coverage were described above:
"statement" and "branch" coverage. There are many other test
coverage metrics besides these two. Another popular metric is
"Modified Condition/Decision Coverage" or MC/DC. Wikipedia defines
MC/DC as follows:

* Each decision tries every possible outcome.
* Each condition in a decision takes on every possible outcome.
* Each entry and exit point is invoked.
* Each condition in a decision is shown to independently affect
  the outcome of the decision.

In the C programming language where && and || are "short-circuit"
operators, MC/DC and branch coverage are very nearly the same thing.
The primary difference is in boolean vector tests. One can test for
any of several bits in bit-vector and still obtain 100% branch test
coverage even though the second element of MC/DC - the requirement
that each condition in a decision take on every possible outcome -
might not be satisfied.

https://sqlite.org/testing.html#mcdc

Whalen, Heimdahl, and De Silva "Efficient Test Coverage Measurement for
MC/DC" describes an algorithm for adding instrumentation by carrying
over information from the AST, but my algorithm analyses the the control
flow graph to instrument for coverage. This has the benefit of being
programming language independent and faithful to compiler decisions
and transformations. I have primarily tested it on C and C++, see
testsuite/gcc.misc-tests and testsuite/g++.dg, and run some manual tests
using D, Rust, and Go. D and Rust mostly behave as you would expect (I
found D would sometimes put the conditions for lambdas into the module)
It does not work as expected for Go as the go front-end evaluates
multi-conditional expressions by folding results into temporaries.

Like Whalen et al this implementation records coverage in fixed-size
bitsets which gcov knows how to interpret. This is very fast, but
introduces a limit on the number of terms in a single boolean
expression, the number of bits in a gcov_unsigned_type (which is
typedef'd to uint64_t), so for most practical purposes this would be
acceptable. This limitation is in the implementation and not the
algorithm, so support for more conditions can be added by also
introducing arbitrary-sized bitsets.

For space overhead, the instrumentation needs two accumulators
(gcov_unsigned_type) per condition in the program which will be written
to the gcov file. In addition, every function gets a pair of local
accumulators, but these accmulators are reused between conditions in the
same function.

For time overhead, there is a zeroing of the local accumulators for
every condition and one or two bitwise operation on every edge taken in
the an expression.

In action it looks pretty similar to the branch coverage. The -g short
opt carries no significance, but was chosen because it was an available
option with the upper-case free too.

gcov --conditions:

3:   17:void fn (int a, int b, int c, int d) {
3:   18:if ((a && (b || c)) && d)
conditions covered 3/8
condition  0 not covered (true false)
condition  1 not covered (true)
condition  2 not covered (true)
condition  3 not covered (true)
1:   19:x = 1;
-:   20:else
2:   21:x = 2;
3:   22:}

gcov --conditions --json-format:

"conditions": [
{
"not_covered_false": [
0
],
"count": 8,
"covered": 3,
"not_covered_true": [
0,
1,
2,
3
]
}
],

Some expressions, mostly those without else-blocks, are effectively
"rewritten" in the CFG construction making the algorithm unable to
distinguish them:

and.c:

if (a && b && c)
x = 1;

ifs.c:

if (a)
if (b)
if (c)
x = 1;

gcc will build the same graph for both these programs, and gcov will
report boths as 3-term expressions. It is vital that it is not
interpreted the other way around (which is consistent with the shape of
the graph) because otherwise the masking would be wrong for and.c which
is a more severe error. While surprising, users would probably expect
some minor rewriting of semantically-identical expressions. I think this
is something that can be improved on later.

and.c.gcov:
#:2:if (a && b && c)
conditions covered 6/6
#:3:x = 1;

ifs.c.gcov:
  

Re: [PATCH v2] libstdc++: Workaround for LLVM-61763 in ranges

2023-10-21 Thread Jonathan Wakely
On Sat, 21 Oct 2023 at 12:16, Jonathan Wakely  wrote:
>
> On Mon, 16 Oct 2023 at 04:56, Benjamin Acker Brock  wrote:
> >
> > > I don't think this patch counts as legally significant, but if you 
> > > contribute again in future you should be aware of 
> > > https://gcc.gnu.org/contribute.html#legal and either complete the 
> > > copyright assignment paperwork, or add a DCO sign-off to the commit 
> > > message.
> >
> > Thanks for the reminder. I just added a sign-off.
>
> Thanks.
>
> > > This should be a complete sentence, so capital letter and full stop.
> >
> > Fixed!
>
> I actually adjusted the ChangeLog further, to name the modified components:
>
>* include/std/ranges (zip_view, adjacent_view): Implement
>workaround for LLVM-61763.
>
> This is the usual convention.
>
> Pushed to trunk now, thanks again for the fix!

I'll backport this to the gcc-13 branch too.


>
>
> > ---
> >
> > Signed-off-by: Benjamin Brock 
> >
> > libstdc++-v3/ChangeLog:
> >
> > * include/std/ranges: Implement workaround for LLVM-61763 in
> >   zip_view and adjacency_view.
> >
> > ---
> >
> > diff --git a/libstdc++-v3/include/std/ranges 
> > b/libstdc++-v3/include/std/ranges
> > index 1d529a886be..7893e3a84c9 100644
> > --- a/libstdc++-v3/include/std/ranges
> > +++ b/libstdc++-v3/include/std/ranges
> > @@ -4632,6 +4632,9 @@ namespace views::__adaptor
> >class zip_view<_Vs...>::_Iterator
> >  : public __detail::__zip_view_iter_cat<_Const, _Vs...>
> >{
> > +#ifdef __clang__ // LLVM-61763 workaround
> > +  public:
> > +#endif
> >  
> > __detail::__tuple_or_pair_t > _Vs>>...> _M_current;
> >
> >  constexpr explicit
> > @@ -4652,11 +4655,13 @@ namespace views::__adaptor
> > return input_iterator_tag{};
> >  }
> >
> > +#ifndef __clang__ // LLVM-61763 workaround
> >  template
> >requires (view<_Ws> && ...) && (sizeof...(_Ws) > 0) && 
> > is_object_v<_Fp>
> > && regular_invocable<_Fp&, range_reference_t<_Ws>...>
> > && std::__detail::__can_reference > range_reference_t<_Ws>...>>
> >friend class zip_transform_view;
> > +#endif
> >
> >public:
> >  // iterator_category defined in __zip_view_iter_cat
> > @@ -5327,6 +5332,9 @@ namespace views::__adaptor
> >template
> >class adjacent_view<_Vp, _Nm>::_Iterator
> >{
> > +#ifdef __clang__ // LLVM-61763 workaround
> > +  public:
> > +#endif
> >  using _Base = __detail::__maybe_const_t<_Const, _Vp>;
> >  array, _Nm> _M_current = array, 
> > _Nm>();
> >
> > @@ -5367,12 +5375,14 @@ namespace views::__adaptor
> >
> >  friend class adjacent_view;
> >
> > +#ifndef __clang__ // LLVM-61763 workaround
> >  template
> >requires view<_Wp> && (_Mm > 0) && is_object_v<_Fp>
> >  && regular_invocable<__detail::__unarize<_Fp&, _Mm>,
> > range_reference_t<_Wp>>
> >  && 
> > std::__detail::__can_reference > _Mm>,
> >
> > range_reference_t<_Wp>>>
> >friend class adjacent_transform_view;
> > +#endif
> >
> >public:
> >  using iterator_category = input_iterator_tag;
> >



Re: [PATCH v2] libstdc++: Workaround for LLVM-61763 in ranges

2023-10-21 Thread Jonathan Wakely
On Mon, 16 Oct 2023 at 04:56, Benjamin Acker Brock  wrote:
>
> > I don't think this patch counts as legally significant, but if you 
> > contribute again in future you should be aware of 
> > https://gcc.gnu.org/contribute.html#legal and either complete the copyright 
> > assignment paperwork, or add a DCO sign-off to the commit message.
>
> Thanks for the reminder. I just added a sign-off.

Thanks.

> > This should be a complete sentence, so capital letter and full stop.
>
> Fixed!

I actually adjusted the ChangeLog further, to name the modified components:

   * include/std/ranges (zip_view, adjacent_view): Implement
   workaround for LLVM-61763.

This is the usual convention.

Pushed to trunk now, thanks again for the fix!


> ---
>
> Signed-off-by: Benjamin Brock 
>
> libstdc++-v3/ChangeLog:
>
> * include/std/ranges: Implement workaround for LLVM-61763 in
>   zip_view and adjacency_view.
>
> ---
>
> diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> index 1d529a886be..7893e3a84c9 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -4632,6 +4632,9 @@ namespace views::__adaptor
>class zip_view<_Vs...>::_Iterator
>  : public __detail::__zip_view_iter_cat<_Const, _Vs...>
>{
> +#ifdef __clang__ // LLVM-61763 workaround
> +  public:
> +#endif
>  __detail::__tuple_or_pair_t _Vs>>...> _M_current;
>
>  constexpr explicit
> @@ -4652,11 +4655,13 @@ namespace views::__adaptor
> return input_iterator_tag{};
>  }
>
> +#ifndef __clang__ // LLVM-61763 workaround
>  template
>requires (view<_Ws> && ...) && (sizeof...(_Ws) > 0) && is_object_v<_Fp>
> && regular_invocable<_Fp&, range_reference_t<_Ws>...>
> && std::__detail::__can_reference range_reference_t<_Ws>...>>
>friend class zip_transform_view;
> +#endif
>
>public:
>  // iterator_category defined in __zip_view_iter_cat
> @@ -5327,6 +5332,9 @@ namespace views::__adaptor
>template
>class adjacent_view<_Vp, _Nm>::_Iterator
>{
> +#ifdef __clang__ // LLVM-61763 workaround
> +  public:
> +#endif
>  using _Base = __detail::__maybe_const_t<_Const, _Vp>;
>  array, _Nm> _M_current = array, 
> _Nm>();
>
> @@ -5367,12 +5375,14 @@ namespace views::__adaptor
>
>  friend class adjacent_view;
>
> +#ifndef __clang__ // LLVM-61763 workaround
>  template
>requires view<_Wp> && (_Mm > 0) && is_object_v<_Fp>
>  && regular_invocable<__detail::__unarize<_Fp&, _Mm>,
> range_reference_t<_Wp>>
>  && 
> std::__detail::__can_reference _Mm>,
>
> range_reference_t<_Wp>>>
>friend class adjacent_transform_view;
> +#endif
>
>public:
>  using iterator_category = input_iterator_tag;
>



Re: [PATCH v2] libstdc++: testsuite: Enhance codecvt_unicode with tests for length()

2023-10-21 Thread Jonathan Wakely
On Wed, 18 Oct 2023 at 11:52, Dimitrij Mijoski  wrote:
>
> We can test codecvt::length() with the same data that we test
> codecvt::in(). For each call of in() we add another call to length().
> Some additional small cosmentic changes are applied.

Pushed to master, thanks!

>
> libstdc++-v3/ChangeLog:
>
> * testsuite/22_locale/codecvt/codecvt_unicode.h: Test length()
> ---
>  .../22_locale/codecvt/codecvt_unicode.h   | 123 --
>  1 file changed, 110 insertions(+), 13 deletions(-)
>
> diff --git a/libstdc++-v3/testsuite/22_locale/codecvt/codecvt_unicode.h 
> b/libstdc++-v3/testsuite/22_locale/codecvt/codecvt_unicode.h
> index d3ae42fac..42270c50f 100644
> --- a/libstdc++-v3/testsuite/22_locale/codecvt/codecvt_unicode.h
> +++ b/libstdc++-v3/testsuite/22_locale/codecvt/codecvt_unicode.h
> @@ -17,7 +17,6 @@
>
>  #include 
>  #include 
> -#include 
>  #include 
>
>  struct test_offsets_ok
> @@ -79,6 +78,11 @@ utf8_to_utf32_in_ok (const std::codecvt mbstate_t> )
>VERIFY (char_traits::compare (out, exp, t.out_size) == 0);
>if (t.out_size < array_size (out))
> VERIFY (out[t.out_size] == 0);
> +
> +  state = {};
> +  auto len = cvt.length (state, in, in + t.in_size, t.out_size);
> +  VERIFY (len >= 0);
> +  VERIFY (static_cast (len) == t.in_size);
>  }
>
>for (auto t : offsets)
> @@ -99,6 +103,11 @@ utf8_to_utf32_in_ok (const std::codecvt mbstate_t> )
>VERIFY (char_traits::compare (out, exp, t.out_size) == 0);
>if (t.out_size < array_size (out))
> VERIFY (out[t.out_size] == 0);
> +
> +  state = {};
> +  auto len = cvt.length (state, in, in + t.in_size, array_size (out));
> +  VERIFY (len >= 0);
> +  VERIFY (static_cast (len) == t.in_size);
>  }
>  }
>
> @@ -163,6 +172,11 @@ utf8_to_utf32_in_partial (const std::codecvt ExternT, mbstate_t> )
>   == 0);
>if (t.expected_out_next < array_size (out))
> VERIFY (out[t.expected_out_next] == 0);
> +
> +  state = {};
> +  auto len = cvt.length (state, in, in + t.in_size, t.out_size);
> +  VERIFY (len >= 0);
> +  VERIFY (static_cast (len) == t.expected_in_next);
>  }
>  }
>
> @@ -303,6 +317,11 @@ utf8_to_utf32_in_error (const std::codecvt ExternT, mbstate_t> )
>if (t.expected_out_next < array_size (out))
> VERIFY (out[t.expected_out_next] == 0);
>
> +  state = {};
> +  auto len = cvt.length (state, in, in + t.in_size, t.out_size);
> +  VERIFY (len >= 0);
> +  VERIFY (static_cast (len) == t.expected_in_next);
> +
>in[t.replace_pos] = old_char;
>  }
>  }
> @@ -334,7 +353,7 @@ utf32_to_utf8_out_ok (const std::codecvt ExternT, mbstate_t> )
>VERIFY (char_traits::length (in) == 4);
>VERIFY (char_traits::length (exp) == 10);
>
> -  const test_offsets_ok offsets[] = {{0, 0}, {1, 1}, {2, 3}, {3, 6}, {4, 
> 10}};
> +  test_offsets_ok offsets[] = {{0, 0}, {1, 1}, {2, 3}, {3, 6}, {4, 10}};
>for (auto t : offsets)
>  {
>ExternT out[array_size (exp) - 1] = {};
> @@ -374,7 +393,7 @@ utf32_to_utf8_out_partial (const std::codecvt ExternT, mbstate_t> )
>VERIFY (char_traits::length (in) == 4);
>VERIFY (char_traits::length (exp) == 10);
>
> -  const test_offsets_partial offsets[] = {
> +  test_offsets_partial offsets[] = {
>  {1, 0, 0, 0}, // no space for first CP
>
>  {2, 1, 1, 1}, // no space for second CP
> @@ -528,6 +547,11 @@ utf8_to_utf16_in_ok (const std::codecvt ExternT, mbstate_t> )
>VERIFY (char_traits::compare (out, exp, t.out_size) == 0);
>if (t.out_size < array_size (out))
> VERIFY (out[t.out_size] == 0);
> +
> +  state = {};
> +  auto len = cvt.length (state, in, in + t.in_size, t.out_size);
> +  VERIFY (len >= 0);
> +  VERIFY (static_cast (len) == t.in_size);
>  }
>
>for (auto t : offsets)
> @@ -548,6 +572,11 @@ utf8_to_utf16_in_ok (const std::codecvt ExternT, mbstate_t> )
>VERIFY (char_traits::compare (out, exp, t.out_size) == 0);
>if (t.out_size < array_size (out))
> VERIFY (out[t.out_size] == 0);
> +
> +  state = {};
> +  auto len = cvt.length (state, in, in + t.in_size, array_size (out));
> +  VERIFY (len >= 0);
> +  VERIFY (static_cast (len) == t.in_size);
>  }
>  }
>
> @@ -617,6 +646,11 @@ utf8_to_utf16_in_partial (const std::codecvt ExternT, mbstate_t> )
>   == 0);
>if (t.expected_out_next < array_size (out))
> VERIFY (out[t.expected_out_next] == 0);
> +
> +  state = {};
> +  auto len = cvt.length (state, in, in + t.in_size, t.out_size);
> +  VERIFY (len >= 0);
> +  VERIFY (static_cast (len) == t.expected_in_next);
>  }
>  }
>
> @@ -757,6 +791,11 @@ utf8_to_utf16_in_error (const std::codecvt ExternT, mbstate_t> )
>if (t.expected_out_next < array_size (out))
> VERIFY (out[t.expected_out_next] == 0);
>
> +  state = {};
> +  auto len = cvt.length (state, in, in + 

[PING 2] [C PATCH] Synthesize nonnull attribute for parameters declared with static

2023-10-21 Thread Martin Uecker
> 
> C programmers increasingly use static to indicate that
> pointer parameters are non-null.  Clang can exploit this
> for warnings and optimizations.  GCC has some warnings
> but not all warnings it has for nonnull.  Below is a
> patch to add a nonnull attribute automatically for such 
> arguments and to remove the special and more limited
> nonnull warnings for static. This patch found some 
> misplaced annotations in one of my projects via
> -Wnonnull-compare which clang does not seem to have, 
> so I think this could be useful.

c: Synthesize nonnull attribute for parameters declared with static 
[PR110815]

Parameters declared with `static` are nonnull. We synthesize
an artifical nonnull attribute for such parameters to get the
same warnings and optimizations.

Bootstrapped and regression tested on x86.

PR c/102558
PR 102556
PR c/110815

gcc/c-family:
* c-attribs.cc (build_attr_access_from_parms): Synthesize
nonnull attribute for parameters declared with `static`.

gcc:
* gimple-ssa-warn-access.cc 
(pass_waccess::maybe_check_access_sizes):
remove warning for parameters declared with `static`.

gcc/testsuite:
* gcc.dg/Wnonnull-8.c: Adapt test.
* gcc.dg/Wnonnull-9.c: New test.

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index e2792ca6898..ae7ffeb1f65 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -5280,6 +5280,7 @@ build_attr_access_from_parms (tree parms, bool 
skip_voidptr)
arg2pos.put (arg, argpos);
 }
 
+  tree nnlist = NULL_TREE;
   argpos = 0;
   for (tree arg = parms; arg; arg = TREE_CHAIN (arg), ++argpos)
 {
@@ -5313,6 +5314,11 @@ build_attr_access_from_parms (tree parms, bool 
skip_voidptr)
   tree str = TREE_VALUE (argspec);
   const char *s = TREE_STRING_POINTER (str);
 
+  /* Collect the list of nonnull arguments which use "[static ..]".  */
+  if (s != NULL && s[0] == '[' && s[1] == 's')
+   nnlist = tree_cons (NULL_TREE, build_int_cst (integer_type_node,
+ argpos + 1), nnlist);
+
   /* Create the attribute access string from the arg spec string,
 optionally followed by position of the VLA bound argument if
 it is one.  */
@@ -5380,6 +5386,10 @@ build_attr_access_from_parms (tree parms, bool 
skip_voidptr)
   if (!spec.length ())
 return NULL_TREE;
 
+  /* If we have nonnull arguments, synthesize an attribute.  */
+  if (nnlist != NULL_TREE)
+nnlist = build_tree_list (get_identifier ("nonnull"), nnlist);
+
   /* Attribute access takes a two or three arguments.  Wrap VBLIST in
  another list in case it has more nodes than would otherwise fit.  */
   vblist = build_tree_list (NULL_TREE, vblist);
@@ -5390,7 +5400,7 @@ build_attr_access_from_parms (tree parms, bool 
skip_voidptr)
   tree str = build_string (spec.length (), spec.c_str ());
   tree attrargs = tree_cons (NULL_TREE, str, vblist);
   tree name = get_identifier ("access");
-  return build_tree_list (name, attrargs);
+  return tree_cons (name, attrargs, nnlist);
 }
 
 /* Handle a "nothrow" attribute; arguments as in
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index b70d67dc47b..b8e89d01088 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -3491,16 +3491,6 @@ pass_waccess::maybe_check_access_sizes (rdwr_map *rwm, 
tree fndecl, tree fntype,
   ptridx + 1, sizidx + 1, sizstr))
arg_warned = OPT_Wnonnull;
}
- else if (access_size && access.second.static_p)
-   {
- /* Warn about null pointers for [static N] array arguments
-but do not warn for ordinary (i.e., nonstatic) arrays.  */
- if (warning_at (loc, OPT_Wnonnull,
- "argument %i to %<%T[static %E]%> "
- "is null where non-null expected",
- ptridx + 1, argtype, access_nelts))
-   arg_warned = OPT_Wnonnull;
-   }
 
  if (arg_warned != no_warning)
{
diff --git a/gcc/testsuite/gcc.dg/Wnonnull-8.c 
b/gcc/testsuite/gcc.dg/Wnonnull-8.c
index 02871a76689..b24fd67cebc 100644
--- a/gcc/testsuite/gcc.dg/Wnonnull-8.c
+++ b/gcc/testsuite/gcc.dg/Wnonnull-8.c
@@ -10,5 +10,5 @@ foo (int a[static 7])
 int
 main ()
 {
-  foo ((int *) 0); /* { dg-warning "argument 1 to 'int\\\[static 7\\\]' is 
null where non-null expected" } */
+  foo ((int *) 0); /* { dg-warning "argument 1 null where non-null 
expected" } */
 }
diff --git a/gcc/testsuite/gcc.dg/Wnonnull-9.c 
b/gcc/testsuite/gcc.dg/Wnonnull-9.c
new file mode 100644
index 000..1c57eefd2ae
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wnonnull-9.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-Wall" } 

[committed] libstdc++: Fix formatting of filesystem directory iterators

2023-10-21 Thread Jonathan Wakely
Tested x86_64-linux. Pushed to trunk.

-- >8 --

Fix indentation.

libstdc++-v3/ChangeLog:

* include/bits/fs_dir.h (operator==(default_sentinel_t)): Fix
indentation.
---
 libstdc++-v3/include/bits/fs_dir.h | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libstdc++-v3/include/bits/fs_dir.h 
b/libstdc++-v3/include/bits/fs_dir.h
index 9dd0f896e46..3b0a493dc3f 100644
--- a/libstdc++-v3/include/bits/fs_dir.h
+++ b/libstdc++-v3/include/bits/fs_dir.h
@@ -449,10 +449,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 }
 
 #if __cplusplus >= 202002L
-  // _GLIBCXX_RESOLVE_LIB_DEFECTS
-  // 3719. Directory iterators should be usable with default sentinel
-  bool operator==(default_sentinel_t) const noexcept
-  { return !_M_dir; }
+// _GLIBCXX_RESOLVE_LIB_DEFECTS
+// 3719. Directory iterators should be usable with default sentinel
+bool operator==(default_sentinel_t) const noexcept
+{ return !_M_dir; }
 #endif
 
 #if __cpp_impl_three_way_comparison < 201907L
@@ -565,10 +565,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 }
 
 #if __cplusplus >= 202002L
-  // _GLIBCXX_RESOLVE_LIB_DEFECTS
-  // 3719. Directory iterators should be usable with default sentinel
-  bool operator==(default_sentinel_t) const noexcept
-  { return !_M_dirs; }
+// _GLIBCXX_RESOLVE_LIB_DEFECTS
+// 3719. Directory iterators should be usable with default sentinel
+bool operator==(default_sentinel_t) const noexcept
+{ return !_M_dirs; }
 #endif
 
 #if __cpp_impl_three_way_comparison < 201907L
-- 
2.41.0



[PATCH v10 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces

2023-10-21 Thread Ajit Agarwal
Hello Vineet and Jeff:

This version 10 of the patch uses abi interfaces to remove zero and sign 
extension elimination.
Bootstrapped and regtested on powerpc-linux-gnu.

In this version (version 9) of the patch following review comments are 
incorporated.

a) Removal of hard code zero_extend and sign_extend  in abi interfaces.
b) Source and destination with different registers are considered.
c) Further enhancements.
d) Added sign extension elimination using abi interfaces.
d) Addressed remaining review comments.

Please check if its addressed bootstrapped failure with RISC-V.
Also please let me know if there is anything missing in this patch.

Ok for trunk?

Thanks & Regards
Ajit


ree: Improve ree pass for rs6000 target using defined abi interfaces

For rs6000 target we see redundant zero and sign extension and done
to improve ree pass to eliminate such redundant zero and sign extension
using defined ABI interfaces.

2023-10-21  Ajit Kumar Agarwal  

gcc/ChangeLog:

* ree.cc (combine_reaching_defs): Use of zero_extend and sign_extend
defined abi interfaces.
(add_removable_extension): Use of defined abi interfaces for no
reaching defs.
(abi_extension_candidate_return_reg_p): New function.
(abi_extension_candidate_p): New function.
(abi_extension_candidate_argno_p): New function.
(abi_handle_regs): New function.
(abi_target_promote_function_mode): New function.

gcc/testsuite/ChangeLog:

* g++.target/powerpc/zext-elim-3.C
---
 gcc/ree.cc| 145 +-
 .../g++.target/powerpc/zext-elim-3.C  |  13 ++
 2 files changed, 154 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-3.C

diff --git a/gcc/ree.cc b/gcc/ree.cc
index fc04249fa84..51b1aab165e 100644
--- a/gcc/ree.cc
+++ b/gcc/ree.cc
@@ -514,7 +514,8 @@ get_uses (rtx_insn *insn, rtx reg)
 if (REGNO (DF_REF_REG (def)) == REGNO (reg))
   break;
 
-  gcc_assert (def != NULL);
+  if (def == NULL)
+return NULL;
 
   ref_chain = DF_REF_CHAIN (def);
 
@@ -750,6 +751,124 @@ get_extended_src_reg (rtx src)
   return src;
 }
 
+/* Return TRUE if target mode is equal to source mode of zero_extend
+   or sign_extend otherwise false.  */
+
+static bool
+abi_target_promote_function_mode (machine_mode mode)
+{
+  int unsignedp;
+  machine_mode tgt_mode
+= targetm.calls.promote_function_mode (NULL_TREE, mode, ,
+  NULL_TREE, 1);
+
+  if (tgt_mode == mode)
+return true;
+  else
+return false;
+}
+
+/* Return TRUE if the candidate insn is zero extend and regno is
+   a return registers.  */
+
+static bool
+abi_extension_candidate_return_reg_p (/*rtx_insn *insn, */int regno)
+{
+  if (targetm.calls.function_value_regno_p (regno))
+return true;
+
+  return false;
+}
+
+/* Return TRUE if reg source operand of zero_extend is argument registers
+   and not return registers and source and destination operand are same
+   and mode of source and destination operand are not same.  */
+
+static bool
+abi_extension_candidate_p (rtx_insn *insn)
+{
+  rtx set = single_set (insn);
+  machine_mode dst_mode = GET_MODE (SET_DEST (set));
+  rtx orig_src = XEXP (SET_SRC (set), 0);
+
+  if (!FUNCTION_ARG_REGNO_P (REGNO (orig_src))
+  || abi_extension_candidate_return_reg_p (/*insn,*/ REGNO (orig_src)))
+return false;
+
+  /* Mode of destination and source should be different.  */
+  if (dst_mode == GET_MODE (orig_src))
+return false;
+
+  machine_mode mode = GET_MODE (XEXP (SET_SRC (set), 0));
+  bool promote_p = abi_target_promote_function_mode (mode);
+
+  /* REGNO of source and destination should be same if not
+  promoted.  */
+  if (!promote_p && REGNO (SET_DEST (set)) != REGNO (orig_src))
+return false;
+
+  return true;
+}
+
+/* Return TRUE if the candidate insn is zero extend and regno is
+   an argument registers.  */
+
+static bool
+abi_extension_candidate_argno_p (/*rtx_code code, */int regno)
+{
+  if (FUNCTION_ARG_REGNO_P (regno))
+return true;
+
+  return false;
+}
+
+/* Return TRUE if the candidate insn doesn't have defs and have
+ * uses without RTX_BIN_ARITH/RTX_COMM_ARITH/RTX_UNARY rtx class.  */
+
+static bool
+abi_handle_regs (rtx_insn *insn)
+{
+  if (side_effects_p (PATTERN (insn)))
+return false;
+
+  struct df_link *uses = get_uses (insn, SET_DEST (PATTERN (insn)));
+
+  if (!uses)
+return false;
+
+  for (df_link *use = uses; use; use = use->next)
+{
+  if (!use->ref)
+   return false;
+
+  if (BLOCK_FOR_INSN (insn) != BLOCK_FOR_INSN (DF_REF_INSN (use->ref)))
+   return false;
+
+  rtx_insn *use_insn = DF_REF_INSN (use->ref);
+
+  if (GET_CODE (PATTERN (use_insn)) == SET)
+   {
+ rtx_code code = GET_CODE (SET_SRC (PATTERN (use_insn)));
+
+ if (GET_RTX_CLASS (code) == RTX_BIN_ARITH
+ || GET_RTX_CLASS (code) == RTX_COMM_ARITH
+ || 

Re: [PATCH v9 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces

2023-10-21 Thread Ajit Agarwal
Hello Vineet:

Thanks for your time and valuable comments.

On 21/10/23 5:26 am, Vineet Gupta wrote:
> On 10/19/23 23:50, Ajit Agarwal wrote:
>> Hello All:
>>
>> This version 9 of the patch uses abi interfaces to remove zero and sign 
>> extension elimination.
>> Bootstrapped and regtested on powerpc-linux-gnu.
>>
>> In this version (version 9) of the patch following review comments are 
>> incorporated.
>>
>> a) Removal of hard code zero_extend and sign_extend  in abi interfaces.
>> b) Source and destination with different registers are considered.
>> c) Further enhancements.
>> d) Added sign extension elimination using abi interfaces.
> 
> As has been trend in the past, I don't think all the review comments have 
> been addressed.
> The standard practice is to reply to reviewer's email and say yay/nay 
> explicitly to each comment. Some of my comments in [1a] are still not 
> resolved, importantly the last two. To be fair you did reply [1b] but the 
> comments were not addressed explicitly.
> 
> [1a] https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630814.html
> [1b] https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630865.html
> 

I have addressed the last 2 comments in the version 10 of the patch. Please let 
me know if there
is anything missing. Regarding last comments with a providing different tests 
if you have any suggestions please 
let me know.

> Anyhow I gave this a try for RISC-V, specially after [2a][2b] as I was 
> curious to see if this uncovers REE handling extraneous extensions which 
> could potentially be eliminated in Expand itself, which is generally better 
> as it happens earlier in the pipeline.
> 
> [2a] 2023-10-16 8eb9cdd14218 expr: don't clear SUBREG_PROMOTED_VAR_P flag for 
> a promoted subreg [target/111466]
> [2b] https://gcc.gnu.org/pipermail/gcc-patches/2023-October/631818.html
> 
> Bad news is with the patch, we fail to even bootstrap risc-v, buckles over 
> when building libgcc itself.
> 
> The reproducer is embarrassingly simple, build with -O2:
> 
> float a;
> b() { return a; }
> 
> See details below
> 
>> Thanks & Regards
>> Ajit
>>
>> ree: Improve ree pass for rs6000 target using defined abi interfaces
>>
>> For rs6000 target we see redundant zero and sign extension and done
>> to improve ree pass to eliminate such redundant zero and sign extension
>> using defined ABI interfaces.
>>
>> 2023-10-20  Ajit Kumar Agarwal  
>>
>> gcc/ChangeLog:
>>
>>  * ree.cc (combine_reaching_defs): Use of zero_extend and sign_extend
>>  defined abi interfaces.
>>  (add_removable_extension): Use of defined abi interfaces for no
>>  reaching defs.
>>  (abi_extension_candidate_return_reg_p): New function.
>>  (abi_extension_candidate_p): New function.
>>  (abi_extension_candidate_argno_p): New function.
>>  (abi_handle_regs): New function.
>>  (abi_target_promote_function_mode): New function.
>>
>> gcc/testsuite/ChangeLog:
>>
>>  * g++.target/powerpc/zext-elim-3.C
>> ---
>>   gcc/ree.cc    | 151 +-
>>   .../g++.target/powerpc/zext-elim-3.C  |  13 ++
>>   2 files changed, 161 insertions(+), 3 deletions(-)
>>   create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-3.C
>>
>> diff --git a/gcc/ree.cc b/gcc/ree.cc
>> index fc04249fa84..9f21f0e9907 100644
>> --- a/gcc/ree.cc
>> +++ b/gcc/ree.cc
>> @@ -514,7 +514,8 @@ get_uses (rtx_insn *insn, rtx reg)
>>   if (REGNO (DF_REF_REG (def)) == REGNO (reg))
>>     break;
>>   -  gcc_assert (def != NULL);
>> +  if (def == NULL)
>> +    return NULL;
>>       ref_chain = DF_REF_CHAIN (def);
>>   @@ -750,6 +751,124 @@ get_extended_src_reg (rtx src)
>>     return src;
>>   }
>>   +/* Return TRUE if target mode is equal to source mode of zero_extend
>> +   or sign_extend otherwise false.  */
>> +
>> +static bool
>> +abi_target_promote_function_mode (machine_mode mode)
>> +{
>> +  int unsignedp;
>> +  machine_mode tgt_mode
>> +    = targetm.calls.promote_function_mode (NULL_TREE, mode, ,
>> +   NULL_TREE, 1);
>> +
>> +  if (tgt_mode == mode)
>> +    return true;
>> +  else
>> +    return false;
>> +}
>> +
>> +/* Return TRUE if the candidate insn is zero extend and regno is
>> +   a return registers.  */
>> +
>> +static bool
>> +abi_extension_candidate_return_reg_p (/*rtx_insn *insn, */int regno)
>> +{
>> +  if (targetm.calls.function_value_regno_p (regno))
>> +    return true;
>> +
>> +  return false;
>> +}
>> +
>> +/* Return TRUE if reg source operand of zero_extend is argument registers
>> +   and not return registers and source and destination operand are same
>> +   and mode of source and destination operand are not same.  */
>> +
>> +static bool
>> +abi_extension_candidate_p (rtx_insn *insn)
>> +{
>> +  rtx set = single_set (insn);
>> +  machine_mode dst_mode = GET_MODE (SET_DEST (set));
>> +  rtx orig_src = XEXP (SET_SRC (set), 0);
>> +
>> +  if 

Re: [PATCH 2/5] LoongArch: Use explicit relocs for GOT access when -mexplicit-relocs=auto and LTO during a final link with linker plugin

2023-10-21 Thread Xi Ruoyao
On Sat, 2023-10-21 at 15:32 +0800, chenglulu wrote:
> > +  /* If we are performing LTO for a final link, and we have the linker
> > + plugin so we know the resolution of the symbols, then all GOT
> > + references are binding to external symbols or preemptable symbols.
> > + So the linker cannot relax them.  */
> > +  return (in_lto_p
> > +     && !flag_incremental_link
> 
> I don’t quite understand this condition "!flag_incremental_link". Can 
> you explain it? Others LGTM.
> 
> Thanks.

If we have two (or several) .o files containing LTO bytecode, GCC
supports "LTO incremental linking" with:

gcc a.o b.o -o ab.o -O2 -flto -flinker-output=nolto-rel

The resulted ab.o will include data and code in a.o and b.o, but it
contains machine code instead of LTO bytecode.  Now if ab.o refers to an
external symbol c, the linker may relax "la.global c" to "la.local c"
(if ab.o is linked together with another file c.o which contains the
definition of c) or not.  As we cannot exclude the possibility of a
relaxation on la.global for incremental linking, just emit la.global and
let the linker to do the correct thing.

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University


Re: [PATCH 2/5] LoongArch: Use explicit relocs for GOT access when -mexplicit-relocs=auto and LTO during a final link with linker plugin

2023-10-21 Thread chenglulu

/* snip */


+/* If -mexplicit-relocs=auto, we use machine operations with reloc hints
+   for cases where the linker is unable to relax so we can schedule the
+   machine operations, otherwise use an assembler pseudo-op so the
+   assembler will generate R_LARCH_RELAX.  */
+
+bool
+loongarch_explicit_relocs_p (enum loongarch_symbol_type type)
+{
+  if (la_opt_explicit_relocs != EXPLICIT_RELOCS_AUTO)
+return la_opt_explicit_relocs == EXPLICIT_RELOCS_ALWAYS;
+
+  /* If we are performing LTO for a final link, and we have the linker
+ plugin so we know the resolution of the symbols, then all GOT
+ references are binding to external symbols or preemptable symbols.
+ So the linker cannot relax them.  */
+  return (in_lto_p
+ && !flag_incremental_link


I don’t quite understand this condition "!flag_incremental_link". Can 
you explain it? Others LGTM.


Thanks.


+ && HAVE_LTO_PLUGIN == 2
+ && (!global_options_set.x_flag_use_linker_plugin
+ || global_options.x_flag_use_linker_plugin)
+ && type == SYMBOL_GOT_DISP);
+}
+




[PATCH] [PR111520] set hardcmp eh probs (was: rename make_eh_edges to make_eh_edge)

2023-10-21 Thread Alexandre Oliva
On Oct 20, 2023, Richard Biener  wrote:

>> * tree-eh.h (make_eh_edges): Rename to...
>> (make_eh_edge): ... this.
>> * tree-eh.cc: Likewise.  Adjust all callers.

Once the above goes in (it depends on the strub monster patch), the
following one should apply as well.  Regstrapped on x86_64-linux-gnu.
Ok to install?

Set execution count of EH blocks, and probability of EH edges.


for  gcc/ChangeLog

PR tree-optimization/111520
* gimple-harden-conditionals.cc
(pass_harden_compares::execute): Set EH edge probability and
EH block execution count.

for  gcc/testsuite/ChangeLog

PR tree-optimization/111520
* g++.dg/torture/harden-comp-pr111520.cc: New.
---
 gcc/gimple-harden-conditionals.cc  |   12 +++-
 .../g++.dg/torture/harden-comp-pr111520.cc |   17 +
 2 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/torture/harden-comp-pr111520.cc

diff --git a/gcc/gimple-harden-conditionals.cc 
b/gcc/gimple-harden-conditionals.cc
index 1999e827a04ca..bded288985063 100644
--- a/gcc/gimple-harden-conditionals.cc
+++ b/gcc/gimple-harden-conditionals.cc
@@ -580,11 +580,21 @@ pass_harden_compares::execute (function *fun)
  if (throwing_compare_p)
{
  add_stmt_to_eh_lp (asgnck, lookup_stmt_eh_lp (asgn));
- make_eh_edge (asgnck);
+ edge eh = make_eh_edge (asgnck);
+ /* This compare looks like it could raise an exception,
+but it's dominated by the original compare, that
+would raise an exception first, so the EH edge from
+this one is never really taken.  */
+ eh->probability = profile_probability::never ();
+ if (eh->dest->count.initialized_p ())
+   eh->dest->count += eh->count ();
+ else
+   eh->dest->count = eh->count ();
 
  edge ckeh;
  basic_block nbb = split_edge (non_eh_succ_edge
(gimple_bb (asgnck), ));
+ gcc_checking_assert (eh == ckeh);
  gsi_split = gsi_start_bb (nbb);
 
  if (dump_file)
diff --git a/gcc/testsuite/g++.dg/torture/harden-comp-pr111520.cc 
b/gcc/testsuite/g++.dg/torture/harden-comp-pr111520.cc
new file mode 100644
index 0..b4381b4d84ec4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/harden-comp-pr111520.cc
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-fharden-compares -fsignaling-nans -fnon-call-exceptions" } */
+
+struct S
+{
+  S (bool);
+  ~S ();
+};
+
+float f;
+
+void
+foo ()
+{
+  S a = 0;
+  S b = f;
+}


-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive


Re: Remove stale Autoconf checks for Perl

2023-10-21 Thread Alexandre Oliva
On May 16, 2023, Thomas Schwinge  wrote:

> OK to push the attached "Remove stale Autoconf checks for Perl"?

LGTM, thanks,

-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice but
very few check the facts.  Think Assange & Stallman.  The empires strike back