Re: [PATCH] mingw32: Fix warning, update documentation

2022-09-08 Thread Jonathan Yong via Gcc-patches

On 9/8/22 11:02, Jan-Benedict Glaw wrote:

Hi!


The mingw32 port is the only port to have TARGET_OVERRIDES_FORMAT_ATTRIBUTES
defined. When this macro is defined, it will never evaluate to NULL, so this
check just leads to a warning:

/usr/lib/gcc-snapshot/bin/g++ -fcf-protection -fno-PIE -c  -DIN_GCC_FRONTEND 
-DIN_GCC_FRONTEND -DIN_GCC_FRONTEND -g -O2   -DIN_GCC  
-DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic 
-Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common 
 -DHAVE_CONFIG_H -I. -Ic-family -I../../gcc/gcc -I../../gcc/gcc/c-family 
-I../../gcc/gcc/../include -I../../gcc/gcc/../libcpp/include 
-I../../gcc/gcc/../libcody  -I../../gcc/gcc/../libdecnumber 
-I../../gcc/gcc/../libdecnumber/bid -I../libdecnumber 
-I../../gcc/gcc/../libbacktrace   -o c-family/c-format.o -MT 
c-family/c-format.o -MMD -MP -MF c-family/.deps/c-format.TPo 
../../gcc/gcc/c-family/c-format.cc
../../gcc/gcc/c-family/c-format.cc: In function 'const char* 
convert_format_name_to_system_name(const char*)':
../../gcc/gcc/c-family/c-format.cc:5114:42: error: the address of 
'mingw_format_attribute_overrides' will never be NULL [-Werror=address]
  5114 |   if (TARGET_OVERRIDES_FORMAT_ATTRIBUTES != NULL
In file included from ./tm.h:26,
  from ../../gcc/gcc/c-family/c-format.cc:23:
../../gcc/gcc/config/i386/mingw32.h:268:44: note: 
'mingw_format_attribute_overrides' declared here
   268 | #define TARGET_OVERRIDES_FORMAT_ATTRIBUTES 
mingw_format_attribute_overrides
   |
^~~~
../../gcc/gcc/c-family/c-format.cc:5079:30: note: in expansion of macro 
'TARGET_OVERRIDES_FORMAT_ATTRIBUTES'
  5079 | extern const target_ovr_attr TARGET_OVERRIDES_FORMAT_ATTRIBUTES[];
   |  ^~
cc1plus: all warnings being treated as errors
make[1]: *** [Makefile:1146: c-family/c-format.o] Error 1
make[1]: Leaving directory 
'/var/lib/laminar/run/gcc-x86_64-w64-mingw32/1/toolchain-build/gcc'
make: *** [Makefile:4588: all-gcc] Error 2



   Also, when TARGET_OVERRIDES_FORMAT_ATTRIBUTES is defined,
TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT must be defined as well. Add
that requirement to the docs.


2022-09-07  Jan-Benedict Glaw  

gcc/ChangeLog:
* c-family/c-format.cc (convert_format_name_to_system_name): Fix 
warning.
* doc/tm.texi.in (TARGET_OVERRIDES_FORMAT_ATTRIBUTES): Document 
requirement
of TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT being defined as well.
* doc/tm.texi: Regenerate.

diff --git a/gcc/c-family/c-format.cc b/gcc/c-family/c-format.cc
index 68b94da40cc..a6c380bf1c8 100644
--- a/gcc/c-family/c-format.cc
+++ b/gcc/c-family/c-format.cc
@@ -5111,8 +5111,7 @@ convert_format_name_to_system_name (const char *attr_name)
  
  #ifdef TARGET_OVERRIDES_FORMAT_ATTRIBUTES

/* Check if format attribute is overridden by target.  */
-  if (TARGET_OVERRIDES_FORMAT_ATTRIBUTES != NULL
-  && TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT > 0)
+  if (TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT > 0)
  {
for (i = 0; i < TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT; ++i)
  {
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 5312059ea79..21b849ea32a 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -7836,7 +7836,8 @@ If defined, this macro is the number of entries in
  If defined, this macro is the name of a global variable containing
  target-specific format overrides for the @option{-Wformat} option. The
  default is to have no target-specific format overrides. If defined,
-@code{TARGET_FORMAT_TYPES} must be defined, too.
+@code{TARGET_FORMAT_TYPES} and @code{TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT}
+must be defined, too.
  @end defmac
  
  @defmac TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT

diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index e47bf28089a..858bfb80cec 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -12043,7 +12043,8 @@ If defined, this macro is the number of entries in
  If defined, this macro is the name of a global variable containing
  target-specific format overrides for the @option{-Wformat} option. The
  default is to have no target-specific format overrides. If defined,
-@code{TARGET_FORMAT_TYPES} must be defined, too.
+@code{TARGET_FORMAT_TYPES} and @code{TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT}
+must be defined, too.
  @end defmac
  
  @defmac TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT



Okay for HEAD?

Thanks,
   Jan-Benedict



Looks good to me, thanks for working on it.


OpenPGP_0x713B5FE29C145D45_and_old_rev.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature


RE: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.

2022-09-08 Thread Eugene Rozenfeld via Gcc-patches
Jason,

Thank for your suggestion. The patch is updated (attached).

Thanks,

Eugene

-Original Message-
From: Jason Merrill  
Sent: Thursday, September 08, 2022 10:26 AM
To: Eugene Rozenfeld ; gcc-patches@gcc.gnu.org
Cc: Andi Kleen ; Jan Hubicka 
Subject: Re: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator 
support.

On 9/1/22 18:22, Eugene Rozenfeld wrote:
> Jason,
> 
> I made another small change in addressing your feedback (attached).
> 
> Thanks,
> 
> Eugene
> 
> -Original Message-
> From: Gcc-patches 
>  On Behalf Of 
> Eugene Rozenfeld via Gcc-patches
> Sent: Thursday, September 01, 2022 1:49 PM
> To: Jason Merrill ; gcc-patches@gcc.gnu.org
> Cc: Andi Kleen ; Jan Hubicka 
> Subject: RE: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator 
> support.
> 
> Jason,
> 
> Thank you for your review. You are correct, we only need to check 
> has_discriminator for the statement that's on the same line.
> I updated the patch (attached).
> 
> Thanks,
> 
> Eugene
> 
> -Original Message-
> From: Jason Merrill 
> Sent: Thursday, August 18, 2022 6:55 PM
> To: Eugene Rozenfeld ; 
> gcc-patches@gcc.gnu.org
> Cc: Andi Kleen ; Jan Hubicka 
> Subject: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator 
> support.
> 
> On 8/3/22 17:25, Eugene Rozenfeld wrote:
>> One more ping for this patch
>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.
>> gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.htmldata=
>> 0
>> 5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da8
>> 1
>> 85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63796470932569195
>> 1 
>> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
>> I 
>> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=K%2BMx6jelnED3n%2Be2d
>> T
>> DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3Dreserved=0
>>
>> CC Jason since this changes discriminators emitted in dwarf.
>>
>> Thanks,
>>
>> Eugene
>>
>> -Original Message-
>> From: Eugene Rozenfeld
>> Sent: Monday, June 27, 2022 12:45 PM
>> To: gcc-patches@gcc.gnu.org; Andi Kleen ; Jan 
>> Hubicka 
>> Subject: RE: [PING][PATCH] Add instruction level discriminator support.
>>
>> Another ping for 
>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.htmldata=05%7C01%7Ceugene.rozenfeld%40microsoft.com%7Cc7c9fab519184eee0bb808da91bf2fea%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637982547579085499%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=Kj3YWJrDqjX%2B0Ml3At5P8NRWc1Xs6mbI%2F1vCk9%2BLaQs%3Dreserved=0
>>  .
>>
>> I got a review from Andi 
>> (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596549.htmldata=05%7C01%7Ceugene.rozenfeld%40microsoft.com%7Cc7c9fab519184eee0bb808da91bf2fea%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637982547579085499%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=NBzFtD0mH7EYKxsVWfqZgSpQmUG18SIt01XRYUlwwW4%3Dreserved=0)
>>  but I also need a review from someone who can approve the changes.
>>
>> Thanks,
>>
>> Eugene
>>
>> -Original Message-
>> From: Eugene Rozenfeld
>> Sent: Friday, June 10, 2022 12:03 PM
>> To: gcc-patches@gcc.gnu.org; Andi Kleen ; Jan 
>> Hubicka 
>> Subject: [PING][PATCH] Add instruction level discriminator support.
>>
>> Hello,
>>
>> I'd like to ping this patch:
>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.
>> gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.htmldata=
>> 0
>> 5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da8
>> 1
>> 85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63796470932569195
>> 1 
>> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
>> I 
>> k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=K%2BMx6jelnED3n%2Be2d
>> T
>> DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3Dreserved=0
>>
>> Thanks,
>>
>> Eugene
>>
>> -Original Message-
>> From: Gcc-patches
>>  On Behalf Of 
>> Eugene Rozenfeld via Gcc-patches
>> Sent: Thursday, June 02, 2022 12:22 AM
>> To: gcc-patches@gcc.gnu.org; Andi Kleen ; Jan 
>> Hubicka 
>> Subject: [EXTERNAL] [PATCH] Add instruction level discriminator support.
>>
>> This is the first in a series of patches to enable discriminator support in 
>> AutoFDO.
>>
>> This patch switches to tracking discriminators per statement/instruction 
>> instead of per basic block. Tracking per basic block was problematic since 
>> not all statements in a basic block needed a discriminator and, also, later 
>> optimizations could move statements between basic blocks making correlation 
>> during AutoFDO compilation unreliable. Tracking per statement also allows us 
>> to assign different discriminators to multiple function calls in the same 
>> basic block. A subsequent patch will add that support.
>>
>> 

Re: [PATCH] c++: Implement C++23 P2266R1, Simpler implicit move [PR101165]

2022-09-08 Thread Marek Polacek via Gcc-patches
On Tue, Sep 06, 2022 at 10:38:12PM -0400, Jason Merrill wrote:
> On 9/3/22 12:42, Marek Polacek wrote:
> > This patch implements https://wg21.link/p2266, which, once again,
> > changes the implicit move rules.  Here's a brief summary of various
> > changes in this area:
> > 
> > r125211: Introduced moving from certain lvalues when returning them
> > r171071: CWG 1148, enable move from value parameter on return
> > r212099: CWG 1579, it's OK to call a converting ctor taking an rvalue
> > r251035: CWG 1579, do maybe-rvalue overload resolution twice
> > r11-2411: Avoid calling const copy ctor on implicit move
> > r11-2412: C++20 implicit move changes, remove the fallback overload
> >resolution, allow move on throw of parameters and implicit
> >   move of rvalue references
> > 
> > P2266 enables the implicit move for functions that return references.  This
> > was a one-line change: check TYPE_REF_P.  That is, we will now perform
> > a move in
> > 
> >X&& foo (X&& x) {
> >  return x;
> >}
> > 
> > P2266 also removes the fallback overload resolution, but this was
> > resolved by r11-2412: we only do convert_for_initialization with
> > LOOKUP_PREFER_RVALUE in C++17 and older.
> 
> I wonder if we want to extend the current C++20 handling to the older modes
> for GCC 13?  Not in this patch, but as a followup.
> 
> > P2266 also says that a returned move-eligible id-expression is always an
> > xvalue.  This required some further short, but nontrivial changes,
> > especially when it comes to deduction, because we have to pay attention
> > to whether we have auto, auto&& (which is like T&&), or decltype(auto)
> > with (un)parenthesized argument.  In C++23,
> > 
> >decltype(auto) f(int&& x) { return (x); }
> >auto&& f(int x) { return x; }
> > 
> > both should deduce to 'int&&' but
> > 
> >decltype(auto) f(int x) { return x; }
> > 
> > should deduce to 'int'.  A cornucopia of tests attached.  I've also
> > verified that we behave like clang++.
> > 
> > xvalue_p seemed to be broken: since the introduction of clk_implicit_rval,
> > it cannot use '==' when checking for clk_rvalueref.
> > 
> > Since this change breaks code, it's only enabled in C++23.  In
> > particular, this code will not compile in C++23:
> > 
> >int& g(int&& x) { return x; }
> 
> Nice that the C++20 compatibility is so simple!
> 
> > because x is now treated as an rvalue, and you can't bind a non-const lvalue
> > reference to an rvalue.
> > 
> > There's one FIXME in elision1.C:five, which we should compile but reject
> > with "passing 'Mutt' as 'this' argument discards qualifiers".  That
> > looks bogus to me, I think I'll open a PR for it.
> 
> Let's fix that now, I think.

Can of worms.   The test is

  struct Mutt {
  operator int*() &&;
  };

  int* five(Mutt x) {
  return x;  // OK since C++20 because P1155
  }

'x' should be treated as an rvalue, therefore the operator fn taking
an rvalue ref to Mutt should be used to convert 'x' to int*.  We fail
because we don't treat 'x' as an rvalue because the function doesn't
return a class.  So the patch should be just

--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10875,10 +10875,7 @@ check_return_expr (tree retval, bool *no_warning)
  Note that these conditions are similar to, but not as strict as,
 the conditions for the named return value optimization.  */
   bool converted = false;
-  tree moved;
-  /* This is only interesting for class type.  */
-  if (CLASS_TYPE_P (functype)
- && (moved = treat_lvalue_as_rvalue_p (retval, /*return*/true)))
+  if (tree moved = treat_lvalue_as_rvalue_p (retval, /*return*/true))
{
  if (cxx_dialect < cxx20)
{

which fixes the test, but breaks a lot of middle-end warnings.  For instance
g++.dg/warn/nonnull3.C, where the patch above changes .gimple:

 bool A::foo (struct A * const this, <<< Unknown tree: offset_type >>> p)
 {
-  bool D.2146;
+  bool D.2150;
 
-  D.2146 = p != -1;
-  return D.2146;
+  p.0_1 = p;
+  D.2150 = p.0_1 != -1;
+  return D.2150;
 }

and we no longer get the warning.  I thought maybe I could undo the implicit
rvalue conversion in cp_fold, when it sees implicit_rvalue_p, but that didn't
work.  So currently I'm stuck.  Should we try to figure this out or push aside?

Marek



[PATCH] Update float 128-bit conversions

2022-09-08 Thread Michael Meissner via Gcc-patches
This patch is a rewrite of the patch submitted on August 18th:

| https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599988.html

This patch reworks the conversions between 128-bit binary floating point types.
Previously, we would call rs6000_expand_float128_convert to do all conversions.
Now, we only define the conversions between the same representation that turn
into a NOP.  The appropriate extend or truncate insn is generated, and after
register allocation, it is converted to a move.

This patch also fixes two places where we want to override the external name
for the conversion function, and the wrong optab was used.  Previously,
rs6000_expand_float128_convert would handle the move or generate the call as
needed.  Now, it lets the machine independent code generate the call.  But if
we use the machine independent code to generate the call, we need to update the
name for two optabs where a truncate would be used in terms of converting
between the modes.  This patch updates those two optabs.

While I know you feel the whole area needs to be rewritten, I would think it is
better to make things work incrementally rather than waiting for some grand
rewrite (that may or may not occur).

With the current sources, we don't yet need this patch.  But we will need this
patch when a future patch is submitted that will change the internal __float128
type to use the _Float128 type when long double is IEEE 128-bit.  I'm trying to
break out the smaller patches that each can stand alone, without having a
single larger patch.  This future patch will fix various testsuite issues with
signalling NaNs when long double is IEEE 128-bit.

I tested this patch on:

1)  LE Power10 using --with-cpu=power10 --with-long-double-format=ieee
2)  LE Power10 using --with-cpu=power9  --with-long-double-format=ibm
3)  LE Power10 using --with-cpu=power8  --with-long-double-format=ibm
4)  LE Power10 using --with-cpu=power10 --with-long-double-format=ibm
5)  LE Power9  using --with-cpu=power9  --with-long-double-format=ibm
6)  BE Power7  using --with-cpu=power7  --with-long-double-format=ibm

There were no regressions in the bootstrap process or running the tests.  Can I
check this patch into the trunk?

2022-09-08   Michael Meissner  

gcc/

* config/rs6000/rs6000.cc (init_float128_ieee): Use the correct
float_extend or float_truncate optab based on how the machine converts
between IEEE 128-bit and IBM 128-bit.
* config/rs6000/rs6000.md (IFKF): Delete.
(IFKF_reg): Delete.
(extendiftf2): Rewrite to be a move if IFmode and TFmode are both IBM
128-bit.  Do not run if TFmode is IEEE 128-bit.
(extendifkf2): Delete.
(extendtfkf2): Delete.
(extendtfif2): Delete.
(trunciftf2): Delete.
(truncifkf2): Delete.
(trunckftf2): Delete.
(extendkftf2): Implement conversion of IEEE 128-bit types as a move.
(trunctfif2): Delete.
(trunctfkf2): Implement conversion of IEEE 128-bit types as a move.
(extendtf2_internal): Delete.
(extendtf2_internal): Delete.
---
 gcc/config/rs6000/rs6000.cc |   4 +-
 gcc/config/rs6000/rs6000.md | 177 ++--
 2 files changed, 50 insertions(+), 131 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index a656cb32a47..6f822434ab0 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -11045,11 +11045,11 @@ init_float128_ieee (machine_mode mode)
   set_conv_libfunc (trunc_optab, SFmode, mode, "__trunckfsf2");
   set_conv_libfunc (trunc_optab, DFmode, mode, "__trunckfdf2");
 
-  set_conv_libfunc (sext_optab, mode, IFmode, "__trunctfkf2");
+  set_conv_libfunc (trunc_optab, mode, IFmode, "__trunctfkf2");
   if (mode != TFmode && FLOAT128_IBM_P (TFmode))
set_conv_libfunc (sext_optab, mode, TFmode, "__trunctfkf2");
 
-  set_conv_libfunc (trunc_optab, IFmode, mode, "__extendkftf2");
+  set_conv_libfunc (sext_optab, IFmode, mode, "__extendkftf2");
   if (mode != TFmode && FLOAT128_IBM_P (TFmode))
set_conv_libfunc (trunc_optab, TFmode, mode, "__extendkftf2");
 
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index ad5a4cf2ef8..838d38616b7 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -543,12 +543,6 @@ (define_mode_iterator FMOVE128_GPR [TI
 ; Iterator for 128-bit VSX types for pack/unpack
 (define_mode_iterator FMOVE128_VSX [V1TI KF])
 
-; Iterators for converting to/from TFmode
-(define_mode_iterator IFKF [IF KF])
-
-; Constraints for moving IF/KFmode.
-(define_mode_attr IFKF_reg [(IF "d") (KF "wa")])
-
 ; Whether a floating point move is ok, don't allow SD without hardware FP
 (define_mode_attr fmove_ok [(SF "")
(DF "")
@@ -9097,106 +9091,65 @@ (define_insn "*ieee_128bit_vsx_nabs2_internal"
   "xxlor %x0,%x1,%x2"
   [(set_attr "type" "veclogical")])
 
-;; 

[PATCH v4 1/2] xtensa: Eliminate unused stack frame allocation/freeing

2022-09-08 Thread Takayuki 'January June' Suwa via Gcc-patches
Changes from v3:
  (xtensa_expand_prologue): Changed to exclude debug insns from DF use chain 
analysis.

---

In the example below, 'x' is once placed on the stack frame and then read
into registers as the argument value of bar():

/* example */
struct foo {
  int a, b;
};
extern struct foo bar(struct foo);
struct foo test(void) {
  struct foo x = { 0, 1 };
  return bar(x);
}

Thanks to the dead store elimination, the initialization of 'x' turns into
merely loading the immediates to registers, but corresponding stack frame
growth is not rolled back.  As a result:

;; prereq: the CALL0 ABI
;; before
test:
addisp, sp, -16 // unused stack frame allocation/freeing
movi.n  a2, 0
movi.n  a3, 1
addisp, sp, 16  // because no instructions that refer to
j.l bar, a9 // the stack pointer between the two

This patch eliminates such unused stack frame allocation/freeing:

;; after
test:
movi.n  a2, 0
movi.n  a3, 1
j.l bar, a9

gcc/ChangeLog:

* config/xtensa/xtensa.cc (machine_function): New boolean member as
a flag that controls whether to emit the insns for stack pointer
adjustment inside of the pro/epilogue.
(xtensa_emit_adjust_stack_ptr): New function to share the common
codes and to emit insns if not inhibited.
(xtensa_expand_epilogue): Change to use the function mentioned
above when using the CALL0 ABI.
(xtensa_expand_prologue): Ditto.
And also change to set the inhibit flag used by
xtensa_emit_adjust_stack_ptr() to true if the stack pointer is only
used for its own adjustment.
---
 gcc/config/xtensa/xtensa.cc | 164 ++--
 1 file changed, 80 insertions(+), 84 deletions(-)

diff --git a/gcc/config/xtensa/xtensa.cc b/gcc/config/xtensa/xtensa.cc
index 93ac6562b22..0f586b09dfb 100644
--- a/gcc/config/xtensa/xtensa.cc
+++ b/gcc/config/xtensa/xtensa.cc
@@ -102,6 +102,7 @@ struct GTY(()) machine_function
   int callee_save_size;
   bool frame_laid_out;
   bool epilogue_done;
+  bool inhibit_logues_a1_adjusts;
 };
 
 /* Vector, indexed by hard register number, which contains 1 for a
@@ -3048,7 +3049,7 @@ xtensa_output_literal (FILE *file, rtx x, machine_mode 
mode, int labelno)
 }
 
 static bool
-xtensa_call_save_reg(int regno)
+xtensa_call_save_reg (int regno)
 {
   if (TARGET_WINDOWED_ABI)
 return false;
@@ -3084,7 +3085,7 @@ compute_frame_size (poly_int64 size)
   cfun->machine->callee_save_size = 0;
   for (regno = 0; regno < FIRST_PSEUDO_REGISTER; ++regno)
 {
-  if (xtensa_call_save_reg(regno))
+  if (xtensa_call_save_reg (regno))
cfun->machine->callee_save_size += UNITS_PER_WORD;
 }
 
@@ -3139,6 +3140,49 @@ xtensa_initial_elimination_offset (int from, int to 
ATTRIBUTE_UNUSED)
   return offset;
 }
 
+#define ADJUST_SP_NONE  0x0
+#define ADJUST_SP_NEED_NOTE 0x1
+#define ADJUST_SP_FRAME_PTR 0x2
+static void
+xtensa_emit_adjust_stack_ptr (HOST_WIDE_INT offset, int flags)
+{
+  rtx_insn *insn;
+  rtx ptr = (flags & ADJUST_SP_FRAME_PTR) ? hard_frame_pointer_rtx
+ : stack_pointer_rtx;
+
+  if (cfun->machine->inhibit_logues_a1_adjusts)
+return;
+
+  if (xtensa_simm8 (offset)
+  || xtensa_simm8x256 (offset))
+insn = emit_insn (gen_addsi3 (stack_pointer_rtx, ptr, GEN_INT (offset)));
+  else
+{
+  rtx tmp_reg = gen_rtx_REG (Pmode, A9_REG);
+
+  if (offset < 0)
+   {
+ emit_move_insn (tmp_reg, GEN_INT (-offset));
+ insn = emit_insn (gen_subsi3 (stack_pointer_rtx, ptr, tmp_reg));
+   }
+  else
+   {
+ emit_move_insn (tmp_reg, GEN_INT (offset));
+ insn = emit_insn (gen_addsi3 (stack_pointer_rtx, ptr, tmp_reg));
+   }
+}
+
+  if (flags & ADJUST_SP_NEED_NOTE)
+{
+  rtx note_rtx = gen_rtx_SET (stack_pointer_rtx,
+ plus_constant (Pmode, stack_pointer_rtx,
+offset));
+
+  RTX_FRAME_RELATED_P (insn) = 1;
+  add_reg_note (insn, REG_FRAME_RELATED_EXPR, note_rtx);
+}
+}
+
 /* minimum frame = reg save area (4 words) plus static chain (1 word)
and the total number of words must be a multiple of 128 bits.  */
 #define MIN_FRAME_SIZE (8 * UNITS_PER_WORD)
@@ -3174,17 +3218,30 @@ xtensa_expand_prologue (void)
   int regno;
   HOST_WIDE_INT offset = 0;
   int callee_save_size = cfun->machine->callee_save_size;
+  df_ref ref;
+  bool stack_pointer_needed = frame_pointer_needed
+ || crtl->calls_eh_return;
+
+  /* Check if the function body really needs the stack pointer.  */
+  if (!stack_pointer_needed)
+   for (ref = DF_REG_USE_CHAIN (A1_REG);
+ref; ref = DF_REF_NEXT_REG (ref))
+ if (DF_REF_CLASS (ref) == DF_REF_REGULAR
+ && 

[PATCH] amdgcn: Add support for additional natively supported floating-point operations

2022-09-08 Thread Kwok Cheung Yeung

Hello

This patch adds support for some additional floating-point operations, 
in scalar and vector modes, which are natively supported by the AMD GCN 
instruction set, but haven't been implemented in GCC yet. With the 
exception of frexp, these implement standard RTL names, and should be 
utilised automatically by GCC.


The instructions for the transcendental functions are documented to have 
limited numerical precision, so they are only used if 
unsafe_math_optimizations are enabled for now.


The sin and cos instructions for some reason are scaled by 2*PI radians 
(i.e. 1.0 == 2*PI radians/360 degrees), so their inputs need to be 
scaled by 1/(2*PI) first. I've implemented this as an expander to two 
instructions - one to do the pre-scaling, one to do the sin/cos. 
1/(2*PI) is a builtin constant for GCN, but the syntax to use it in the 
LLVM assembler was wrong - now fixed.


I have also added some extra GCN-specific builtins to access the vector 
versions of some of these operations (to implement vectorized versions 
of library math routines) and to access the frexp operations.


Okay for trunk?

Thanks

KwokFrom 5592c4512212ba74a7a690821650ddcba05df848 Mon Sep 17 00:00:00 2001
From: Kwok Cheung Yeung 
Date: Thu, 8 Sep 2022 17:37:26 +
Subject: [PATCH] amdgcn: Add support for additional natively supported
 floating-point operations

This adds support for the following natively supported floating-point
operations, in scalar and vectorized modes:

floor, ceil, exp2*, log2*, sin*, cos*, ldexp, frexp

* These operations are single-precision float only and are only active
if unsafe_math_optimizations are enabled (due to potential numerical
precision issues).

2022-09-08  Kwok Cheung Yeung  

gcc/
* config/gcn/gcn-builtins.def (FABSVF, LDEXPVF, LDEXPV, FREXPVF_EXP,
FREXPVF_MANT, FREXPV_EXP, FREXPV_MANT): Add new builtins.
* config/gcn/gcn-protos.h (gcn_dconst1over2pi): New prototype.
* config/gcn/gcn-valu.md (MATH_UNOP_1OR2REG, MATH_UNOP_1REG,
MATH_UNOP_TRIG): New iterators.
(math_unop): New attributes.
(2, 2,
2, 2,
2_insn, 2_insn,
ldexp3, ldexp3,
frexp_exp2, frexp_mant2,
frexp_exp2, frexp_mant2): New instructions.
(2, 2): New expanders.
* config/gcn/gcn.cc (init_ext_gcn_constants): Update definition of
dconst1over2pi.
(gcn_dconst1over2pi): New.
(gcn_builtin_type_index): Add entry for v64df type.
(v64df_type_node): New.
(gcn_init_builtin_types): Initialize v64df_type_node.
(gcn_expand_builtin_1): Expand new builtins to instructions.
(print_operand): Fix assembler output for 1/(2*PI) constant.
* config/gcn/gcn.md (unspec): Add new entries.
---
 gcc/config/gcn/gcn-builtins.def |  35 ++
 gcc/config/gcn/gcn-protos.h |   1 +
 gcc/config/gcn/gcn-valu.md  | 181 
 gcc/config/gcn/gcn.cc   | 114 +++-
 gcc/config/gcn/gcn.md   |   4 +-
 5 files changed, 332 insertions(+), 3 deletions(-)

diff --git a/gcc/config/gcn/gcn-builtins.def b/gcc/config/gcn/gcn-builtins.def
index 54e4ea4e953..27691909925 100644
--- a/gcc/config/gcn/gcn-builtins.def
+++ b/gcc/config/gcn/gcn-builtins.def
@@ -59,6 +59,41 @@ DEF_BUILTIN (SQRTF, 3 /*CODE_FOR_sqrtf */,
 _A2 (GCN_BTI_SF, GCN_BTI_SF),
 gcn_expand_builtin_1)
 
+DEF_BUILTIN (FABSVF, 3 /*CODE_FOR_fabsvf */,
+"fabsvf", B_INSN,
+_A2 (GCN_BTI_V64SF, GCN_BTI_V64SF),
+gcn_expand_builtin_1)
+
+DEF_BUILTIN (LDEXPVF, 3 /*CODE_FOR_ldexpvf */,
+"ldexpvf", B_INSN,
+_A3 (GCN_BTI_V64SF, GCN_BTI_V64SF, GCN_BTI_V64SI),
+gcn_expand_builtin_1)
+
+DEF_BUILTIN (LDEXPV, 3 /*CODE_FOR_ldexpv */,
+"ldexpv", B_INSN,
+_A3 (GCN_BTI_V64DF, GCN_BTI_V64DF, GCN_BTI_V64SI),
+gcn_expand_builtin_1)
+
+DEF_BUILTIN (FREXPVF_EXP, 3 /*CODE_FOR_frexpvf_exp */,
+"frexpvf_exp", B_INSN,
+_A2 (GCN_BTI_V64SI, GCN_BTI_V64SF),
+gcn_expand_builtin_1)
+
+DEF_BUILTIN (FREXPVF_MANT, 3 /*CODE_FOR_frexpvf_mant */,
+"frexpvf_mant", B_INSN,
+_A2 (GCN_BTI_V64SF, GCN_BTI_V64SF),
+gcn_expand_builtin_1)
+
+DEF_BUILTIN (FREXPV_EXP, 3 /*CODE_FOR_frexpv_exp */,
+"frexpv_exp", B_INSN,
+_A2 (GCN_BTI_V64SI, GCN_BTI_V64DF),
+gcn_expand_builtin_1)
+
+DEF_BUILTIN (FREXPV_MANT, 3 /*CODE_FOR_frexpv_mant */,
+"frexpv_mant", B_INSN,
+_A2 (GCN_BTI_V64DF, GCN_BTI_V64DF),
+gcn_expand_builtin_1)
+
 DEF_BUILTIN (CMP_SWAP, -1,
"cmp_swap", B_INSN,
_A4 (GCN_BTI_UINT, GCN_BTI_VOIDPTR, GCN_BTI_UINT, GCN_BTI_UINT),
diff --git a/gcc/config/gcn/gcn-protos.h b/gcc/config/gcn/gcn-protos.h
index 38197b929fd..ca804609c09 100644
--- a/gcc/config/gcn/gcn-protos.h
+++ b/gcc/config/gcn/gcn-protos.h
@@ -54,6 +54,7 @@ extern 

Re: [PATCH] optc-save-gen.awk: adjust generated array compare

2022-09-08 Thread Jason Merrill via Gcc-patches

On 9/8/22 14:01, Martin Liška wrote:

On 9/8/22 18:23, Jason Merrill wrote:

It seems to me that the warning is pointing out that comparing the address of 
the array is nonsensical, and we should remove it and just have the memcmp.


Yes, thanks for the pointer. We should always compare the array types with 
memcmp.

Ready to be installed?


OK.



stddef.h: Add C2x unreachable macro

2022-09-08 Thread Joseph Myers
C2x adds a macro unreachable to stddef.h, with the same semantics as
__builtin_unreachable.  Define this macro accordingly.

Bootstrapped with no regressions for x86_64-pc-linux-gnu.  OK to commit?

gcc/
* ginclude/stddef.h [__STDC_VERSION__ > 201710L] (unreachable):
New macro.

gcc/testsuite/
* gcc.dg/c11-unreachable-1.c, gcc.dg/c2x-unreachable-1.c: New
tests.

diff --git a/gcc/ginclude/stddef.h b/gcc/ginclude/stddef.h
index 315ff786694..3d29213e8f1 100644
--- a/gcc/ginclude/stddef.h
+++ b/gcc/ginclude/stddef.h
@@ -451,6 +451,10 @@ typedef struct {
 #endif
 #endif /* C23.  */
 
+#if defined __STDC_VERSION__ && __STDC_VERSION__ > 201710L
+#define unreachable() (__builtin_unreachable ())
+#endif
+
 #endif /* _STDDEF_H was defined this time */
 
 #endif /* !_STDDEF_H && !_STDDEF_H_ && !_ANSI_STDDEF_H && !__STDDEF_H__
diff --git a/gcc/testsuite/gcc.dg/c11-unreachable-1.c 
b/gcc/testsuite/gcc.dg/c11-unreachable-1.c
new file mode 100644
index 000..28e48392ed1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/c11-unreachable-1.c
@@ -0,0 +1,9 @@
+/* Test unreachable not defined in  for C11.  */
+/* { dg-do preprocess } */
+/* { dg-options "-std=c11 -pedantic-errors" } */
+
+#include 
+
+#ifdef unreachable
+#error "unreachable defined"
+#endif
diff --git a/gcc/testsuite/gcc.dg/c2x-unreachable-1.c 
b/gcc/testsuite/gcc.dg/c2x-unreachable-1.c
new file mode 100644
index 000..468f1f87ebb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/c2x-unreachable-1.c
@@ -0,0 +1,29 @@
+/* Test unreachable in  for C2x.  */
+/* { dg-do run } */
+/* { dg-options "-std=c2x -pedantic-errors -O2" } */
+
+#include 
+
+#ifndef unreachable
+#error "unreachable not defined"
+#endif
+
+extern void *p;
+extern __typeof__ (unreachable ()) *p;
+
+volatile int x = 1;
+
+extern void not_defined (void);
+
+extern void exit (int);
+
+int
+main ()
+{
+  if (x == 2)
+{
+  unreachable ();
+  not_defined ();
+}
+  exit (0);
+}

-- 
Joseph S. Myers
jos...@codesourcery.com


Re: [PATCH] : [gcc/config/rs600/rs6000.cc][Fix typo] Add parentheses for if statement

2022-09-08 Thread Akari Takahashi via Gcc-patches
Hello:
I sent a pull request. Please review and merge.

Branch name:
patch/rc6000/fixtypo

Log:
commit d55c6c49e0f7760c0b85ee74eb9c2b4e68275a26 (HEAD -> patch/rc6000/fixtypo)
Author: Takahashi Akari 
Date:   Fri Sep 9 03:37:34 2022 +0900

[Fix typo] Add parentheses for if statement in line 18117.

Diff:
git diff 30c811f2bac73e63e0b461ba7ed3805b77898798
d55c6c49e0f7760c0b85ee74eb9c2b4e68275a26
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index a656cb32a47..bcf634a146d 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -18114,7 +18114,7 @@ get_memref_parts (rtx mem, rtx *base,
HOST_WIDE_INT *offset,
  HOST_WIDE_INT *size)
 {
   rtx addr_rtx;
-  if MEM_SIZE_KNOWN_P (mem)
+  if (MEM_SIZE_KNOWN_P (mem))
 *size = MEM_SIZE (mem);
   else
 return false;

Takahashi Akari
https://github.com/takahashi-akari
GPG Key: 9DD8 F529 83A0 3182 D318 6184 9309 E8D2 2FD4 4365

On Thu, Sep 8, 2022 at 8:20 AM Akari Takahashi
 wrote:
>
> Hello:
> I am very interested in GCC and have joined the FSF membership.
> I found a small bug in the latest source code, so I report it.
>
> Patch:
> [Fix typo]Add parentheses for if statement in line 18117.
> gcc/config/rs600/rs6000.cc
>
> Diff:
> --
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index bcf634a146d..a656cb32a47 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -18114,7 +18114,7 @@ get_memref_parts (rtx mem, rtx *base,
> HOST_WIDE_INT *offset,
>   HOST_WIDE_INT *size)
>  {
>rtx addr_rtx;
> -  if (MEM_SIZE_KNOWN_P (mem))
> +  if MEM_SIZE_KNOWN_P (mem)
>  *size = MEM_SIZE (mem);
>else
>  return false;
> --
>
> Takahashi Akari


[committed] libstdc++: Add always_inline attribute to std::byte operators

2022-09-08 Thread Jonathan Wakely via Gcc-patches
These are all trivial bit-twiddling operations that don't need the
overhead of a function call in unoptimized code.

Tested powerpc64le-linux, pushed to trunk.

-- >8 --

libstdc++-v3/ChangeLog:

* include/c_global/cstddef (byte): Add always_inline attribute
to all operator overloads.
(to_integer): Add always_inline attribute.
---
 libstdc++-v3/include/c_global/cstddef | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/libstdc++-v3/include/c_global/cstddef 
b/libstdc++-v3/include/c_global/cstddef
index 4970c2dfcfb..df2d88a2ce5 100644
--- a/libstdc++-v3/include/c_global/cstddef
+++ b/libstdc++-v3/include/c_global/cstddef
@@ -119,55 +119,66 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 using __byte_op_t = typename __byte_operand<_IntegerType>::__type;
 
   template
+[[__gnu__::__always_inline__]]
 constexpr __byte_op_t<_IntegerType>
 operator<<(byte __b, _IntegerType __shift) noexcept
 { return (byte)(unsigned char)((unsigned)__b << __shift); }
 
   template
+[[__gnu__::__always_inline__]]
 constexpr __byte_op_t<_IntegerType>
 operator>>(byte __b, _IntegerType __shift) noexcept
 { return (byte)(unsigned char)((unsigned)__b >> __shift); }
 
+  [[__gnu__::__always_inline__]]
   constexpr byte
   operator|(byte __l, byte __r) noexcept
   { return (byte)(unsigned char)((unsigned)__l | (unsigned)__r); }
 
+  [[__gnu__::__always_inline__]]
   constexpr byte
   operator&(byte __l, byte __r) noexcept
   { return (byte)(unsigned char)((unsigned)__l & (unsigned)__r); }
 
+  [[__gnu__::__always_inline__]]
   constexpr byte
   operator^(byte __l, byte __r) noexcept
   { return (byte)(unsigned char)((unsigned)__l ^ (unsigned)__r); }
 
+  [[__gnu__::__always_inline__]]
   constexpr byte
   operator~(byte __b) noexcept
   { return (byte)(unsigned char)~(unsigned)__b; }
 
   template
+[[__gnu__::__always_inline__]]
 constexpr __byte_op_t<_IntegerType>&
 operator<<=(byte& __b, _IntegerType __shift) noexcept
 { return __b = __b << __shift; }
 
   template
+[[__gnu__::__always_inline__]]
 constexpr __byte_op_t<_IntegerType>&
 operator>>=(byte& __b, _IntegerType __shift) noexcept
 { return __b = __b >> __shift; }
 
+  [[__gnu__::__always_inline__]]
   constexpr byte&
   operator|=(byte& __l, byte __r) noexcept
   { return __l = __l | __r; }
 
+  [[__gnu__::__always_inline__]]
   constexpr byte&
   operator&=(byte& __l, byte __r) noexcept
   { return __l = __l & __r; }
 
+  [[__gnu__::__always_inline__]]
   constexpr byte&
   operator^=(byte& __l, byte __r) noexcept
   { return __l = __l ^ __r; }
 
   template
-[[nodiscard]]
+[[nodiscard,__gnu__::__always_inline__]]
 constexpr _IntegerType
 to_integer(__byte_op_t<_IntegerType> __b) noexcept
 { return _IntegerType(__b); }
-- 
2.37.3



[committed] libstdc++: Find make_error_code and make_error_condition via ADL only

2022-09-08 Thread Jonathan Wakely via Gcc-patches
Tested powerpc64le-linux, pushed to trunk.

-- >8 --

The new proposed resolution for LWG 3629 says that std::error_code and
std::error_condition should only use ADL to find their customization
points. This means we need to use a poison pill to prevent lookup from
finding overloads in the enclosing namespaces.

We can also remove the forward declarations of std::make_error_code and
std::make_error_condition, because they aren't needed now. ADL can find
them anyway (when std is an associated namespace), and unqualified name
lookup will not (and should not) find them.

libstdc++-v3/ChangeLog:

* include/std/system_error (__adl_only::make_error_code): Add
deleted function.
(__adl_only::make_error_condition): Likewise.
(error_code::error_code(ErrorCodeEnum)): Add using-declaration
for deleted function.
(error_condition::error_condition(ErrorConditionEnum)):
Likewise.
* testsuite/19_diagnostics/error_code/cons/lwg3629.cc: New test.
* testsuite/19_diagnostics/error_condition/cons/lwg3629.cc: New test.
---
 libstdc++-v3/include/std/system_error | 18 +--
 .../19_diagnostics/error_code/cons/lwg3629.cc | 48 +++
 .../error_condition/cons/lwg3629.cc   | 48 +++
 3 files changed, 109 insertions(+), 5 deletions(-)
 create mode 100644 
libstdc++-v3/testsuite/19_diagnostics/error_code/cons/lwg3629.cc
 create mode 100644 
libstdc++-v3/testsuite/19_diagnostics/error_condition/cons/lwg3629.cc

diff --git a/libstdc++-v3/include/std/system_error 
b/libstdc++-v3/include/std/system_error
index 050439427cc..e12bb2f0e1e 100644
--- a/libstdc++-v3/include/std/system_error
+++ b/libstdc++-v3/include/std/system_error
@@ -195,7 +195,11 @@ _GLIBCXX_END_INLINE_ABI_NAMESPACE(_V2)
* @{
*/
 
-  error_code make_error_code(errc) noexcept;
+namespace __adl_only
+{
+  void make_error_code() = delete;
+  void make_error_condition() = delete;
+}
 
   /** Class error_code
*
@@ -231,7 +235,10 @@ _GLIBCXX_END_INLINE_ABI_NAMESPACE(_V2)
 template>
   error_code(_ErrorCodeEnum __e) noexcept
-  { *this = make_error_code(__e); }
+  {
+   using __adl_only::make_error_code;
+   *this = make_error_code(__e);
+  }
 
 error_code(const error_code&) = default;
 error_code& operator=(const error_code&) = default;
@@ -330,8 +337,6 @@ _GLIBCXX_END_INLINE_ABI_NAMESPACE(_V2)
 operator<<(basic_ostream<_CharT, _Traits>& __os, const error_code& __e)
 { return (__os << __e.category().name() << ':' << __e.value()); }
 
-  error_condition make_error_condition(errc) noexcept;
-
   /** Class error_condition
*
* This class represents error conditions that may be visible at an API
@@ -363,7 +368,10 @@ _GLIBCXX_END_INLINE_ABI_NAMESPACE(_V2)
 template>
   error_condition(_ErrorConditionEnum __e) noexcept
-  { *this = make_error_condition(__e); }
+  {
+   using __adl_only::make_error_condition;
+   *this = make_error_condition(__e);
+  }
 
 error_condition(const error_condition&) = default;
 error_condition& operator=(const error_condition&) = default;
diff --git a/libstdc++-v3/testsuite/19_diagnostics/error_code/cons/lwg3629.cc 
b/libstdc++-v3/testsuite/19_diagnostics/error_code/cons/lwg3629.cc
new file mode 100644
index 000..b1e0b7f0c81
--- /dev/null
+++ b/libstdc++-v3/testsuite/19_diagnostics/error_code/cons/lwg3629.cc
@@ -0,0 +1,48 @@
+// { dg-do compile { target c++11 } }
+
+// 3629. make_error_code and make_error_condition are customization points
+// Verify that make_error_code is looked up using ADL only.
+
+namespace user
+{
+  struct E1;
+}
+
+// N.B. not in associated namespace of E1, and declared before .
+user::E1 make_error_code(user::E1);
+
+#include  // declares std::make_error_code(future_errc)
+#include 
+
+namespace user
+{
+  struct E1
+  {
+operator std::error_code() const;
+  };
+
+  struct E2
+  {
+operator std::future_errc() const;
+  };
+
+  struct E3
+  {
+operator std::errc() const;
+  };
+}
+
+template<> struct std::is_error_code_enum : std::true_type { };
+template<> struct std::is_error_code_enum : std::true_type { };
+template<> struct std::is_error_code_enum : std::true_type { };
+
+// ::make_error_code(E1) should not be found by name lookup.
+std::error_code e1( user::E1{} ); // { dg-error "here" }
+
+// std::make_error_code(errc) should not be found by name lookup.
+std::error_code e2( user::E2{} ); // { dg-error "here" }
+
+// std::make_error_code(future_errc) should not be found by name lookup.
+std::error_code e3( user::E3{} ); // { dg-error "here" }
+
+// { dg-error "use of deleted function" "" { target *-*-* } 0 }
diff --git 
a/libstdc++-v3/testsuite/19_diagnostics/error_condition/cons/lwg3629.cc 
b/libstdc++-v3/testsuite/19_diagnostics/error_condition/cons/lwg3629.cc
new file mode 100644
index 000..e34b53de8a1
--- /dev/null
+++ 

Re: [PATCH] libstdc++: Refactor implementation of operator+ for std::string

2022-09-08 Thread Jonathan Wakely via Gcc-patches
On Thu, 8 Sep 2022, 18:51 François Dumont via Libstdc++, <
libstd...@gcc.gnu.org> wrote:

> On 05/09/22 20:30, Will Hawkins wrote:
> > Based on Jonathan's work, here is a patch for the implementation of
> operator+
> > on std::string that makes sure we always use the best allocation
> strategy.
> >
> > I have attempted to learn from all the feedback that I got on a previous
> > submission -- I hope I did the right thing.
> >
> > Passes abi and conformance testing on x86-64 trunk.
> >
> > Sincerely,
> > Will
> >
> > -- >8 --
> >
> > Create a single function that performs one-allocation string
> concatenation
> > that can be used by various different version of operator+.
> >
> > libstdc++-v3/ChangeLog:
> >
> >   * include/bits/basic_string.h:
> >   Add common function that performs single-allocation string
> >   concatenation. (__str_cat)
> >   Use __str_cat to perform optimized operator+, where relevant.
> >   * include/bits/basic_string.tcc::
> >   Remove single-allocation implementation of operator+.
> >
> > Signed-off-by: Will Hawkins 
> > ---
> >   libstdc++-v3/include/bits/basic_string.h   | 66 --
> >   libstdc++-v3/include/bits/basic_string.tcc | 41 --
> >   2 files changed, 49 insertions(+), 58 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/bits/basic_string.h
> b/libstdc++-v3/include/bits/basic_string.h
> > index 0df64ea98ca..4078651fadb 100644
> > --- a/libstdc++-v3/include/bits/basic_string.h
> > +++ b/libstdc++-v3/include/bits/basic_string.h
> > @@ -3481,6 +3481,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >   _GLIBCXX_END_NAMESPACE_CXX11
> >   #endif
> >
> > +  template
> > +_GLIBCXX20_CONSTEXPR
> > +inline _Str
> > +__str_concat(typename _Str::value_type const* __lhs,
> > +  typename _Str::size_type __lhs_len,
> > +  typename _Str::value_type const* __rhs,
> > +  typename _Str::size_type __rhs_len,
> > +  typename _Str::allocator_type const& __a)
> > +{
> > +  typedef typename _Str::allocator_type allocator_type;
> > +  typedef __gnu_cxx::__alloc_traits _Alloc_traits;
> > +  _Str __str(_Alloc_traits::_S_select_on_copy(__a));
> > +  __str.reserve(__lhs_len + __rhs_len);
> > +  __str.append(__lhs, __lhs_len);
> > +  __str.append(__rhs, __rhs_len);
> > +  return __str;
> > +}
> > +
> > // operator+
> > /**
> >  *  @brief  Concatenate two strings.
> > @@ -3490,13 +3508,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
> >  */
> > template
> >   _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> > -basic_string<_CharT, _Traits, _Alloc>
> > +inline basic_string<_CharT, _Traits, _Alloc>
> >   operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
> > const basic_string<_CharT, _Traits, _Alloc>& __rhs)
> >   {
> > -  basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
> > -  __str.append(__rhs);
> > -  return __str;
> > +  typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> > +  return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
> > +  __rhs.c_str(), __rhs.size(),
>
> You should use data() rather than c_str() here and all other operators.
>
> It is currently the same but is more accurate in your context. Maybe one
> day it will make a difference.
>


I don't think so, that would be a major breaking change, for no benefit. I
think it's safe to assume they will always stay equivalent now.



> > +  __lhs.get_allocator());
> >   }
> >
> > /**
> > @@ -3507,9 +3526,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
> >  */
> > template
> >   _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> > -basic_string<_CharT,_Traits,_Alloc>
> > +inline basic_string<_CharT,_Traits,_Alloc>
>
> Why inlining ?
>
> I guess it is done this way to limit code bloat.
>
> >   operator+(const _CharT* __lhs,
> > -   const basic_string<_CharT,_Traits,_Alloc>& __rhs);
> > +   const basic_string<_CharT,_Traits,_Alloc>& __rhs)
> > +{
> > +  __glibcxx_requires_string(__lhs);
> > +  typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> > +  return std::__str_concat<_Str>(__lhs, _Traits::length(__lhs),
> > +  __rhs.c_str(), __rhs.size(),
> > +  __rhs.get_allocator());
> > +}
> >
> > /**
> >  *  @brief  Concatenate character and string.
> > @@ -3519,8 +3545,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
> >  */
> > template
> >   _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
> > -basic_string<_CharT,_Traits,_Alloc>
> > -operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>&
> __rhs);
> > +inline basic_string<_CharT,_Traits,_Alloc>
> > +operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>&
> __rhs)
> > +{
> > +  typedef basic_string<_CharT, _Traits, _Alloc> _Str;
> > +  return 

Re: [PATCH] optc-save-gen.awk: adjust generated array compare

2022-09-08 Thread Martin Liška
On 9/8/22 18:23, Jason Merrill wrote:
> It seems to me that the warning is pointing out that comparing the address of 
> the array is nonsensical, and we should remove it and just have the memcmp.

Yes, thanks for the pointer. We should always compare the array types with 
memcmp.

Ready to be installed?
Thanks,
MartinFrom 32e875bf395ebb0444ced281c5e7634474100fb8 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Thu, 8 Sep 2022 20:00:33 +0200
Subject: [PATCH] opts: always compare array option values with memcmp

gcc/ChangeLog:

	* optc-save-gen.awk: Always compare array option values with memcmp.
---
 gcc/optc-save-gen.awk | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gcc/optc-save-gen.awk b/gcc/optc-save-gen.awk
index 233d1fbb637..49065ced0b3 100644
--- a/gcc/optc-save-gen.awk
+++ b/gcc/optc-save-gen.awk
@@ -1093,8 +1093,7 @@ for (i = 0; i < n_target_array; i++) {
 	name = var_target_array[i]
 	size = var_target_array_size[i]
 	type = var_target_array_type[i]
-	print "  if (ptr1->" name" != ptr2->" name "";
-	print "  || memcmp (ptr1->" name ", ptr2->" name ", " size " * sizeof(" type ")))"
+	print "  if (memcmp (ptr1->" name ", ptr2->" name ", " size " * sizeof(" type ")))"
 	print "return false;";
 }
 for (i = 0; i < n_target_val; i++) {
-- 
2.37.3



Re: [PATCH] libstdc++: Refactor implementation of operator+ for std::string

2022-09-08 Thread François Dumont via Gcc-patches

On 05/09/22 20:30, Will Hawkins wrote:

Based on Jonathan's work, here is a patch for the implementation of operator+
on std::string that makes sure we always use the best allocation strategy.

I have attempted to learn from all the feedback that I got on a previous
submission -- I hope I did the right thing.

Passes abi and conformance testing on x86-64 trunk.

Sincerely,
Will

-- >8 --

Create a single function that performs one-allocation string concatenation
that can be used by various different version of operator+.

libstdc++-v3/ChangeLog:

* include/bits/basic_string.h:
Add common function that performs single-allocation string
concatenation. (__str_cat)
Use __str_cat to perform optimized operator+, where relevant.
* include/bits/basic_string.tcc::
Remove single-allocation implementation of operator+.

Signed-off-by: Will Hawkins 
---
  libstdc++-v3/include/bits/basic_string.h   | 66 --
  libstdc++-v3/include/bits/basic_string.tcc | 41 --
  2 files changed, 49 insertions(+), 58 deletions(-)

diff --git a/libstdc++-v3/include/bits/basic_string.h 
b/libstdc++-v3/include/bits/basic_string.h
index 0df64ea98ca..4078651fadb 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -3481,6 +3481,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
  _GLIBCXX_END_NAMESPACE_CXX11
  #endif
  
+  template

+_GLIBCXX20_CONSTEXPR
+inline _Str
+__str_concat(typename _Str::value_type const* __lhs,
+typename _Str::size_type __lhs_len,
+typename _Str::value_type const* __rhs,
+typename _Str::size_type __rhs_len,
+typename _Str::allocator_type const& __a)
+{
+  typedef typename _Str::allocator_type allocator_type;
+  typedef __gnu_cxx::__alloc_traits _Alloc_traits;
+  _Str __str(_Alloc_traits::_S_select_on_copy(__a));
+  __str.reserve(__lhs_len + __rhs_len);
+  __str.append(__lhs, __lhs_len);
+  __str.append(__rhs, __rhs_len);
+  return __str;
+}
+
// operator+
/**
 *  @brief  Concatenate two strings.
@@ -3490,13 +3508,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
 */
template
  _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-basic_string<_CharT, _Traits, _Alloc>
+inline basic_string<_CharT, _Traits, _Alloc>
  operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
  const basic_string<_CharT, _Traits, _Alloc>& __rhs)
  {
-  basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
-  __str.append(__rhs);
-  return __str;
+  typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+  return std::__str_concat<_Str>(__lhs.c_str(), __lhs.size(),
+__rhs.c_str(), __rhs.size(),


You should use data() rather than c_str() here and all other operators.

It is currently the same but is more accurate in your context. Maybe one 
day it will make a difference.



+__lhs.get_allocator());
  }
  
/**

@@ -3507,9 +3526,16 @@ _GLIBCXX_END_NAMESPACE_CXX11
 */
template
  _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-basic_string<_CharT,_Traits,_Alloc>
+inline basic_string<_CharT,_Traits,_Alloc>


Why inlining ?

I guess it is done this way to limit code bloat.


  operator+(const _CharT* __lhs,
- const basic_string<_CharT,_Traits,_Alloc>& __rhs);
+ const basic_string<_CharT,_Traits,_Alloc>& __rhs)
+{
+  __glibcxx_requires_string(__lhs);
+  typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+  return std::__str_concat<_Str>(__lhs, _Traits::length(__lhs),
+__rhs.c_str(), __rhs.size(),
+__rhs.get_allocator());
+}
  
/**

 *  @brief  Concatenate character and string.
@@ -3519,8 +3545,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
 */
template
  _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR
-basic_string<_CharT,_Traits,_Alloc>
-operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs);
+inline basic_string<_CharT,_Traits,_Alloc>
+operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Alloc>& __rhs)
+{
+  typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+  return std::__str_concat<_Str>(__builtin_addressof(__lhs), 1,
+__rhs.c_str(), __rhs.size(),
+__rhs.get_allocator());
+}
  
/**

 *  @brief  Concatenate string and C string.
@@ -3534,11 +3566,12 @@ _GLIBCXX_END_NAMESPACE_CXX11
  operator+(const basic_string<_CharT, _Traits, _Alloc>& __lhs,
  const _CharT* __rhs)
  {
-  basic_string<_CharT, _Traits, _Alloc> __str(__lhs);
-  __str.append(__rhs);
-  return __str;
+  __glibcxx_requires_string(__rhs);
+  typedef basic_string<_CharT, _Traits, _Alloc> _Str;
+  return 

Re: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support.

2022-09-08 Thread Jason Merrill via Gcc-patches

On 9/1/22 18:22, Eugene Rozenfeld wrote:

Jason,

I made another small change in addressing your feedback (attached).

Thanks,

Eugene

-Original Message-
From: Gcc-patches  On 
Behalf Of Eugene Rozenfeld via Gcc-patches
Sent: Thursday, September 01, 2022 1:49 PM
To: Jason Merrill ; gcc-patches@gcc.gnu.org
Cc: Andi Kleen ; Jan Hubicka 
Subject: RE: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator 
support.

Jason,

Thank you for your review. You are correct, we only need to check 
has_discriminator for the statement that's on the same line.
I updated the patch (attached).

Thanks,

Eugene

-Original Message-
From: Jason Merrill 
Sent: Thursday, August 18, 2022 6:55 PM
To: Eugene Rozenfeld ; gcc-patches@gcc.gnu.org
Cc: Andi Kleen ; Jan Hubicka 
Subject: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator 
support.

On 8/3/22 17:25, Eugene Rozenfeld wrote:

One more ping for this patch
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.
gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.htmldata=0
5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da81
85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637964709325691951
%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=K%2BMx6jelnED3n%2Be2dT
DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3Dreserved=0

CC Jason since this changes discriminators emitted in dwarf.

Thanks,

Eugene

-Original Message-
From: Eugene Rozenfeld
Sent: Monday, June 27, 2022 12:45 PM
To: gcc-patches@gcc.gnu.org; Andi Kleen ; Jan
Hubicka 
Subject: RE: [PING][PATCH] Add instruction level discriminator support.

Another ping for 
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.htmldata=05%7C01%7CEugene.Rozenfeld%40microsoft.com%7Cf217ebc45428465857bd08da8c5b6fb2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637976621612503972%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=b0kTdzWRyiwdtcEFasyNlKv1vj%2FqwnipN3776C7xWcg%3Dreserved=0
 .

I got a review from Andi 
(https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596549.htmldata=05%7C01%7CEugene.Rozenfeld%40microsoft.com%7Cf217ebc45428465857bd08da8c5b6fb2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637976621612503972%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=qxjBUCcGiKXtR4%2BOJq%2FFQN11C2M6BBurTguOBOjWJDw%3Dreserved=0)
 but I also need a review from someone who can approve the changes.

Thanks,

Eugene

-Original Message-
From: Eugene Rozenfeld
Sent: Friday, June 10, 2022 12:03 PM
To: gcc-patches@gcc.gnu.org; Andi Kleen ; Jan
Hubicka 
Subject: [PING][PATCH] Add instruction level discriminator support.

Hello,

I'd like to ping this patch:
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.
gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.htmldata=0
5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da81
85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637964709325691951
%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=K%2BMx6jelnED3n%2Be2dT
DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3Dreserved=0

Thanks,

Eugene

-Original Message-
From: Gcc-patches
 On Behalf Of
Eugene Rozenfeld via Gcc-patches
Sent: Thursday, June 02, 2022 12:22 AM
To: gcc-patches@gcc.gnu.org; Andi Kleen ; Jan
Hubicka 
Subject: [EXTERNAL] [PATCH] Add instruction level discriminator support.

This is the first in a series of patches to enable discriminator support in 
AutoFDO.

This patch switches to tracking discriminators per statement/instruction 
instead of per basic block. Tracking per basic block was problematic since not 
all statements in a basic block needed a discriminator and, also, later 
optimizations could move statements between basic blocks making correlation 
during AutoFDO compilation unreliable. Tracking per statement also allows us to 
assign different discriminators to multiple function calls in the same basic 
block. A subsequent patch will add that support.

The idea of this patch is based on commit
4c311d95cf6d9519c3c20f641cc77af7df491fdf
by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different 
approach. In Dehao's work special (normally unused) location ids and side 
tables were used to keep track of locations with discriminators. Things have 
changed since then and I don't think we have unused location ids anymore. 
Instead, I made discriminators a part of ad-hoc locations.

The difference from Dehao's work also includes support for discriminator 
reading/writing in lto streaming and in modules.

Tested on x86_64-pc-linux-gnu.



@@ -1190,12 +1217,12 @@ assign_discriminators (void)
  || (last && 

Re: [PATCH v2] c++: Fix type completeness checks for type traits [PR106838]

2022-09-08 Thread Jonathan Wakely via Gcc-patches
On Thu, 8 Sept 2022 at 17:12, Jason Merrill  wrote:
>
> On 9/8/22 08:56, Jonathan Wakely wrote:
> > On Wed, 7 Sept 2022 at 16:11, Jason Merrill wrote:
> >>
> >> On 9/7/22 08:18, Jonathan Wakely wrote:
> >>> @@ -12080,23 +12098,37 @@ finish_trait_expr (location_t loc, 
> >>> cp_trait_kind kind, tree type1, tree type2)
> >>>case CPTK_HAS_TRIVIAL_COPY:
> >>>case CPTK_HAS_TRIVIAL_DESTRUCTOR:
> >>>case CPTK_HAS_UNIQUE_OBJ_REPRESENTATIONS:
> >>> -case CPTK_HAS_VIRTUAL_DESTRUCTOR:
> >>> -case CPTK_IS_ABSTRACT:
> >>> +  if (!check_trait_type (type1))
> >>> + return error_mark_node;
> >>> +  break;
> >>> +
> >>>case CPTK_IS_AGGREGATE:
> >>
> >> Hmm, why does std::is_aggregate require a complete array element type?
> >> An array is an aggregate regardless of its element type.  I'd think it
> >> should be kind 4 instead.
> >
> > That makes sense. It doesn't match the precondition for the library
> > trait in the standard, but violating that library precondition is
> > undefined, not ill-formed, so it seems harmless for libstdc++ to also
> > relax that precondition and allow arrays of incomplete type.
> >
> >>
> >>> -case CPTK_IS_EMPTY:
> >>> -case CPTK_IS_FINAL:
> >>>case CPTK_IS_LITERAL_TYPE:
> >>>case CPTK_IS_POD:
> >>> -case CPTK_IS_POLYMORPHIC:
> >>>case CPTK_IS_STD_LAYOUT:
> >>>case CPTK_IS_TRIVIAL:
> >>>case CPTK_IS_TRIVIALLY_COPYABLE:
> >>> -  if (!check_trait_type (type1))
> >>> +  if (!check_trait_type (type1, /* kind = */ 2))
> >>> + return error_mark_node;
> >>> +  break;
> >>> +
> >>> +case CPTK_IS_EMPTY:
> >>
> >> I also wonder why std::is_empty excludes unions, since there can now be
> >> an empty field of union type with the addition of [[no_unique_address]]?
> >
> > True, although the main purpose of is_empty is to decide whether to
> > derive from it to get the EBO. With [[no_unique_address]] there's no
> > reason to do that, you can just add the attribute unconditionally and if
> > it's not empty, the attribute is a no-op. So I find it hard to care
> > about this case :-)
> >
> > If the FE trait requires unions to be complete, then the library trait
> > would need to special-case unions to not use the FE trait, so that it
> > can still give the same answer (i.e. false) for incomplete union types.
> > I don't think changing this one has any advantage.
> >
> >>
> >>> +case CPTK_IS_POLYMORPHIC:
> >>> +case CPTK_IS_ABSTRACT:
> >>> +case CPTK_HAS_VIRTUAL_DESTRUCTOR:
> >>> +  if (!check_trait_type (type1, /* kind = */ 3))
> >>> + return error_mark_node;
> >>> +  break;
> >>> +
> >>> +case CPTK_IS_FINAL:
> >>
> >> And I wonder a bit why std::is_final cares about the completeness of
> >> unions, which can't be (or have) base classes, but I guess you can still
> >> ask about whether the keyword was present, even though it has no effect.
> >
> > Yes, as currently specified, it tells you whether the specifier was
> > present, nothing more.
> >
> > Thanks for the review, here's an updated patch that moves __is_aggregate
> > to kind 4. I didn't change __is_empty or __is_final though, did you want
> > those changed?
> >
> > Tested x86_64-linux.
>
> That's fine, the patch is OK with the typo fix below:

Oops, I'll fix that - thanks.


>
> > -- >8 --
> >
> > The check_trait_type function is used for a number of different type
> > traits that have different requirements on their arguments. For example,
> > __is_constructible allows arrays of unknown bound even if the array
> > element is an incomplete type, but __is_aggregate does not, it always
> > requires the array element type to be complete. Other traits have
> > different requirements again, e.g. __is_empty allows incomplete unions,
> > and arrays (of known or unknown bound) of incomplete types.
> >
> > This alters the check_trait_type function to take an additional KIND
> > parameter which indicates which set of type trait requirements to check.
> >
> > As noted in a comment, the requirements for __is_aggregate deviate from
> > the ones for std::is_aggregate in the standard. It's not necessary for
> > the elements of an array to be complete types, because arrays are always
> > aggregates.
> >
> > The type_has_virtual_destructor change is needed to avoid an ICE.
> > Previously it could never be called for incomplete union types as they
> > were (incorrectly) rejected by check_trait_type.
> >
> > This change causes some additional diagnostics in some libstdc++ tests,
> > where the front end was not previously complaining about invalid types
> > that the library assertions diagnosed. We should consider removing the
> > library assertions from traits where the front end implements the
> > correct checks now.
> >
> >   PR c++/106838
> >
> > gcc/cp/ChangeLog:
> >
> >   * class.cc (type_has_virtual_destructor): Return false for
> >   union types.
> >   * semantics.cc (check_trait_type): Add KIND 

Re: [PATCH] optc-save-gen.awk: adjust generated array compare

2022-09-08 Thread Jason Merrill via Gcc-patches

On 9/8/22 11:29, Chung-Lin Tang wrote:

Hi Joseph,
Jan-Benedict reported a build-bot error for the nios2 port under 
--enable-werror-always:

options-save.cc: In function 'bool cl_target_option_eq(const cl_target_option*, 
const cl_target_option*)':
options-save.cc:9291:38: error: comparison between two arrays 
[-Werror=array-compare]
  9291 |   if (ptr1->saved_custom_code_status != ptr2->saved_custom_code_status
   |   ~~~^
options-save.cc:9291:38: note: use unary '+' which decays operands to pointers or 
'&'component_ref' not supported by dump_decl[0] != 
&'component_ref' not supported by dump_decl[0]' to compare the 
addresses
options-save.cc:9294:37: error: comparison between two arrays 
[-Werror=array-compare]
  9294 |   if (ptr1->saved_custom_code_index != ptr2->saved_custom_code_index
   |   ~~^~~~
...

This is due to an array-typed TargetSave state in config/nios2/nios2.opt:
...
TargetSave
enum nios2_ccs_code saved_custom_code_status[256]

TargetSave
int saved_custom_code_index[256]
...


This patch adjusts the generated array state compare from 'ptr1->array' into 
'>array[0]' in gcc/optc-save-gen.awk,
seems sufficient to pass the tougher checks.

Tested by ensuring the compiler builds, which should be sufficient here.
Okay to commit to mainline?


Martin added the array support in r219636, any thoughts?

It seems to me that the warning is pointing out that comparing the 
address of the array is nonsensical, and we should remove it and just 
have the memcmp.



Thanks,
Chung-Lin

* optc-save-gen.awk: Adjust array compare to use '>name[0]'
instead of 'ptr->name'.




Re: [PATCH v2] c++: Fix type completeness checks for type traits [PR106838]

2022-09-08 Thread Jason Merrill via Gcc-patches

On 9/8/22 08:56, Jonathan Wakely wrote:

On Wed, 7 Sept 2022 at 16:11, Jason Merrill wrote:


On 9/7/22 08:18, Jonathan Wakely wrote:

@@ -12080,23 +12098,37 @@ finish_trait_expr (location_t loc, cp_trait_kind 
kind, tree type1, tree type2)
   case CPTK_HAS_TRIVIAL_COPY:
   case CPTK_HAS_TRIVIAL_DESTRUCTOR:
   case CPTK_HAS_UNIQUE_OBJ_REPRESENTATIONS:
-case CPTK_HAS_VIRTUAL_DESTRUCTOR:
-case CPTK_IS_ABSTRACT:
+  if (!check_trait_type (type1))
+ return error_mark_node;
+  break;
+
   case CPTK_IS_AGGREGATE:


Hmm, why does std::is_aggregate require a complete array element type?
An array is an aggregate regardless of its element type.  I'd think it
should be kind 4 instead.


That makes sense. It doesn't match the precondition for the library
trait in the standard, but violating that library precondition is
undefined, not ill-formed, so it seems harmless for libstdc++ to also
relax that precondition and allow arrays of incomplete type.




-case CPTK_IS_EMPTY:
-case CPTK_IS_FINAL:
   case CPTK_IS_LITERAL_TYPE:
   case CPTK_IS_POD:
-case CPTK_IS_POLYMORPHIC:
   case CPTK_IS_STD_LAYOUT:
   case CPTK_IS_TRIVIAL:
   case CPTK_IS_TRIVIALLY_COPYABLE:
-  if (!check_trait_type (type1))
+  if (!check_trait_type (type1, /* kind = */ 2))
+ return error_mark_node;
+  break;
+
+case CPTK_IS_EMPTY:


I also wonder why std::is_empty excludes unions, since there can now be
an empty field of union type with the addition of [[no_unique_address]]?


True, although the main purpose of is_empty is to decide whether to
derive from it to get the EBO. With [[no_unique_address]] there's no
reason to do that, you can just add the attribute unconditionally and if
it's not empty, the attribute is a no-op. So I find it hard to care
about this case :-)

If the FE trait requires unions to be complete, then the library trait
would need to special-case unions to not use the FE trait, so that it
can still give the same answer (i.e. false) for incomplete union types.
I don't think changing this one has any advantage.




+case CPTK_IS_POLYMORPHIC:
+case CPTK_IS_ABSTRACT:
+case CPTK_HAS_VIRTUAL_DESTRUCTOR:
+  if (!check_trait_type (type1, /* kind = */ 3))
+ return error_mark_node;
+  break;
+
+case CPTK_IS_FINAL:


And I wonder a bit why std::is_final cares about the completeness of
unions, which can't be (or have) base classes, but I guess you can still
ask about whether the keyword was present, even though it has no effect.


Yes, as currently specified, it tells you whether the specifier was
present, nothing more.

Thanks for the review, here's an updated patch that moves __is_aggregate
to kind 4. I didn't change __is_empty or __is_final though, did you want
those changed?

Tested x86_64-linux.


That's fine, the patch is OK with the typo fix below:


-- >8 --

The check_trait_type function is used for a number of different type
traits that have different requirements on their arguments. For example,
__is_constructible allows arrays of unknown bound even if the array
element is an incomplete type, but __is_aggregate does not, it always
requires the array element type to be complete. Other traits have
different requirements again, e.g. __is_empty allows incomplete unions,
and arrays (of known or unknown bound) of incomplete types.

This alters the check_trait_type function to take an additional KIND
parameter which indicates which set of type trait requirements to check.

As noted in a comment, the requirements for __is_aggregate deviate from
the ones for std::is_aggregate in the standard. It's not necessary for
the elements of an array to be complete types, because arrays are always
aggregates.

The type_has_virtual_destructor change is needed to avoid an ICE.
Previously it could never be called for incomplete union types as they
were (incorrectly) rejected by check_trait_type.

This change causes some additional diagnostics in some libstdc++ tests,
where the front end was not previously complaining about invalid types
that the library assertions diagnosed. We should consider removing the
library assertions from traits where the front end implements the
correct checks now.

PR c++/106838

gcc/cp/ChangeLog:

* class.cc (type_has_virtual_destructor): Return false for
union types.
* semantics.cc (check_trait_type): Add KIND parameter to support
different sets of requirements.
(finish_trait_expr): Pass KIND argument for relevant traits.

gcc/ChangeLog:

* doc/extend.texi (Type Traits): Fix requirements. Document
__is_aggregate and __is_final.

gcc/testsuite/ChangeLog:

* g++.dg/ext/array4.C: Fix invalid use of __is_constructible.
* g++.dg/ext/unary_trait_incomplete.C: Fix tests for traits with
different requirements.

libstdc++-v3/ChangeLog:

* testsuite/20_util/is_complete_or_unbounded/memoization_neg.cc:
Prune additional 

Re: Modula-2: merge followup (brief update on the progress of the new linking implementation)

2022-09-08 Thread Gaius Mulley via Gcc-patches
Martin Liška  writes:

> Note I've just converted the current Modula-2 manual to RST (Sphinx):
> https://splichal.eu/scripts/sphinx/
>
> It contains some minor issues, but in general it should be pretty fine. Note 
> pygments
> contains a corresponding lexer:
> https://pygments.org/docs/lexers/#multi-dialect-lexer-for-modula-2

very nice - well done!  All very useful - and much easier to see bugs in
gm2.texi.  I see a few missing options (missing from gm2.texi) and also
I see problems with the libraries - meta data (const/type/var) is
rendered (@findex perhaps should be ignored or processed).  Or I could
disable it from gcc/m2/tools-src/def2texi.py)?

It is starting to look very nice indeed

regards,
Gaius


Re: [Patch] OpenMP: Document ompx warnings + add Fortran omx warning [PR106670]

2022-09-08 Thread Jakub Jelinek via Gcc-patches
On Mon, Aug 29, 2022 at 11:24:52AM +0200, Tobias Burnus wrote:
>   PR fortran/106670
> 
> gcc/fortran/ChangeLog:
> 
>   * scanner.cc (skip_fixed_omp_sentinel): Add -Wsurprising warning
>   for 'omx' sentinels with -fopenmp.
>   * invoke.texi (-Wsurprising): Document additional warning case.
> 
> libgomp/ChangeLog:
> 
>   * libgomp.texi (OpenMP 5.2): Add comment to ompx/omx entry.
> 
> gcc/testsuite/ChangeLog:
> 
>   * c-c++-common/gomp/ompx-1.c: New test.
>   * c-c++-common/gomp/ompx-2.c: New test.
>   * g++.dg/gomp/ompx-attrs-1.C: New test.
>   * gfortran.dg/gomp/ompx-1.f90: New test.
>   * gfortran.dg/gomp/omx-1.f: New test.
>   * gfortran.dg/gomp/omx-2.f: New test.

> --- a/libgomp/libgomp.texi
> +++ b/libgomp/libgomp.texi
> @@ -359,7 +359,13 @@ to address of matching mapped list item per 5.1, Sect. 
> 2.21.7.2 @tab N @tab
>  @item @code{omp_in_explicit_task} routine and @emph{implicit-task-var} ICV
>@tab N @tab
>  @item @code{omp}/@code{ompx}/@code{omx} sentinels and 
> @code{omp_}/@code{ompx_}
> -  namespaces @tab N/A @tab
> +  namespaces @tab N/A
> +  @tab warning for @code{omp/ompx} sentinels@footnote{@code{omp/ompx}
> +  sentinels as C/C++ pragma and C++ attributes are warned for with
> +  @code{-Wunknown-pragmas} (implied by @code{-Wall}) and 
> @code{-Wattributes}
> +  (by default), respectively; for Fortran free-source code, the there is 
> a

s/by default/enabled by default/
s/the there/there/

> +  warning by default and for fixed-source code with @code{-Wsurprising}

s/by default/enabled by default/

> +  (enabled by @code{-Wall})}
>  @item Clauses on @code{end} directive can be on directive @tab N @tab
>  @item Deprecation of no-argument @code{destroy} clause on @code{depobj}
>@tab N @tab

Otherwise LGTM.

Jakub



Re: [wwwdocs] gcc-13/changes.html + projects/gomp/: OpenMP update

2022-09-08 Thread Jakub Jelinek via Gcc-patches
On Fri, Sep 02, 2022 at 09:37:57AM +0200, Tobias Burnus wrote:
> Update the OpenMP status for features that were added in the last months.
> 
> Comments/suggestions? Okay to commit?
> 
> Tobias
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
> München, HRB 106955

> gcc-13/changes.html + projects/gomp/: OpenMP update
> 
> * htdocs/gcc-13/changes.html: Update OpenMP entry; fix html syntax.
> * htdocs/projects/gomp/index.html: Update OpenMP 5.x implementation status;
>   add missing item from libgomp.texi + flip two items to have same order as
>   the .texi.
> 
>  htdocs/gcc-13/changes.html  | 42 
> -
>  htdocs/projects/gomp/index.html | 40 +--
>  2 files changed, 63 insertions(+), 19 deletions(-)

I think some of the  or  issues have been fixed in the mean time,
so it might not apply cleanly.
LGTM, whether you want to update it further for doacross/omp_cur_iteration
now or whether we do it incrementally up to you.

Jakub



Re: [Patch] libgomp.texi: Document libmemkind + nvptx/gcn specifics

2022-09-08 Thread Jakub Jelinek via Gcc-patches
On Mon, Aug 29, 2022 at 12:54:33PM +0200, Tobias Burnus wrote:
> I had this patch lying around since about half a year. I did tweak and 
> agumented it
> a bit today, but finally want to get rid of it (locally - by getting it 
> committed) ...
> 
> This patch changes -misa to -march for nvptx (the latter is now an alias
> for the former), it adds a new section about libmemkind and some information
> about interns of our nvptx/gcn implementation. (The latter should be mostly
> correct, but I might have missed some fine print or a more recent update.)
> 
> OK for mainline?
> 
> Tobias
> 
> 
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
> München, HRB 106955

> libgomp.texi: Document libmemkind + nvptx/gcn specifics
> 
> libgomp/ChangeLog:
> 
>   * libgomp.texi (OpenMP-Implementation Specifics): New; add libmemkind
>   section; move OpenMP Context Selectors from ...
>   (Offload-Target Specifics): ... here; add 'AMD Radeo (GCN)' and
>   'nvptx' sections.
> +All OpenMP and OpenACC levels are used, i.e.
> +@itemize
> +@item OpenMP's simd and OpenACC's vector map to work items (thread)
> +@item OpenMP's threads (``parallel'') and OpenACC's workers map
> +  to wavefronts
> +@item OpenMP's teams and OpenACC's gang use use a threadpool with the

s/use use/use/

> +All OpenMP and OpenACC levels are used, i.e.
> +@itemize
> +@item OpenMP's simd and OpenACC's vector map to threads
> +@item OpenMP's threads (``parallel'') and OpenACC's workers map to warps
> +@item OpenMP's teams and OpenACC's gang use use a threadpool with the

Again.

Otherwise LGTM.

Jakub



[PATCH] optc-save-gen.awk: adjust generated array compare

2022-09-08 Thread Chung-Lin Tang
Hi Joseph,
Jan-Benedict reported a build-bot error for the nios2 port under 
--enable-werror-always:

options-save.cc: In function 'bool cl_target_option_eq(const cl_target_option*, 
const cl_target_option*)':
options-save.cc:9291:38: error: comparison between two arrays 
[-Werror=array-compare]
 9291 |   if (ptr1->saved_custom_code_status != ptr2->saved_custom_code_status
  |   ~~~^
options-save.cc:9291:38: note: use unary '+' which decays operands to pointers 
or '&'component_ref' not supported by dump_decl[0] != 
&'component_ref' not supported by dump_decl[0]' to compare 
the addresses
options-save.cc:9294:37: error: comparison between two arrays 
[-Werror=array-compare]
 9294 |   if (ptr1->saved_custom_code_index != ptr2->saved_custom_code_index
  |   ~~^~~~
...

This is due to an array-typed TargetSave state in config/nios2/nios2.opt:
...
TargetSave
enum nios2_ccs_code saved_custom_code_status[256]

TargetSave
int saved_custom_code_index[256]
...


This patch adjusts the generated array state compare from 'ptr1->array' into 
'>array[0]' in gcc/optc-save-gen.awk,
seems sufficient to pass the tougher checks.

Tested by ensuring the compiler builds, which should be sufficient here.
Okay to commit to mainline?

Thanks,
Chung-Lin

* optc-save-gen.awk: Adjust array compare to use '>name[0]'
instead of 'ptr->name'.
diff --git a/gcc/optc-save-gen.awk b/gcc/optc-save-gen.awk
index 233d1fbb637..27aabf2955e 100644
--- a/gcc/optc-save-gen.awk
+++ b/gcc/optc-save-gen.awk
@@ -1093,7 +1093,7 @@ for (i = 0; i < n_target_array; i++) {
name = var_target_array[i]
size = var_target_array_size[i]
type = var_target_array_type[i]
-   print "  if (ptr1->" name" != ptr2->" name "";
+   print "  if (>" name"[0] != >" name "[0]";
print "  || memcmp (ptr1->" name ", ptr2->" name ", " size " * 
sizeof(" type ")))"
print "return false;";
 }


Re: [Patch] OpenMP/Fortran: Permit end-clause on directive

2022-09-08 Thread Jakub Jelinek via Gcc-patches
On Thu, Sep 08, 2022 at 05:21:08PM +0200, Jakub Jelinek via Gcc-patches wrote:
> Otherwise LGTM.

Oh, and what Harald wrote, it might be better to split the nowait-4.f90 test
into 2, one for all the valid nowait cases and one for the invalid ones,
such that the first one can be compile tested all the way through assembly.

Jakub



[PATCH, nios2, committed] Add #undef of MUSL_DYNAMIC_LINKER

2022-09-08 Thread Chung-Lin Tang
This patch adds an #undef of MUSL_DYNAMIC_LINKER before its #define in 
config/nios2/linux.h.
This makes the nios2-linux build pass when the compiler is configured with 
--enable-werror-always.

Patch pushed to master at 0697bd070c4fffb33468976c93baff9493922fb3

Chung-LinFrom 0697bd070c4fffb33468976c93baff9493922fb3 Mon Sep 17 00:00:00 2001
From: Chung-Lin Tang 
Date: Thu, 8 Sep 2022 23:14:38 +0800
Subject: [PATCH] nios2: Add #undef of MUSL_DYNAMIC_LINKER

Add #undef of MUSL_DYNAMIC_LINKER before #define, to satisfy build checks
when configured with --enable-werror-always.

gcc/ChangeLog:

* config/nios2/linux.h (MUSL_DYNAMIC_LINKER): Add #undef before #define.
---
 gcc/config/nios2/linux.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gcc/config/nios2/linux.h b/gcc/config/nios2/linux.h
index f5dd813acad..9e53dd657e4 100644
--- a/gcc/config/nios2/linux.h
+++ b/gcc/config/nios2/linux.h
@@ -30,6 +30,8 @@
 #define CPP_SPEC "%{posix:-D_POSIX_SOURCE} %{pthread:-D_REENTRANT}"
 
 #define GLIBC_DYNAMIC_LINKER "/lib/ld-linux-nios2.so.1"
+
+#undef MUSL_DYNAMIC_LINKER
 #define MUSL_DYNAMIC_LINKER  "/lib/ld-musl-nios2.so.1"
 
 #undef LINK_SPEC
-- 
2.17.1



Re: [Patch] OpenMP/Fortran: Permit end-clause on directive

2022-09-08 Thread Jakub Jelinek via Gcc-patches
On Fri, Aug 26, 2022 at 08:21:26PM +0200, Tobias Burnus wrote:
> I did run into some issues related to this; those turned out to be
> unrelated, but I end ended up implementing this feature.
> 
> Side remark: 'omp parallel workshare' seems to actually permit 'nowait'
> now, but I guess that's an unintended change due to the
> syntax-representation change. Hence, it is now tracked as Spec Issue
> 3338 and I do not permit it.
> 
> OK for mainline?
> 
> Tobias
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
> München, HRB 106955

> OpenMP/Fortran: Permit end-clause on directive
> 
> gcc/fortran/ChangeLog:
> 
>   * openmp.cc (OMP_DO_CLAUSES, OMP_SCOPE_CLAUSES,
>   OMP_SECTIONS_CLAUSES, OMP_SINGLE_CLAUSES): Add 'nowait'.

This doesn't describe what the patch actually does, Add 'nowait'.
is only true for the first 3, for OMP_SINGLE_CLAUSES IMHO you
want a separate
(OMP_SINGLE_CLAUSES): Add 'nowait' and 'copyprivate'.
entry.

> @@ -3855,7 +3857,7 @@ cleanup:
> | OMP_CLAUSE_ORDER | OMP_CLAUSE_ALLOCATE)
>  #define OMP_SINGLE_CLAUSES \
>(omp_mask (OMP_CLAUSE_PRIVATE) | OMP_CLAUSE_FIRSTPRIVATE   \
> -   | OMP_CLAUSE_ALLOCATE)
> +   | OMP_CLAUSE_ALLOCATE | OMP_CLAUSE_NOWAIT | OMP_CLAUSE_COPYPRIVATE)
>  #define OMP_ORDERED_CLAUSES \
>(omp_mask (OMP_CLAUSE_THREADS) | OMP_CLAUSE_SIMD)
>  #define OMP_DECLARE_TARGET_CLAUSES \

> @@ -5909,13 +5915,11 @@ gfc_match_omp_teams_distribute_simd (void)
>  match
>  gfc_match_omp_workshare (void)
>  {
> -  if (gfc_match_omp_eos () != MATCH_YES)
> -{
> -  gfc_error ("Unexpected junk after $OMP WORKSHARE statement at %C");
> -  return MATCH_ERROR;
> -}
> +  gfc_omp_clauses *c;
> +  if (gfc_match_omp_clauses (, omp_mask (OMP_CLAUSE_NOWAIT)) != MATCH_YES)
> +return MATCH_ERROR;
>new_st.op = EXEC_OMP_WORKSHARE;
> -  new_st.ext.omp_clauses = gfc_get_omp_clauses ();
> +  new_st.ext.omp_clauses = c;
>return MATCH_YES;
>  }

I think it would be better to introduce OMP_WORKSHARE_CLAUSES and use
it in both gfc_match_omp_workshare and just use
  return match_omp (EXEC_OMP_WORKSHARE, OMP_WORKSHARE_CLAUSES);
?

> @@ -6954,6 +6952,9 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses 
> *omp_clauses,
> }
>   break;
> case OMP_LIST_COPYPRIVATE:
> + if (omp_clauses->nowait)
> +   gfc_error ("NOWAIT clause must not be be used with COPYPRIVATE "

s/be be/be/
> +  "clause at %L", >where);
>   for (; n != NULL; n = n->next)
> {
>   if (n->sym->as && n->sym->as->type == AS_ASSUMED_SIZE)

> @@ -5284,7 +5285,13 @@ parse_omp_do (gfc_statement omp_st)
>if (st == omp_end_st)
>  {
>if (new_st.op == EXEC_OMP_END_NOWAIT)
> - cp->ext.omp_clauses->nowait |= new_st.ext.omp_bool;
> + {
> +   if (cp->ext.omp_clauses->nowait && new_st.ext.omp_bool)
> + gfc_error_now ("Duplicated NOWAIT clause on %s and %s at %C",
> +gfc_ascii_statement (omp_st),
> +gfc_ascii_statement (omp_end_st));
> +   cp->ext.omp_clauses->nowait |= new_st.ext.omp_bool;
> + }
>else
>   gcc_assert (new_st.op == EXEC_NOP);
>gfc_clear_new_st ();

Not sure if the standard is clear enough that unique clauses can't be
repeated on both directive and corresponding end directive.  But let's
assume that is the case.

> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/gomp/copyprivate-2.f90
> @@ -0,0 +1,69 @@
> +  FUNCTION t()
> +INTEGER :: a, b, t
> +a = 0
> +t = b
> +b = 0
> +!$OMP PARALLEL REDUCTION(+:b)
> +  !$OMP SINGLE COPYPRIVATE (b) NOWAIT  ! { dg-error "NOWAIT clause must 
> not be be used with COPYPRIVATE clause" }

Here too (several times).

> +!$OMP ATOMIC WRITE
> +b = 6
> +  !$OMP END SINGLE
> +!$OMP END PARALLEL
> +t = t + b
> +  END FUNCTION
> +
> +  FUNCTION t2()
> +INTEGER :: a, b, t2
> +a = 0
> +t2 = b
> +b = 0
> +!$OMP PARALLEL REDUCTION(+:b)
> +  !$OMP SINGLE NOWAIT COPYPRIVATE (b)  ! { dg-error "NOWAIT clause must 
> not be be used with COPYPRIVATE clause" }
> +!$OMP ATOMIC WRITE
> +b = 6
> +  !$OMP END SINGLE
> +!$OMP END PARALLEL
> +t2 = t2 + b
> +  END FUNCTION
> +
> +  FUNCTION t3()
> +INTEGER :: a, b, t3
> +a = 0
> +t3 = b
> +b = 0
> +!$OMP PARALLEL REDUCTION(+:b)
> +  !$OMP SINGLE COPYPRIVATE (b)  ! { dg-error "NOWAIT clause must not be 
> be used with COPYPRIVATE clause" }
> +!$OMP ATOMIC WRITE
> +b = 6
> +  !$OMP END SINGLE NOWAIT
> +!$OMP END PARALLEL
> +t3 = t3 + b
> +  END FUNCTION
> +
> +  FUNCTION t4()
> +INTEGER :: a, b, t4
> +a = 0
> +t4 = b
> +b = 0
> +!$OMP PARALLEL 

[pushed] vect: Fix scalar stmt typo in vect_optimize_slp_pass [PR106886]

2022-09-08 Thread Richard Sandiford via Gcc-patches
Fix a stupid typo in my vect_optimize_slp_pass patch.

Tested on aarch64-linux-gnu, pushed as obvious.

Richard


gcc/
PR tree-optimization/106886
* tree-vect-slp.cc (vect_optimize_slp_pass::get_result_with_layout):
Fix copying of scalar stmts.

gcc/testsuite/
PR tree-optimization/106886
* gcc.dg/vect/bb-slp-layout-21.c: New test.
---
 gcc/testsuite/gcc.dg/vect/bb-slp-layout-21.c | 23 
 gcc/tree-vect-slp.cc |  2 +-
 2 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/vect/bb-slp-layout-21.c

diff --git a/gcc/testsuite/gcc.dg/vect/bb-slp-layout-21.c 
b/gcc/testsuite/gcc.dg/vect/bb-slp-layout-21.c
new file mode 100644
index 000..c851d58c97a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/bb-slp-layout-21.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=bdver2" { target x86_64-*-* i?86-*-* } } */
+
+int rl2GeomExport64_little_endian, rl2GeomExport64_little_endian_arch;
+void rl2GeomExport64(unsigned char *p, double value) {
+  union {
+unsigned char byte[8];
+double double_value;
+  } convert;
+  convert.double_value = value;
+  if (rl2GeomExport64_little_endian_arch)
+if (rl2GeomExport64_little_endian) {
+  *(p + 7) = convert.byte[0];
+  *(p + 6) = convert.byte[1];
+  *(p + 5) = convert.byte[2];
+  *(p + 4) = convert.byte[3];
+  *(p + 3) = convert.byte[4];
+  *(p + 2) = convert.byte[5];
+  *(p + 1) = convert.byte[6];
+  *p = convert.byte[7];
+} else
+  *p = convert.byte[7];
+}
diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
index 3fa2dc96dda..ca3422c2a1e 100644
--- a/gcc/tree-vect-slp.cc
+++ b/gcc/tree-vect-slp.cc
@@ -5212,7 +5212,7 @@ vect_optimize_slp_pass::get_result_with_layout (slp_tree 
node,
   if (SLP_TREE_SCALAR_STMTS (node).length ())
{
  auto  = SLP_TREE_SCALAR_STMTS (result);
- stmts.safe_splice (SLP_TREE_SCALAR_STMTS (result));
+ stmts.safe_splice (SLP_TREE_SCALAR_STMTS (node));
  if (from_layout_i != 0)
vect_slp_permute (m_perms[from_layout_i], stmts, false);
  if (to_layout_i != 0)
-- 
2.25.1



[committed] c++: Add testcase for already fixed PR [PR99209]

2022-09-08 Thread Patrick Palka via Gcc-patches
This was incidentally fixed by r13-806-g221acd67ca50f8.

PR c++/99209

gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/lambda-uneval17.C: New test.
---
 gcc/testsuite/g++.dg/cpp2a/lambda-uneval17.C | 17 +
 1 file changed, 17 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-uneval17.C

diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-uneval17.C 
b/gcc/testsuite/g++.dg/cpp2a/lambda-uneval17.C
new file mode 100644
index 000..74e0f876371
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/lambda-uneval17.C
@@ -0,0 +1,17 @@
+// PR c++/99209
+// { dg-do compile { target c++20 } }
+
+constexpr char f(...) = delete;
+constexpr decltype(auto) f_adl(auto a) { return f(a); }
+
+namespace A {
+constexpr char f(auto) { return 'A'; }
+template void g(char FunctionParam = 
f_adl([]{})) {
+char Local = f_adl([]{});
+}
+}
+
+namespace B {
+constexpr char f(auto) = delete;
+void call() { A::g(); }
+}
-- 
2.37.3.518.g79f2338b37



[PATCH] Fix some gimple_ctrl_altering_p mistakes

2022-09-08 Thread Richard Biener via Gcc-patches
CFG cleanup resets the control altering flag for noreturn functions
when they are ECF_LEAF (like __builtin_unreachable ()).  The
.ABNORMAL_DISPATCHER call built during CFG construction is not
marked as control altering.  Several passes inserting traps or
unreachables fail to set the flag.  And more.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

PR middle-end/106870
* gimple-harden-conditionals.cc (insert_check_and_trap):
Set the control-altering flag on the built IFN_TRAP.
* gimple.cc (gimple_build_builtin_unreachable): Likewise.
* tree-cfg.cc (handle_abnormal_edges): Set the control-altering
flag on the .ABNORMAL_DISPATCHER call.
* tree-cfgcleanup.cc (cleanup_call_ctrl_altering_flag): Avoid
resetting the control altering flag for ECF_NORETURN calls.
(cleanup_control_flow_bb): Set the control altering flag on
discovered noreturn calls.
* symtab-thunks.cc (expand_thunk): Set the control altering
flag for the noreturn tailcall case.
* tree-eh.cc (lower_resx): Likewisw for trap and unwind_resume
calls.
---
 gcc/gimple-harden-conditionals.cc |  1 +
 gcc/gimple.cc |  1 +
 gcc/symtab-thunks.cc  |  1 +
 gcc/tree-cfg.cc   |  3 ++-
 gcc/tree-cfgcleanup.cc| 11 ---
 gcc/tree-eh.cc|  4 +++-
 6 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/gcc/gimple-harden-conditionals.cc 
b/gcc/gimple-harden-conditionals.cc
index 4ca6776fca7..1b3dd563baa 100644
--- a/gcc/gimple-harden-conditionals.cc
+++ b/gcc/gimple-harden-conditionals.cc
@@ -238,6 +238,7 @@ insert_check_and_trap (location_t loc, gimple_stmt_iterator 
*gsip,
 
   gimple_stmt_iterator gsit = gsi_after_labels (trp);
   gcall *trap = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0);
+  gimple_call_set_ctrl_altering (trap, true);
   gimple_set_location (trap, loc);
   gsi_insert_before (, trap, GSI_SAME_STMT);
 
diff --git a/gcc/gimple.cc b/gcc/gimple.cc
index cd5ad0c718b..4d45311b45c 100644
--- a/gcc/gimple.cc
+++ b/gcc/gimple.cc
@@ -440,6 +440,7 @@ gimple_build_builtin_unreachable (location_t loc)
   gcc_checking_assert (data == NULL_TREE);
   g = gimple_build_call_internal (IFN_TRAP, 0);
 }
+  gimple_call_set_ctrl_altering (g, true);
   gimple_set_location (g, loc);
   return g;
 }
diff --git a/gcc/symtab-thunks.cc b/gcc/symtab-thunks.cc
index b0439702230..bd50c689761 100644
--- a/gcc/symtab-thunks.cc
+++ b/gcc/symtab-thunks.cc
@@ -635,6 +635,7 @@ expand_thunk (cgraph_node *node, bool output_asm_thunks,
}
   else
{
+ gimple_call_set_ctrl_altering (call, true);
  gimple_call_set_tail (call, true);
  cfun->tail_call_marked = true;
  remove_edge (single_succ_edge (bb));
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index 3e2d0adf4a2..a939a11d8bb 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -821,8 +821,9 @@ handle_abnormal_edges (basic_block *dispatcher_bbs, 
basic_block for_bb,
   else
{
  tree arg = inner ? boolean_true_node : boolean_false_node;
- gimple *g = gimple_build_call_internal (IFN_ABNORMAL_DISPATCHER,
+ gcall *g = gimple_build_call_internal (IFN_ABNORMAL_DISPATCHER,
 1, arg);
+ gimple_call_set_ctrl_altering (g, true);
  gimple_stmt_iterator gsi = gsi_after_labels (*dispatcher);
  gsi_insert_after (, g, GSI_NEW_STMT);
 
diff --git a/gcc/tree-cfgcleanup.cc b/gcc/tree-cfgcleanup.cc
index 3535a7e28a4..b4869aee78d 100644
--- a/gcc/tree-cfgcleanup.cc
+++ b/gcc/tree-cfgcleanup.cc
@@ -220,9 +220,10 @@ cleanup_call_ctrl_altering_flag (basic_block bb, gimple 
*bb_end)
 return;
 
   int flags = gimple_call_flags (bb_end);
-  if (((flags & (ECF_CONST | ECF_PURE))
-   && !(flags & ECF_LOOPING_CONST_OR_PURE))
-  || (flags & ECF_LEAF))
+  if (!(flags & ECF_NORETURN)
+  && (((flags & (ECF_CONST | ECF_PURE))
+  && !(flags & ECF_LOOPING_CONST_OR_PURE))
+ || (flags & ECF_LEAF)))
 gimple_call_set_ctrl_altering (bb_end, false);
   else
 {
@@ -328,6 +329,10 @@ cleanup_control_flow_bb (basic_block bb)
gsi_remove (, true);
   if (remove_fallthru_edge (bb->succs))
retval = true;
+  tree lhs = gimple_call_lhs (stmt);
+  if (!lhs
+ || !should_remove_lhs_p (lhs))
+   gimple_call_set_ctrl_altering (stmt, true);
 }
 
   return retval;
diff --git a/gcc/tree-eh.cc b/gcc/tree-eh.cc
index 076ecd3ec9a..ae8fa21d9a3 100644
--- a/gcc/tree-eh.cc
+++ b/gcc/tree-eh.cc
@@ -3321,7 +3321,7 @@ lower_resx (basic_block bb, gresx *stmt,
   int lp_nr;
   eh_region src_r, dst_r;
   gimple_stmt_iterator gsi;
-  gimple *x;
+  gcall *x;
   tree fn, src_nr;
   bool ret = false;
 
@@ -3346,6 +3346,7 @@ lower_resx (basic_block bb, gresx *stmt,
 
   fn = builtin_decl_implicit (BUILT_IN_TRAP);
   x = 

RE: [PATCH] arm: Fix constant immediates predicates and constraints for some MVE builtins

2022-09-08 Thread Kyrylo Tkachov via Gcc-patches


> -Original Message-
> From: Christophe Lyon 
> Sent: Wednesday, September 7, 2022 5:59 PM
> To: Kyrylo Tkachov ; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] arm: Fix constant immediates predicates and
> constraints for some MVE builtins
> 
> 
> 
> On 9/7/22 15:42, Kyrylo Tkachov wrote:
> >
> >
> >> -Original Message-
> >> From: Christophe Lyon 
> >> Sent: Wednesday, September 7, 2022 2:41 PM
> >> To: Kyrylo Tkachov ; gcc-patches@gcc.gnu.org
> >> Subject: Re: [PATCH] arm: Fix constant immediates predicates and
> >> constraints for some MVE builtins
> >>
> >>
> >>
> >> On 9/7/22 15:34, Kyrylo Tkachov wrote:
> >>> Hi Christophe,
> >>>
>  -Original Message-
>  From: Gcc-patches   bounces+kyrylo.tkachov=arm@gcc.gnu.org> On Behalf Of
> Christophe
>  Lyon via Gcc-patches
>  Sent: Wednesday, September 7, 2022 2:03 PM
>  To: gcc-patches@gcc.gnu.org
>  Subject: [PATCH] arm: Fix constant immediates predicates and
> constraints
> >> for
>  some MVE builtins
> 
>  Several MVE builtins incorrectly use the same predicate/constraint
>  pair for several modes, which does not match the specification.
>  This patch uses the appropriate iterator instead.
> 
> >>>
> >>> This looks ok to me.
> >>> I presume you've tested this appropriately?
> >>
> >> I tested it manually with an offending testcase.
> >>
> >> Unfortunately, the existing testcases all use '1' as immediate, so this
> >> does not really check the boundaries. We do plan to improve the existing
> >> tests in a later patch that will more generally improve the MVE tests.
> >
> > Sure, improving the tests is definitely worth it here.
> > I meant more in the context of a standard bootstrap and testsuite run.
> 
> OK, just ran a bootstrap on arm-linux-gnueabihf, no issue, and
> regression tested on a cross arm-eabi with the default
> RUNTESTFLAGS/target-board, no issue either.
> 

Ok, thanks for testing.
Kyrill

> Thanks,
> 
> Christophe
> 
> > Thanks,
> > Kyrill
> >
> >>
> >> Christophe
> >>
> >>> If so, ok for trunk.
> >>> Thanks,
> >>> Kyrill
> >>>
>  2022-09-06  Christophe Lyon  
> 
>   gcc/
>   * config/arm/mve.md (mve_vqshluq_n_s): Use
>   MVE_pred/MVE_constraint instead of mve_imm_7/Ra.
>   (mve_vqshluq_m_n_s): Likewise.
>   (mve_vqrshrnbq_n_): Use
>  MVE_pred3/MVE_constraint3
>   instead of mve_imm_8/Rb.
>   (mve_vqrshrunbq_n_s): Likewise.
>   (mve_vqrshrntq_n_): Likewise.
>   (mve_vqrshruntq_n_s): Likewise.
>   (mve_vrshrnbq_n_): Likewise.
>   (mve_vrshrntq_n_): Likewise.
>   (mve_vqrshrnbq_m_n_): Likewise.
>   (mve_vqrshrntq_m_n_): Likewise.
>   (mve_vrshrnbq_m_n_): Likewise.
>   (mve_vrshrntq_m_n_): Likewise.
>   (mve_vqrshrunbq_m_n_s): Likewise.
>   (mve_vsriq_n_  instead
>   of mve_imm_selective_upto_8/Rg.
>   (mve_vsriq_m_n_): Likewise.
>  ---
> gcc/config/arm/mve.md | 30 +++---
> 1 file changed, 15 insertions(+), 15 deletions(-)
> 
>  diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md
>  index c4dec01baac..714178609f7 100644
>  --- a/gcc/config/arm/mve.md
>  +++ b/gcc/config/arm/mve.md
>  @@ -1624,7 +1624,7 @@ (define_insn "mve_vqshluq_n_s"
>   [
>    (set (match_operand:MVE_2 0 "s_register_operand" "=w")
>   (unspec:MVE_2 [(match_operand:MVE_2 1 "s_register_operand"
>  "w")
>  -   (match_operand:SI 2 "mve_imm_7" "Ra")]
>  +   (match_operand:SI 2 ""
>  "")]
>    VQSHLUQ_N_S))
>   ]
>   "TARGET_HAVE_MVE"
>  @@ -2615,7 +2615,7 @@ (define_insn
> >> "mve_vqrshrnbq_n_"
>    (set (match_operand: 0 "s_register_operand"
> "=w")
>   (unspec: [(match_operand: 1
>  "s_register_operand" "0")
>    (match_operand:MVE_5 2
>  "s_register_operand" "w")
>  - (match_operand:SI 3 "mve_imm_8" "Rb")]
>  + (match_operand:SI 3 ""
>  "")]
>    VQRSHRNBQ_N))
>   ]
>   "TARGET_HAVE_MVE"
>  @@ -2630,7 +2630,7 @@ (define_insn "mve_vqrshrunbq_n_s"
>    (set (match_operand: 0 "s_register_operand"
> "=w")
>   (unspec: [(match_operand: 1
>  "s_register_operand" "0")
>    (match_operand:MVE_5 2
>  "s_register_operand" "w")
>  - (match_operand:SI 3 "mve_imm_8" "Rb")]
>  + (match_operand:SI 3 ""
>  "")]
>    VQRSHRUNBQ_N_S))
>   ]
>   "TARGET_HAVE_MVE"
>  @@ -3570,7 +3570,7 @@ (define_insn "mve_vsriq_n_"
>    (set (match_operand:MVE_2 0 "s_register_operand" "=w")
>   (unspec:MVE_2 [(match_operand:MVE_2 1 "s_register_operand"
>  "0")
>  

Re: [RFC] postreload cse'ing vector constants

2022-09-08 Thread Robin Dapp via Gcc-patches
> Which is this from the mail archives:
> 
> https://gcc.gnu.org/pipermail/gcc-patches/1998-June/000308.html
> 
> I would tend to agree that for equal cost that the constant would be 
> preferred since that should be better from a scheduling/dependency 
> standpoint.   So it seems to me we can drive this purely from a costing 
> standpoint.

I did bootstrapping and ran the testsuite on x86(-64), aarch64, Power9
and s390.  Everything looks good except two additional fails on x86
where code actually looks worse.

gcc.target/i386/keylocker-encodekey128.c

17c17,18
<   movaps  %xmm4, k2(%rip)
---
>   pxor%xmm0, %xmm0
>   movaps  %xmm0, k2(%rip)

gcc.target/i386/keylocker-encodekey256.c:

19c19,20
<   movaps  %xmm4, k3(%rip)
---
>   pxor%xmm0, %xmm0
>   movaps  %xmm0, k3(%rip)

Regards
 Robin


[PATCH v2] c++: Fix type completeness checks for type traits [PR106838]

2022-09-08 Thread Jonathan Wakely via Gcc-patches
On Wed, 7 Sept 2022 at 16:11, Jason Merrill wrote:
>
> On 9/7/22 08:18, Jonathan Wakely wrote:
> > @@ -12080,23 +12098,37 @@ finish_trait_expr (location_t loc, cp_trait_kind 
> > kind, tree type1, tree type2)
> >   case CPTK_HAS_TRIVIAL_COPY:
> >   case CPTK_HAS_TRIVIAL_DESTRUCTOR:
> >   case CPTK_HAS_UNIQUE_OBJ_REPRESENTATIONS:
> > -case CPTK_HAS_VIRTUAL_DESTRUCTOR:
> > -case CPTK_IS_ABSTRACT:
> > +  if (!check_trait_type (type1))
> > + return error_mark_node;
> > +  break;
> > +
> >   case CPTK_IS_AGGREGATE:
>
> Hmm, why does std::is_aggregate require a complete array element type?
> An array is an aggregate regardless of its element type.  I'd think it
> should be kind 4 instead.

That makes sense. It doesn't match the precondition for the library
trait in the standard, but violating that library precondition is
undefined, not ill-formed, so it seems harmless for libstdc++ to also
relax that precondition and allow arrays of incomplete type.

>
> > -case CPTK_IS_EMPTY:
> > -case CPTK_IS_FINAL:
> >   case CPTK_IS_LITERAL_TYPE:
> >   case CPTK_IS_POD:
> > -case CPTK_IS_POLYMORPHIC:
> >   case CPTK_IS_STD_LAYOUT:
> >   case CPTK_IS_TRIVIAL:
> >   case CPTK_IS_TRIVIALLY_COPYABLE:
> > -  if (!check_trait_type (type1))
> > +  if (!check_trait_type (type1, /* kind = */ 2))
> > + return error_mark_node;
> > +  break;
> > +
> > +case CPTK_IS_EMPTY:
>
> I also wonder why std::is_empty excludes unions, since there can now be
> an empty field of union type with the addition of [[no_unique_address]]?

True, although the main purpose of is_empty is to decide whether to
derive from it to get the EBO. With [[no_unique_address]] there's no
reason to do that, you can just add the attribute unconditionally and if
it's not empty, the attribute is a no-op. So I find it hard to care
about this case :-)

If the FE trait requires unions to be complete, then the library trait
would need to special-case unions to not use the FE trait, so that it
can still give the same answer (i.e. false) for incomplete union types.
I don't think changing this one has any advantage.

>
> > +case CPTK_IS_POLYMORPHIC:
> > +case CPTK_IS_ABSTRACT:
> > +case CPTK_HAS_VIRTUAL_DESTRUCTOR:
> > +  if (!check_trait_type (type1, /* kind = */ 3))
> > + return error_mark_node;
> > +  break;
> > +
> > +case CPTK_IS_FINAL:
>
> And I wonder a bit why std::is_final cares about the completeness of
> unions, which can't be (or have) base classes, but I guess you can still
> ask about whether the keyword was present, even though it has no effect.

Yes, as currently specified, it tells you whether the specifier was
present, nothing more.

Thanks for the review, here's an updated patch that moves __is_aggregate
to kind 4. I didn't change __is_empty or __is_final though, did you want
those changed?

Tested x86_64-linux.


-- >8 --

The check_trait_type function is used for a number of different type
traits that have different requirements on their arguments. For example,
__is_constructible allows arrays of unknown bound even if the array
element is an incomplete type, but __is_aggregate does not, it always
requires the array element type to be complete. Other traits have
different requirements again, e.g. __is_empty allows incomplete unions,
and arrays (of known or unknown bound) of incomplete types.

This alters the check_trait_type function to take an additional KIND
parameter which indicates which set of type trait requirements to check.

As noted in a comment, the requirements for __is_aggregate deviate from
the ones for std::is_aggregate in the standard. It's not necessary for
the elements of an array to be complete types, because arrays are always
aggregates.

The type_has_virtual_destructor change is needed to avoid an ICE.
Previously it could never be called for incomplete union types as they
were (incorrectly) rejected by check_trait_type.

This change causes some additional diagnostics in some libstdc++ tests,
where the front end was not previously complaining about invalid types
that the library assertions diagnosed. We should consider removing the
library assertions from traits where the front end implements the
correct checks now.

PR c++/106838

gcc/cp/ChangeLog:

* class.cc (type_has_virtual_destructor): Return false for
union types.
* semantics.cc (check_trait_type): Add KIND parameter to support
different sets of requirements.
(finish_trait_expr): Pass KIND argument for relevant traits.

gcc/ChangeLog:

* doc/extend.texi (Type Traits): Fix requirements. Document
__is_aggregate and __is_final.

gcc/testsuite/ChangeLog:

* g++.dg/ext/array4.C: Fix invalid use of __is_constructible.
* g++.dg/ext/unary_trait_incomplete.C: Fix tests for traits with
different requirements.

libstdc++-v3/ChangeLog:

* 

Re: [PATCH] c++: unnecessary instantiation of constexpr var [PR99130]

2022-09-08 Thread Jason Merrill via Gcc-patches

On 9/7/22 16:40, Patrick Palka wrote:

On Wed, 7 Sep 2022, Jason Merrill wrote:


On 9/7/22 15:41, Patrick Palka wrote:

Here the use of the constexpr member/variable specialization 'value'
from within an unevaluated context causes us to overeagerly instantiate
it, via maybe_instantiate_decl called from mark_used, despite only its
declaration not its definition being needed.


If the issue is with unevaluated context, maybe maybe_instantiate_decl should
guard the call to decl_maybe_constant_var_p with !cp_unevaluated_operand?


Hmm, that seems to work too.  But IIUC this would mean in an evaluated
(but non-constexpr) context we'd continue to instantiate constexpr
variables _immediately_ rather than ideally allowing mark_used to
postpone their instantiation until the end of TU processing (which is
what happens with the below approach).

Another benefit of the below approach is that from within a template
definition we we now avoid instantiation altogether e.g. for

   template constexpr int value = /* blah */;

   template
   int f() { return value; }

we no longer instantiate value which IIUC is consistent with how we
handle other kinds of specializations used within a template definition.
So making mark_used no longer instantiate constexpr variables immediately
(in both evaluated and unevaluated contexts) seems to yield the most
benefits.


Makes sense.  The patch is OK.


We used to have the same issue for constexpr function specializations
until r6-1309-g81371eff9bc7ef made us delay their instantiation until
necessary during constexpr evaluation.

So this patch makes us avoid unnecessarily instantiating constexpr
variable template specializations from mark_used as well.  To that end
this patch pulls out the test in maybe_instantiate_decl

(decl_maybe_constant_var_p (decl)
 || (TREE_CODE (decl) == FUNCTION_DECL
 && DECL_OMP_DECLARE_REDUCTION_P (decl))
 || undeduced_auto_decl (decl))

into each of its three callers (including mark_used) and refines the
test appropriately.  The net result is that only mark_used is changed,
because the other two callers, resolve_address_of_overloaded_function
and decl_constant_var_p, already guard the call appropriately.  And
presumably decl_constant_var_p will take care of instantiation when
needed for e.g. constexpr evaluation.

Bootstrapped and regteste on x86_64-pc-linux-gnu, does this look OK for
trunk?

PR c++/99130

gcc/cp/ChangeLog:

* decl2.cc (maybe_instantiate_decl): Adjust function comment. >>>  
Check VAR_OR_FUNCTION_DECL_P. Pull out the disjunction into ...
(mark_used): ... here, removing the decl_maybe_constant_var_p
part of it.
gcc/testsuite/ChangeLog:

* g++.dg/cpp1y/var-templ70.C: New test.
---
   gcc/cp/decl2.cc  | 33 
   gcc/testsuite/g++.dg/cpp1y/var-templ70.C | 19 ++
   2 files changed, 30 insertions(+), 22 deletions(-)
   create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ70.C

diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 89ab2545d64..cd188813bee 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -5381,24 +5381,15 @@ possibly_inlined_p (tree decl)
 return true;
   }
   -/* Normally, we can wait until instantiation-time to synthesize DECL.
-   However, if DECL is a static data member initialized with a constant
-   or a constexpr function, we need it right now because a reference to
-   such a data member or a call to such function is not value-dependent.
-   For a function that uses auto in the return type, we need to instantiate
-   it to find out its type.  For OpenMP user defined reductions, we need
-   them instantiated for reduction clauses which inline them by hand
-   directly.  */
+/* If DECL is a function or variable template specialization, instantiate
+   its definition now.  */
 void
   maybe_instantiate_decl (tree decl)
   {
-  if (DECL_LANG_SPECIFIC (decl)
+  if (VAR_OR_FUNCTION_DECL_P (decl)
+  && DECL_LANG_SPECIFIC (decl)
 && DECL_TEMPLATE_INFO (decl)
-  && (decl_maybe_constant_var_p (decl)
- || (TREE_CODE (decl) == FUNCTION_DECL
- && DECL_OMP_DECLARE_REDUCTION_P (decl))
- || undeduced_auto_decl (decl))
 && !DECL_DECLARED_CONCEPT_P (decl)
 && !uses_template_parms (DECL_TI_ARGS (decl)))
   {
@@ -5700,15 +5691,13 @@ mark_used (tree decl, tsubst_flags_t complain)
 return false;
   }
   -  /* Normally, we can wait until instantiation-time to synthesize DECL.
- However, if DECL is a static data member initialized with a constant
- or a constexpr function, we need it right now because a reference to
- such a data member or a call to such function is not value-dependent.
- For a function that uses auto in the return type, we need to
instantiate
- it to find out its type.  For OpenMP user defined reductions, we need
- them instantiated for reduction clauses which inline them by hand
- directly.  

Re: [PATCH] Teach vectorizer to deal with bitfield accesses (was: [RFC] Teach vectorizer to deal with bitfield reads)

2022-09-08 Thread Richard Biener via Gcc-patches
On Thu, 25 Aug 2022, Andre Vieira (lists) wrote:

> 
> On 17/08/2022 13:49, Richard Biener wrote:
> > Yes, of course.  What you need to do is subtract DECL_FIELD_BIT_OFFSET
> > of the representative from DECL_FIELD_BIT_OFFSET of the original bitfield
> > access - that's the offset within the representative (by construction
> > both fields share DECL_FIELD_OFFSET).
> Doh! That makes sense...
> >> So instead I change bitpos such that:
> >> align_of_representative = TYPE_ALIGN (TREE_TYPE (representative));
> >> bitpos -= bitpos.to_constant () / align_of_representative *
> >> align_of_representative;
> > ?  Not sure why alignment comes into play here?
> Yeah just forget about this... it was my ill attempt at basically doing what
> you described above.
> > Not sure what you are saying but "yes", all shifting and masking should
> > happen in the type of the representative.
> >
> > +  tree bitpos_tree = build_int_cst (bitsizetype, bitpos);
> >
> > for your convenience there's bitsize_int (bitpos) you can use.
> >
> > I don't think you are using the correct bitpos though, you fail to
> > adjust it for the BIT_FIELD_REF/BIT_INSERT_EXPR.
> Not sure I understand what you mean? I do adjust it, I've changed it now so it
> should hopefully be clearer.
> >
> > +build_int_cst (bitsizetype, TYPE_PRECISION
> > (bf_type)),
> >
> > the size of the bitfield reference is DECL_SIZE of the original
> > FIELD_DECL - it might be bigger than the precision of its type.
> > You probably want to double-check it's equal to the precision
> > (because of the insert but also because of all the masking) and
> > refuse to lower if not.
> I added a check for this but out of curiosity, how can the DECL_SIZE of a
> bitfield FIELD_DECL be different than it's type's precision?

It's probably not possible to create a C testcase but I don't see
what makes this impossible in general to have padding in a bitfield 
object.

> >
> > +/* Return TRUE if there are bitfields to lower in this LOOP.  Fill
> > TO_LOWER
> > +   with data structures representing these bitfields.  */
> > +
> > +static bool
> > +bitfields_to_lower_p (class loop *loop,
> > + vec  _to_lower,
> > + vec  _to_lower)
> > +{
> > +  basic_block *bbs = get_loop_body (loop);
> > +  gimple_stmt_iterator gsi;
> >
> > as said I'd prefer to do this walk as part of the other walks we
> > already do - if and if only because get_loop_body () is a DFS
> > walk over the loop body (you should at least share that).
> I'm now sharing the use of ifc_bbs. The reason why I'd rather not share the
> walk over them is because it becomes quite complex to split out the decision
> to not lower if's because there are none, for which we will still want to
> lower bitfields, versus not lowering if's when they are there but aren't
> lowerable at which point we will forego lowering bitfields since we will not
> vectorize this loop anyway.
> >
> > +  value = fold_build1 (NOP_EXPR, load_type, value);
> >
> > fold_convert (load_type, value)
> >
> > +  if (!CONSTANT_CLASS_P (value))
> > +   {
> > + pattern_stmt
> > +   = gimple_build_assign (vect_recog_temp_ssa_var (load_type,
> > NULL),
> > +  value);
> > + value = gimple_get_lhs (pattern_stmt);
> >
> > there's in principle
> >
> >   gimple_seq stmts = NULL;
> >   value = gimple_convert (, load_type, value);
> >   if (!gimple_seq_empty_p (stmts))
> > {
> >   pattern_stmt = gimple_seq_first_stmt (stmts);
> >   append_pattern_def_seq (vinfo, stmt_info, pattern_stmt);
> > }
> >
> > though a append_pattern_def_seq helper to add a convenience sequence
> > would be nice to have here.
> Ended up using the existing 'vect_convert_input', seems to do nicely here.
> > You probably want to double-check your lowering code by
> > bootstrapping / testing with -ftree-loop-if-convert.
> Done, this lead me to find a new failure mode, where the type of the first
> operand of BIT_FIELD_REF was a FP type (TF mode), which then lead to failures
> when constructing the masking and shifting. I ended up adding a nop-conversion
> to an INTEGER type of the same width first if necessary.

You want a VIEW_CONVERT (aka bit-cast) here.

> Also did a follow-up
> bootstrap with the addition of `-ftree-vectorize` and `-fno-vect-cost-model`
> to further test the codegen. All seems to be working on aarch64-linux-gnu.

+static tree
+get_bitfield_rep (gassign *stmt, bool write, tree *bitpos,
+ tree *struct_expr)
...
+  /* Bail out if the DECL_SIZE of the field_decl isn't the same as the 
BF's
+ precision.  */
+  unsigned HOST_WIDE_INT decl_size = tree_to_uhwi (DECL_SIZE 
(field_decl));
+  if (TYPE_PRECISION (TREE_TYPE (gimple_assign_lhs (stmt))) != decl_size)
+return NULL_TREE;

you can

use compare_tree_int (DECL_SIZE (field_decl), TYPE_PRECISION (...)) != 0

which avoids caring for the case the size isn't a 

[committed] openmp: Implement doacross(sink: omp_cur_iteration - 1)

2022-09-08 Thread Jakub Jelinek via Gcc-patches
Hi!

This patch implements doacross(sink: omp_cur_iteration - 1) that the
previous patchset emitted a sorry on during omp expansion.
It can be implemented with existing library functions.

To recap, depend(source)/doacross(source:)/doacross(source:omp_cur_iteration)
is implemented calling GOMP_doacross_post or GOMP_doacross_ull_post,
called with an array of long or unsigned long long elements, one for
all collapsed loops together and one for each further ordered loop if any.
We initialize that array in each thread when grabbing further set of iterations
and update it at the end of loops, so that it represents the current iteration
(as 0 based counters).  When the worksharing loop is created, we tell the
library through another similar array the counts (the loop needs to be
rectangular) in each dimension, first element is count of all logical iterations
in the collapsed loops.

depend(sink:v1 op N1, v2 op N2, ...) is then implemented by conditionally 
calling
GOMP_doacross_wait/GOMP_doacross_ull_wait.  For N? of 0 there is no check,
otherwise if it wants to wait in a particular dimension for a previous 
iteration,
we check that the corresponding iterator isn't the first one (or first few),
where the previous iterator in that dimension would be out of range, and 
similarly
for checking of next iteration in a dimension that it isn't the last one (or 
last few)
where it would be similarly out of bounds.  Then the collapsed loop counters are
folded into a single 0 based counter (first argument) and then other 0 based
iterations counters on what iteration it should wait for.

Now, doacross(sink: omp_cur_iteration - 1) is supposed to wait for the previous
logical iteration in the combined iteration space of all ordered loops.
For the very first iteration in that combined iteration space it does nothing,
there is no previous iteration.  And similarly it does nothing if there
are more ordered loops than collapsed loop and it isn't the first logical
iteration of the combined loops inside of the collapsed loops, because as 
implemented
we know the previous iteration in that case is always executed by the same 
thread
as the current one.
In the implementation, we use the same value as is stored in the first element
of the array for GOMP_doacross_post/GOMP_doacross_ull_post, if that value is 0,
we do nothing.  The rest is different based on if ordered argument is equal to
collapse or not.  If it is, then we otherwise call
GOMP_doacross_wait/GOMP_doacross_ull_wait with a single argument, one less than
that counter we compare against 0.
If ordered argument is bigger than collapse, we add a per-thread boolean 
variable
.first.N, which we set to true at the start of the outermost ordered loop inside
of the collapsed set of loops and set to false at the end of the innermost
ordered loop.  If .first.N is false, we don't do anything (we know the previous
iteration was handled by the current thread and by my reading of the spec we 
don't
need to emit even a memory barrier in that case, because it is just 
synchronization
with the same thread), otherwise we call 
GOMP_doacross_wait/GOMP_doacross_ull_wait
with the first argument one less than the counter we compare against 0, and then
one less than 2nd and following counts if iterations we pass to the workshare
initialization.  If say .counts.N passed to the workshare initialization is
{ 256, 13, 5, 2 } for collapse(3) ordered(6) loop, then
GOMP_doacross_post/GOMP_doacross_ull_post is called with arguments equal to
.ordereda.N[0] - 1, 12, 4, 1.

Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.

For Tobias:
The new libgomp.c/ testcases are modified copies of existing testcases,
doacross-4.c is a copy of doacross-2.c with just using the OpenMP 5.2 syntax
(i.e. doacross clauses instead of depend (including the corresponding slight
syntax differences)), while doacross-{5,6,7}.c are copies of
doacross-{1,2,3}.c which use the new syntax and use
doacross(sink:omp_cur_iteration - 1) as much as possible (but not more than
once in each loop) and corresponding adjustments on the checking.
And, doacross-7.c is actually the same as doacross-6.c with differences in
schedule clauses only.

2022-09-08  Jakub Jelinek  

gcc/
* omp-expand.cc (expand_omp_ordered_sink): Add CONT_BB argument.
Add doacross(sink:omp_cur_iteration-1) support.
(expand_omp_ordered_source_sink): Clear counts[fd->ordered + 1].
Adjust expand_omp_ordered_sink caller.
(expand_omp_for_ordered_loops): If counts[fd->ordered + 1] is
non-NULL, set that variable to true at the start of outermost
non-collapsed loop and set it to false at the end of innermost
ordered loop.
(expand_omp_for_generic): If fd->ordered, allocate
1 + (fd->ordered - fd->collapse) further elements in counts array.
Copy to counts + 2 + fd->ordered the counts of fd->collapse ..
fd->ordered - 1 loop if any.
gcc/testsuite/
* 

[PATCH] tree-optimization/106881 - constrain uninit control edge add

2022-09-08 Thread Richard Biener via Gcc-patches
The following avoids adding fallthru edges to the control chain from
the post-dominator walk.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

PR tree-optimization/106881
* gimple-predicate-analysis.cc (compute_control_dep_chain_pdom):
Add only non-fallthru edges and avoid the same set of edges
as the caller does.

* gcc.dg/uninit-pr106881.c: New testcase.
---
 gcc/gimple-predicate-analysis.cc   |  9 ++---
 gcc/testsuite/gcc.dg/uninit-pr106881.c | 16 
 2 files changed, 22 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/uninit-pr106881.c

diff --git a/gcc/gimple-predicate-analysis.cc b/gcc/gimple-predicate-analysis.cc
index b03b42e029e..8b7f49903c3 100644
--- a/gcc/gimple-predicate-analysis.cc
+++ b/gcc/gimple-predicate-analysis.cc
@@ -1065,9 +1065,12 @@ compute_control_dep_chain_pdom (basic_block cd_bb, 
const_basic_block dep_bb,
 gcc.dg/unninit-pred-12.c and PR106754.  */
   if (single_pred_p (cd_bb))
{
- edge e2 = find_edge (prev_cd_bb, cd_bb);
- gcc_assert (e2);
- cur_cd_chain.safe_push (e2);
+ edge e2 = single_pred_edge (cd_bb);
+ gcc_assert (e2->src == prev_cd_bb);
+ /* But avoid adding fallthru or abnormal edges.  */
+ if (!(e2->flags & (EDGE_FAKE | EDGE_ABNORMAL | EDGE_DFS_BACK))
+ && !single_succ_p (prev_cd_bb))
+   cur_cd_chain.safe_push (e2);
}
 }
   return found_cd_chain;
diff --git a/gcc/testsuite/gcc.dg/uninit-pr106881.c 
b/gcc/testsuite/gcc.dg/uninit-pr106881.c
new file mode 100644
index 000..343b13ed73d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/uninit-pr106881.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fexceptions -Wuninitialized" } */
+
+void l_free (void *);
+char *l_settings_get_string ();
+void eap_append_secret ();
+inline void auto_free(void *a) {
+  void **p = a;
+  l_free(*p); /* { dg-warning "uninitialized" } */
+}
+void eap_gtc_check_settings() {
+  char *identity __attribute__((cleanup(auto_free)));
+  char password __attribute__((cleanup(auto_free)));
+  identity = l_settings_get_string();
+  eap_append_secret();
+}
-- 
2.35.3


[PATCH] mingw32: Fix warning, update documentation

2022-09-08 Thread Jan-Benedict Glaw
Hi!


The mingw32 port is the only port to have TARGET_OVERRIDES_FORMAT_ATTRIBUTES
defined. When this macro is defined, it will never evaluate to NULL, so this
check just leads to a warning:

/usr/lib/gcc-snapshot/bin/g++ -fcf-protection -fno-PIE -c  -DIN_GCC_FRONTEND 
-DIN_GCC_FRONTEND -DIN_GCC_FRONTEND -g -O2   -DIN_GCC  
-DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic 
-Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common 
 -DHAVE_CONFIG_H -I. -Ic-family -I../../gcc/gcc -I../../gcc/gcc/c-family 
-I../../gcc/gcc/../include -I../../gcc/gcc/../libcpp/include 
-I../../gcc/gcc/../libcody  -I../../gcc/gcc/../libdecnumber 
-I../../gcc/gcc/../libdecnumber/bid -I../libdecnumber 
-I../../gcc/gcc/../libbacktrace   -o c-family/c-format.o -MT 
c-family/c-format.o -MMD -MP -MF c-family/.deps/c-format.TPo 
../../gcc/gcc/c-family/c-format.cc
../../gcc/gcc/c-family/c-format.cc: In function 'const char* 
convert_format_name_to_system_name(const char*)':
../../gcc/gcc/c-family/c-format.cc:5114:42: error: the address of 
'mingw_format_attribute_overrides' will never be NULL [-Werror=address]
 5114 |   if (TARGET_OVERRIDES_FORMAT_ATTRIBUTES != NULL
In file included from ./tm.h:26,
 from ../../gcc/gcc/c-family/c-format.cc:23:
../../gcc/gcc/config/i386/mingw32.h:268:44: note: 
'mingw_format_attribute_overrides' declared here
  268 | #define TARGET_OVERRIDES_FORMAT_ATTRIBUTES 
mingw_format_attribute_overrides
  |
^~~~
../../gcc/gcc/c-family/c-format.cc:5079:30: note: in expansion of macro 
'TARGET_OVERRIDES_FORMAT_ATTRIBUTES'
 5079 | extern const target_ovr_attr TARGET_OVERRIDES_FORMAT_ATTRIBUTES[];
  |  ^~
cc1plus: all warnings being treated as errors
make[1]: *** [Makefile:1146: c-family/c-format.o] Error 1
make[1]: Leaving directory 
'/var/lib/laminar/run/gcc-x86_64-w64-mingw32/1/toolchain-build/gcc'
make: *** [Makefile:4588: all-gcc] Error 2



  Also, when TARGET_OVERRIDES_FORMAT_ATTRIBUTES is defined,
TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT must be defined as well. Add
that requirement to the docs.


2022-09-07  Jan-Benedict Glaw  

gcc/ChangeLog:
* c-family/c-format.cc (convert_format_name_to_system_name): Fix 
warning.
* doc/tm.texi.in (TARGET_OVERRIDES_FORMAT_ATTRIBUTES): Document 
requirement
of TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT being defined as well.
* doc/tm.texi: Regenerate.

diff --git a/gcc/c-family/c-format.cc b/gcc/c-family/c-format.cc
index 68b94da40cc..a6c380bf1c8 100644
--- a/gcc/c-family/c-format.cc
+++ b/gcc/c-family/c-format.cc
@@ -5111,8 +5111,7 @@ convert_format_name_to_system_name (const char *attr_name)
 
 #ifdef TARGET_OVERRIDES_FORMAT_ATTRIBUTES
   /* Check if format attribute is overridden by target.  */
-  if (TARGET_OVERRIDES_FORMAT_ATTRIBUTES != NULL
-  && TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT > 0)
+  if (TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT > 0)
 {
   for (i = 0; i < TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT; ++i)
 {
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 5312059ea79..21b849ea32a 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -7836,7 +7836,8 @@ If defined, this macro is the number of entries in
 If defined, this macro is the name of a global variable containing
 target-specific format overrides for the @option{-Wformat} option. The
 default is to have no target-specific format overrides. If defined,
-@code{TARGET_FORMAT_TYPES} must be defined, too.
+@code{TARGET_FORMAT_TYPES} and @code{TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT}
+must be defined, too.
 @end defmac
 
 @defmac TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index e47bf28089a..858bfb80cec 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -12043,7 +12043,8 @@ If defined, this macro is the number of entries in
 If defined, this macro is the name of a global variable containing
 target-specific format overrides for the @option{-Wformat} option. The
 default is to have no target-specific format overrides. If defined,
-@code{TARGET_FORMAT_TYPES} must be defined, too.
+@code{TARGET_FORMAT_TYPES} and @code{TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT}
+must be defined, too.
 @end defmac
 
 @defmac TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT


Okay for HEAD?

Thanks,
  Jan-Benedict

-- 


signature.asc
Description: PGP signature


[PATCH] testsuite/106872 - fix uninit predicate dump scan

2022-09-08 Thread Richard Biener via Gcc-patches
On ppc we see a doloop temp rather than ivtmp.

Tested on x86_64-unknown-linux-gnu and ppc64le, pushed.

PR testsuite/106872
* gcc.dg/uninit-pred-12.c: Adjust.
---
 gcc/testsuite/gcc.dg/uninit-pred-12.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/uninit-pred-12.c 
b/gcc/testsuite/gcc.dg/uninit-pred-12.c
index ebf0288af1f..4c66486fb3a 100644
--- a/gcc/testsuite/gcc.dg/uninit-pred-12.c
+++ b/gcc/testsuite/gcc.dg/uninit-pred-12.c
@@ -31,4 +31,4 @@ unsigned foo (unsigned v, int y, int w)
 }
 
 /* Make sure predicate analysis picked up the loop exit condition.  */
-/* { dg-final { scan-tree-dump "AND \\(NOT \\(ivtmp" "uninit1" } } */
+/* { dg-final { scan-tree-dump "AND \\(NOT \\((ivtmp|doloop)" "uninit1" } } */
-- 
2.35.3


Re: [PATCH] Implement known/maybe fpclassify like API for frange.

2022-09-08 Thread Aldy Hernandez via Gcc-patches
On Thu, Sep 8, 2022 at 9:27 AM Richard Biener
 wrote:
>
>
>
> > Am 08.09.2022 um 08:28 schrieb Aldy Hernandez :
> >
> > This is what I have in mind for the fpclassify-like methods on the
> > current implementation.  I'll get to the removal of the tristates after
> > Cauldron.
> >
> > As you mentioned, isnormal is kinda tricky, and likely to be confusing
> > for the user.  We can revisit it if it's important.
>
> Yeah.
>
> > ?? I assume maybe_inf() does not return true for NAN ??
>
> Only maybe_nan and known_nan return true for NaN.
>
> >
> > Also, what are your thoughts on signbit?  Perhaps a method returning
> > true if we're sure, and the actual signbit result as a reference?
> >
> >bool known_signbit (const frange , int );
>
> That works for me.  You could look at

ok.  settled on:
+  bool known_signbit (bool ) const;

It even cleaned up the __builtin_signbit folding kludge because now we
can just bail on NANs from known_signbit until they're properly
handled.

> Tree_expr_nonnegative_p to see if that’s good enough to be used there.  Not 
> sure if -0. is negative or if only x for which x < 0. counts as such..  But 
> yes, access to the sign bit is useful.

We're definitely keeping track of signed zeros.  That was the whole
point of the sign bit.  I'll be removing it also, as it's a tristate.

tree_expr_nonnegative_p could definitely be made to make use of
it...especially because we have global ranges.  I'll look into it.

>
> > How does this look?  I must say, the uses look much cleaner.
>
> Yes, I like it.

Attached is the patch I'm pushing.

Tested on x86-64 Linux.

Thanks.
Aldy

>
> Richard
>
> > Aldy
> >
> > gcc/ChangeLog:
> >
> >* gimple-range-fold.cc
> >(fold_using_range::range_of_builtin_int_call): Use fpclassify like API.
> >* range-op-float.cc (finite_operand_p): Same.
> >(finite_operands_p): Same.
> >(foperator_lt::fold_range): Same.
> >(foperator_le::fold_range): Same.
> >(foperator_gt::fold_range): Same.
> >(foperator_ge::fold_range): Same.
> >(foperator_unordered::fold_range): Same.
> >(foperator_unordered::op1_range): Same.
> >(foperator_ordered::fold_range): Same.
> >* value-range.cc (frange::set_nan): Same.
> >(frange::set_signbit): Same.
> >(frange::union_): Same.
> >(frange::intersect): Same.
> >(frange::operator==): Same.
> >(frange::singleton_p): Same.
> >(frange::verify_range): Same.
> >(range_tests_nan): Same.
> >(range_tests_floats): Same.
> >* value-range.h(frange::known_finite): New.
> >(frange::maybe_inf): New.
> >(frange::known_inf): New.
> >(frange::maybe_nan): New.
> >(frange::known_nan): New.
> > ---
> > gcc/gimple-range-fold.cc |  2 +-
> > gcc/range-op-float.cc| 26 ---
> > gcc/value-range.cc   | 37 ++-
> > gcc/value-range.h| 54 +++-
> > 4 files changed, 83 insertions(+), 36 deletions(-)
> >
> > diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
> > index c9c7a2ccc70..47a1f49eb36 100644
> > --- a/gcc/gimple-range-fold.cc
> > +++ b/gcc/gimple-range-fold.cc
> > @@ -1031,7 +1031,7 @@ fold_using_range::range_of_builtin_int_call (irange 
> > , gcall *call,
> >  {
> >if (tmp.get_signbit ().varying_p ()
> >// FIXME: We don't support signed NANs yet.
> > -|| !tmp.get_nan ().no_p ())
> > +|| tmp.maybe_nan ())
> >  return false;
> >if (tmp.get_signbit ().yes_p ())
> >  r.set_nonzero (type);
> > diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
> > index 5fbbaa1fb36..0f928b6c098 100644
> > --- a/gcc/range-op-float.cc
> > +++ b/gcc/range-op-float.cc
> > @@ -167,7 +167,7 @@ frange_set_nan (frange , tree type)
> > static inline bool
> > finite_operand_p (const frange )
> > {
> > -  return flag_finite_math_only || op1.get_nan ().no_p ();
> > +  return flag_finite_math_only || !op1.maybe_nan ();
> > }
> >
> > // Return TRUE if OP1 and OP2 are known to be free of NANs.
> > @@ -175,9 +175,7 @@ finite_operand_p (const frange )
> > static inline bool
> > finite_operands_p (const frange , const frange )
> > {
> > -  return (flag_finite_math_only
> > -  || (op1.get_nan ().no_p ()
> > -  && op2.get_nan ().no_p ()));
> > +  return flag_finite_math_only || (!op1.maybe_nan () && !op2.maybe_nan ());
> > }
> >
> > // Floating version of relop_early_resolve that takes into account NAN
> > @@ -546,7 +544,7 @@ foperator_lt::fold_range (irange , tree type,
> >   else
> >r = range_true_and_false (type);
> > }
> > -  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
> > +  else if (op1.known_nan () || op2.known_nan ())
> > r = range_false (type);
> >   else
> > r = range_true_and_false (type);
> > @@ -648,7 +646,7 @@ foperator_le::fold_range (irange , tree type,
> >   else
> >r = range_true_and_false (type);
> > }
> > -  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p 

[committed] d: Include tm.h in all D target platform sources, remove memmodel.h

2022-09-08 Thread Iain Buclaw via Gcc-patches
Hi,

This patch re-adds tm.h to all D-specific target platform sources,
previously removed by an earlier change that fixed up tm_d.h generation.

The tm.h header would pull in config/elfos.h, which defines
TARGET_D_MINFO_SECTION needed for the D module support in the front-end
to emit data to the correct section for the run-time library to pick up.

The removal of it in r13-2385 caused a stage2 bootstrap failure on all
Solaris targets, and others would have been affected as well.

The memmodel header has also been removed as it is no longer required
now tm_p.h is no longer used by these sources.

Bootstrapped and regression tested on x86_64-linux-gnu and
x86_64-pc-solaris2.11, committed to mainline.

Regards,
Iain

---
gcc/ChangeLog:

* config/darwin-d.cc: Include tm.h.
* config/dragonfly-d.cc: Likewise.
* config/freebsd-d.cc: Remove memmodel.h.
* config/glibc-d.cc: Likewise.
* config/netbsd-d.cc: Include tm.h.
* config/openbsd-d.cc: Likewise.
* config/sol2-d.cc: Likewise.
---
 gcc/config/darwin-d.cc| 1 +
 gcc/config/dragonfly-d.cc | 1 +
 gcc/config/freebsd-d.cc   | 1 -
 gcc/config/glibc-d.cc | 1 -
 gcc/config/netbsd-d.cc| 1 +
 gcc/config/openbsd-d.cc   | 1 +
 gcc/config/sol2-d.cc  | 1 +
 7 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/gcc/config/darwin-d.cc b/gcc/config/darwin-d.cc
index e983883dba6..2ceebc49851 100644
--- a/gcc/config/darwin-d.cc
+++ b/gcc/config/darwin-d.cc
@@ -18,6 +18,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "tm.h"
 #include "tm_d.h"
 #include "d/d-target.h"
 #include "d/d-target-def.h"
diff --git a/gcc/config/dragonfly-d.cc b/gcc/config/dragonfly-d.cc
index d431638f7da..881c5e60b9a 100644
--- a/gcc/config/dragonfly-d.cc
+++ b/gcc/config/dragonfly-d.cc
@@ -18,6 +18,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "tm.h"
 #include "tm_d.h"
 #include "d/d-target.h"
 #include "d/d-target-def.h"
diff --git a/gcc/config/freebsd-d.cc b/gcc/config/freebsd-d.cc
index 189e4a69e78..c795ca2978c 100644
--- a/gcc/config/freebsd-d.cc
+++ b/gcc/config/freebsd-d.cc
@@ -18,7 +18,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
-#include "memmodel.h"
 #include "tm.h"
 #include "tm_d.h"
 #include "d/d-target.h"
diff --git a/gcc/config/glibc-d.cc b/gcc/config/glibc-d.cc
index 80ef27d19c6..1411f1973e5 100644
--- a/gcc/config/glibc-d.cc
+++ b/gcc/config/glibc-d.cc
@@ -19,7 +19,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "system.h"
 #include "coretypes.h"
 #include "tm.h"
-#include "memmodel.h"
 #include "tm_d.h"
 #include "d/d-target.h"
 #include "d/d-target-def.h"
diff --git a/gcc/config/netbsd-d.cc b/gcc/config/netbsd-d.cc
index cd0c95568a1..dbabae7ab71 100644
--- a/gcc/config/netbsd-d.cc
+++ b/gcc/config/netbsd-d.cc
@@ -20,6 +20,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "tm.h"
 #include "tm_d.h"
 #include "d/d-target.h"
 #include "d/d-target-def.h"
diff --git a/gcc/config/openbsd-d.cc b/gcc/config/openbsd-d.cc
index 33c7e41ab62..bb3a3f28f6d 100644
--- a/gcc/config/openbsd-d.cc
+++ b/gcc/config/openbsd-d.cc
@@ -20,6 +20,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "tm.h"
 #include "tm_d.h"
 #include "d/d-target.h"
 #include "d/d-target-def.h"
diff --git a/gcc/config/sol2-d.cc b/gcc/config/sol2-d.cc
index 0ace79d5aae..cecb49cc826 100644
--- a/gcc/config/sol2-d.cc
+++ b/gcc/config/sol2-d.cc
@@ -18,6 +18,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "tm.h"
 #include "tm_d.h"
 #include "d/d-target.h"
 #include "d/d-target-def.h"
-- 
2.34.1



Re: [PATCH] Teach vectorizer to deal with bitfield accesses (was: [RFC] Teach vectorizer to deal with bitfield reads)

2022-09-08 Thread Andre Vieira (lists) via Gcc-patches

Ping.

On 25/08/2022 10:09, Andre Vieira (lists) via Gcc-patches wrote:


On 17/08/2022 13:49, Richard Biener wrote:

Yes, of course.  What you need to do is subtract DECL_FIELD_BIT_OFFSET
of the representative from DECL_FIELD_BIT_OFFSET of the original 
bitfield

access - that's the offset within the representative (by construction
both fields share DECL_FIELD_OFFSET).

Doh! That makes sense...

So instead I change bitpos such that:
align_of_representative = TYPE_ALIGN (TREE_TYPE (representative));
bitpos -= bitpos.to_constant () / align_of_representative *
align_of_representative;

?  Not sure why alignment comes into play here?
Yeah just forget about this... it was my ill attempt at basically 
doing what you described above.

Not sure what you are saying but "yes", all shifting and masking should
happen in the type of the representative.

+  tree bitpos_tree = build_int_cst (bitsizetype, bitpos);

for your convenience there's bitsize_int (bitpos) you can use.

I don't think you are using the correct bitpos though, you fail to
adjust it for the BIT_FIELD_REF/BIT_INSERT_EXPR.
Not sure I understand what you mean? I do adjust it, I've changed it 
now so it should hopefully be clearer.


+    build_int_cst (bitsizetype, TYPE_PRECISION
(bf_type)),

the size of the bitfield reference is DECL_SIZE of the original
FIELD_DECL - it might be bigger than the precision of its type.
You probably want to double-check it's equal to the precision
(because of the insert but also because of all the masking) and
refuse to lower if not.
I added a check for this but out of curiosity, how can the DECL_SIZE 
of a bitfield FIELD_DECL be different than it's type's precision?


+/* Return TRUE if there are bitfields to lower in this LOOP. Fill
TO_LOWER
+   with data structures representing these bitfields.  */
+
+static bool
+bitfields_to_lower_p (class loop *loop,
+ vec  _to_lower,
+ vec  _to_lower)
+{
+  basic_block *bbs = get_loop_body (loop);
+  gimple_stmt_iterator gsi;

as said I'd prefer to do this walk as part of the other walks we
already do - if and if only because get_loop_body () is a DFS
walk over the loop body (you should at least share that).
I'm now sharing the use of ifc_bbs. The reason why I'd rather not 
share the walk over them is because it becomes quite complex to split 
out the decision to not lower if's because there are none, for which 
we will still want to lower bitfields, versus not lowering if's when 
they are there but aren't lowerable at which point we will forego 
lowering bitfields since we will not vectorize this loop anyway.


+  value = fold_build1 (NOP_EXPR, load_type, value);

fold_convert (load_type, value)

+  if (!CONSTANT_CLASS_P (value))
+   {
+ pattern_stmt
+   = gimple_build_assign (vect_recog_temp_ssa_var (load_type,
NULL),
+  value);
+ value = gimple_get_lhs (pattern_stmt);

there's in principle

  gimple_seq stmts = NULL;
  value = gimple_convert (, load_type, value);
  if (!gimple_seq_empty_p (stmts))
    {
  pattern_stmt = gimple_seq_first_stmt (stmts);
  append_pattern_def_seq (vinfo, stmt_info, pattern_stmt);
    }

though a append_pattern_def_seq helper to add a convenience sequence
would be nice to have here.
Ended up using the existing 'vect_convert_input', seems to do nicely 
here.

You probably want to double-check your lowering code by
bootstrapping / testing with -ftree-loop-if-convert.
Done, this lead me to find a new failure mode, where the type of the 
first operand of BIT_FIELD_REF was a FP type (TF mode), which then 
lead to failures when constructing the masking and shifting. I ended 
up adding a nop-conversion to an INTEGER type of the same width first 
if necessary. Also did a follow-up bootstrap with the addition of 
`-ftree-vectorize` and `-fno-vect-cost-model` to further test the 
codegen. All seems to be working on aarch64-linux-gnu.


Re: [PATCH] Use mallinfo2 with glibc >= 2.33

2022-09-08 Thread Jonathan Wakely via Gcc-patches
On Thu, 8 Sept 2022 at 06:03, François Dumont  wrote:
>
>  libstdc++: glibc mallinfo deprecated, use mallinfo2 when version =>
> 2.33
>
>  glibc mallinfo is now deprecated resulting in make check-performance
>  failure. When glibc => 2.33 prefer mallinfo2.
>
>  libstdcxx-v3/ChangeLog:
>
>  * testsuite/util/testsuite_performance.h
> (__gnu_test::MallocInfo): New.
>  (__gnu_test::malloc_info): New, replace mallinfo on current
> platform
>  supporting it and use mallinfo2 when glibc >= 2.33.
>
> Tested under Linux x86_64.
>
> Ok to commit ?

Yes, looks good, thanks!


>
> François
>
> On 07/09/22 19:10, Jonathan Wakely wrote:
> > On Wed, 7 Sept 2022 at 18:03, François Dumont via Libstdc++
> >  wrote:
> >> libstdc++: Use glibc >= 2.33 mallinfo2 function
> >>
> >> mallinfo started to be deprecated which makes performance tests failed
> >> to build, just
> >> adopt mallinfo2.
> >>
> >> libstdcxx-v3/ChangeLog:
> >>
> >>   * testsuite/util/testsuite_performance.h (__mallinfo): New, our
> >> own mallinfo
> > There's no reason to use a reserved name here, this isn't a header
> > that users include.
> >
> > I would call the struct MallocInfo and the function malloc_info().
> > Even better, put them both in namespace __gnu_test, as
> > __gnu_test::MallocInfo and __gnu_test::malloc_info (without the extern
> > "C" language linkage). If we're not calling the glibc function
> > directly, but via our own wrapper, then there's no reason it has to
> > use the name "mallinfo", no reason it has to be in the global
> > namespace, and no reason it has to be extern "C" (in fact, I don't
> > think there was ever a reason for it to be extern "C").
> >
> >
> >
> >>   struct with just what we need. When using glibc >= 2.33 use
> >> mallinfo2 to
> >>   populate it.
> >>
> >> Tested under Linux x86_64,
> >>
> >> Ok to commit ?
> >>
> >> François
>



Re: [PATCH] pch: Fix the reconstruction of adhoc data hash table

2022-09-08 Thread Richard Biener via Gcc-patches



> Am 08.09.2022 um 00:05 schrieb Lewis Hyatt via Gcc-patches 
> :
> 
> The function rebuild_location_adhoc_htab() was meant to reconstruct the
> adhoc location hash map after restoring a line_maps instance from a
> PCH. However, the function has never performed as intended because it
> missed the last step of adding the data into the newly reconstructed hash
> map. This patch fixes that.
> 
> It does not seem possible to construct a test case such that the current
> incorrect behavior is observable as a compiler issue. It would be
> observable, if it were possible for a precompiled header to contain an
> adhoc location with a non-zero custom data pointer. But currently, such
> data pointers are used only by the middle end to track inlining
> information, and this happens later, too late to show up in a PCH.
> 
> I also noted that location_adhoc_data_update, which updates the hash map
> pointers in a different scenario, was relying on undefined pointer
> arithmetic behavior. I'm not aware of this having caused any issue in
> practice, but in this patch I have also changed it to use defined pointer
> operations instead.


Ok.

Thanks,
Richard 

> libcpp/ChangeLog:
> 
>* line-map.cc (location_adhoc_data_update): Remove reliance on
>undefined behavior.
>(get_combined_adhoc_loc): Likewise.
>(rebuild_location_adhoc_htab): Fix issue where the htab was not
>properly updated.
> ---
> 
> Notes:
>Hello-
> 
>While working on something unrelated in line-map.cc, I noticed that the
>function rebuild_location_adhoc_htab(), whose job is to reconstruct the
>adhoc data hash table after a line_maps instance is reconstructed from PCH,
>doesn't actually rebuild the hash table at all:
> 
>void
>rebuild_location_adhoc_htab (line_maps *set)
>{
>  unsigned i;
>  set->location_adhoc_data_map.htab =
>  htab_create (100, location_adhoc_data_hash, location_adhoc_data_eq, 
> NULL);
>  for (i = 0; i < set->location_adhoc_data_map.curr_loc; i++)
>htab_find_slot (set->location_adhoc_data_map.htab,
>set->location_adhoc_data_map.data + i, INSERT);
> ^^
>}
> 
>In order to have the intended effect, it needs to set the return value of
>htab_find_slot to be set->location_adhoc_data_map.data + i, otherwise it
>doesn't effectively do anything except make the hash table think it has
>curr_loc elements set, when in fact it has 0. Subsequent calls to
>htab_traverse, for instance, will do nothing, and any lookups will also 
> fail.
> 
>I tried for some time to construct a test case that would demonstrate an
>observable consequence of this issue, but I don't think it's possible... 
> The
>nontrivial uses of this hash map are in the middle end (e.g. to produce the
>trace of where a given expression was inlined from), and all this happens
>after PCH was read in, and doesn't require any state to be read from the
>PCH. It would become apparent in the future, however, if the ability to
>attach arbitrary data to an adhoc location were used in other ways, perhaps
>somewhere in libcpp; in that hypothetical case, the data would be lost when
>reading back in the PCH.
> 
>There is another kinda related function, location_adhoc_data_update, which
>updates all the pointers in the hash map whenever they are invalidated. It
>seems to me, that this function invokes undefined behavior, since it adds 
> an
>arbitrary offset to the pointers, which do not necessarily point into the
>same array after they were realloced. I don't think it's led to any 
> problems
>in practice but in this patch I also changed that to use well-defined
>operations. Note sure how people may feel about that, since it does require
>on the surface 2x as many operations with this change, but I can't see how
>the current approach is guaranteed to be valid on all architectures?
> 
>Bootstrap + regtest looks good on x86-64 Linux. Thanks a lot to whoever may
>have time to take a look at it.
> 
>-Lewis
> 
> libcpp/line-map.cc | 41 +++--
> 1 file changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/libcpp/line-map.cc b/libcpp/line-map.cc
> index 62077c3857c..391f1d4bbc1 100644
> --- a/libcpp/line-map.cc
> +++ b/libcpp/line-map.cc
> @@ -85,27 +85,38 @@ location_adhoc_data_eq (const void *l1, const void *l2)
>  && lb1->data == lb2->data);
> }
> 
> -/* Update the hashtable when location_adhoc_data is reallocated.  */
> +/* Update the hashtable when location_adhoc_data_map::data is reallocated.
> +   The param is an array of two pointers, the previous value of the data
> +   pointer, and then the new value.  The pointers stored in the hash map
> +   are then rebased to be relative to the new data pointer instead of the
> +   old one.  */
> 
> static int
> -location_adhoc_data_update (void **slot, void *data)
> 

Re: [PATCH] Handle OPAQUE_TYPE specially in verify_type [PR106833]

2022-09-08 Thread Richard Biener via Gcc-patches



> Am 08.09.2022 um 07:53 schrieb Kewen.Lin :
> 
> Hi,
> 
> As PR106833 shows, cv-qualified opaque type can cause ICE
> during LTO.  It exposes that we missd to handle OPAQUE_TYPE
> well in type verification.  As Richi pointed out, also
> assuming that target will always define TYPE_MAIN_VARIANT
> and TYPE_CANONICAL for opaque type, this patch is to check
> both are OPAQUE_TYPE_P.  Besides, it also checks the only
> available size and alignment information as well as type
> mode for TYPE_MAIN_VARIANT.
> 
> Bootstrapped and regtested on powerpc64-linux-gnu P7 and
> powerpc64le-linux-gnu P9 and P10.
> 
> Is it ok for trunk?
> 
> BR,
> Kewen
> -
>PR middle-end/106833
> 
> gcc/ChangeLog:
> 
>* tree.cc (verify_opaque_type): New function.
>(verify_match): New macro.
>(verify_type): Call verify_opaque_type for OPAQUE_TYPE.
> 
> gcc/testsuite/ChangeLog:
> 
>* gcc.target/powerpc/pr106833.c: New test.
> ---
> gcc/testsuite/gcc.target/powerpc/pr106833.c | 14 +++
> gcc/tree.cc | 45 -
> 2 files changed, 58 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr106833.c
> 
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr106833.c 
> b/gcc/testsuite/gcc.target/powerpc/pr106833.c
> new file mode 100644
> index 000..968d75184ff
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr106833.c
> @@ -0,0 +1,14 @@
> +/* { dg-do link } */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-require-effective-target lto } */
> +/* { dg-options "-flto -mdejagnu-cpu=power10" } */
> +
> +/* Verify there is no ICE in LTO mode.  */
> +
> +int main ()
> +{
> +  float *b;
> +  const __vector_quad c;
> +  __builtin_mma_disassemble_acc (b, );
> +  return 0;
> +}
> diff --git a/gcc/tree.cc b/gcc/tree.cc
> index fed1434d141..e67caa8f85d 100644
> --- a/gcc/tree.cc
> +++ b/gcc/tree.cc
> @@ -13670,6 +13670,42 @@ gimple_canonical_types_compatible_p (const_tree t1, 
> const_tree t2,
> }
> }
> 
> +/* For OPAQUE_TYPE T, it has only size and alignment information, so verify
> +   these properties of T match TV which is the main variant of T.  Also
> +   verify the type of TC, which is the canonical of T, is OPAQUE_TYPE.  */
> +
> +static void
> +verify_opaque_type (const_tree t, tree tv, tree tc)
> +{
> +  gcc_assert (OPAQUE_TYPE_P (t));
> +  gcc_assert (tv && tv == TYPE_MAIN_VARIANT (tv));
> +  gcc_assert (tc && tc == TYPE_CANONICAL (tc));
> +
> +#define verify_match(flag, t, tv)
>   \
> +  do 
>   \
> +{
>   \
> +  if (flag (t) != flag (tv)) 
>   \
> +{  \
> +  error ("opaque type differs by %s", #flag);  \
> +  debug_tree (tv); \
> +}  \
> +}
>   \
> +  while (false)
> +
> +  if (t != tv)
> +{
> +  verify_match (TREE_CODE, t, tv);
> +  verify_match (TYPE_MODE, t, tv);
> +  verify_match (TYPE_SIZE, t, tv);

TYPE_SIZE is a tree, you should probably 
Compare this with operand_equal_p.  It’s 
Not documented to be a constant size?
Thus some VLA vector mode might be allowed ( a poly_int size), BLKmode
Is ruled out(?), the docs say we have
‚An MODE_Opaque‘ here but I don’t see
This being verified?

The macro makes this a bit unworldly
For the only benefit of elaborate diagnostic
Which I think isn’t really necessary

> +  verify_match (TYPE_ALIGN, t, tv);
> +  verify_match (TYPE_USER_ALIGN, t, tv);
> +}
> +
> +  if (t != tc)
> +verify_match (TREE_CODE, t, tc);
> +#undef verify_match
> +}
> +
> /* Verify type T.  */
> 
> void
> @@ -13677,6 +13713,14 @@ verify_type (const_tree t)
> {
>   bool error_found = false;
>   tree mv = TYPE_MAIN_VARIANT (t);
> +  tree ct = TYPE_CANONICAL (t);
> +
> +  if (OPAQUE_TYPE_P (t))
> +{
> +  verify_opaque_type (t, mv, ct);
> +  return;
> +}
> +
>   if (!mv)
> {
>   error ("main variant is not defined");
> @@ -13691,7 +13735,6 @@ verify_type (const_tree t)
>   else if (t != mv && !verify_type_variant (t, mv))
> error_found = true;
> 
> -  tree ct = TYPE_CANONICAL (t);
>   if (!ct)
> ;
>   else if (TYPE_CANONICAL (ct) != ct)
> --
> 2.27.0
> 


Re: [PATCH] Implement known/maybe fpclassify like API for frange.

2022-09-08 Thread Richard Biener via Gcc-patches



> Am 08.09.2022 um 08:28 schrieb Aldy Hernandez :
> 
> This is what I have in mind for the fpclassify-like methods on the
> current implementation.  I'll get to the removal of the tristates after
> Cauldron.
> 
> As you mentioned, isnormal is kinda tricky, and likely to be confusing
> for the user.  We can revisit it if it's important.

Yeah.

> ?? I assume maybe_inf() does not return true for NAN ??

Only maybe_nan and known_nan return true for NaN.

> 
> Also, what are your thoughts on signbit?  Perhaps a method returning
> true if we're sure, and the actual signbit result as a reference?
> 
>bool known_signbit (const frange , int );

That works for me.  You could look at
Tree_expr_nonnegative_p to see if that’s good enough to be used there.  Not 
sure if -0. is negative or if only x for which x < 0. counts as such..  But 
yes, access to the sign bit is useful.

> How does this look?  I must say, the uses look much cleaner.

Yes, I like it.

Richard 

> Aldy
> 
> gcc/ChangeLog:
> 
>* gimple-range-fold.cc
>(fold_using_range::range_of_builtin_int_call): Use fpclassify like API.
>* range-op-float.cc (finite_operand_p): Same.
>(finite_operands_p): Same.
>(foperator_lt::fold_range): Same.
>(foperator_le::fold_range): Same.
>(foperator_gt::fold_range): Same.
>(foperator_ge::fold_range): Same.
>(foperator_unordered::fold_range): Same.
>(foperator_unordered::op1_range): Same.
>(foperator_ordered::fold_range): Same.
>* value-range.cc (frange::set_nan): Same.
>(frange::set_signbit): Same.
>(frange::union_): Same.
>(frange::intersect): Same.
>(frange::operator==): Same.
>(frange::singleton_p): Same.
>(frange::verify_range): Same.
>(range_tests_nan): Same.
>(range_tests_floats): Same.
>* value-range.h(frange::known_finite): New.
>(frange::maybe_inf): New.
>(frange::known_inf): New.
>(frange::maybe_nan): New.
>(frange::known_nan): New.
> ---
> gcc/gimple-range-fold.cc |  2 +-
> gcc/range-op-float.cc| 26 ---
> gcc/value-range.cc   | 37 ++-
> gcc/value-range.h| 54 +++-
> 4 files changed, 83 insertions(+), 36 deletions(-)
> 
> diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
> index c9c7a2ccc70..47a1f49eb36 100644
> --- a/gcc/gimple-range-fold.cc
> +++ b/gcc/gimple-range-fold.cc
> @@ -1031,7 +1031,7 @@ fold_using_range::range_of_builtin_int_call (irange , 
> gcall *call,
>  {
>if (tmp.get_signbit ().varying_p ()
>// FIXME: We don't support signed NANs yet.
> -|| !tmp.get_nan ().no_p ())
> +|| tmp.maybe_nan ())
>  return false;
>if (tmp.get_signbit ().yes_p ())
>  r.set_nonzero (type);
> diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
> index 5fbbaa1fb36..0f928b6c098 100644
> --- a/gcc/range-op-float.cc
> +++ b/gcc/range-op-float.cc
> @@ -167,7 +167,7 @@ frange_set_nan (frange , tree type)
> static inline bool
> finite_operand_p (const frange )
> {
> -  return flag_finite_math_only || op1.get_nan ().no_p ();
> +  return flag_finite_math_only || !op1.maybe_nan ();
> }
> 
> // Return TRUE if OP1 and OP2 are known to be free of NANs.
> @@ -175,9 +175,7 @@ finite_operand_p (const frange )
> static inline bool
> finite_operands_p (const frange , const frange )
> {
> -  return (flag_finite_math_only
> -  || (op1.get_nan ().no_p ()
> -  && op2.get_nan ().no_p ()));
> +  return flag_finite_math_only || (!op1.maybe_nan () && !op2.maybe_nan ());
> }
> 
> // Floating version of relop_early_resolve that takes into account NAN
> @@ -546,7 +544,7 @@ foperator_lt::fold_range (irange , tree type,
>   else
>r = range_true_and_false (type);
> }
> -  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
> +  else if (op1.known_nan () || op2.known_nan ())
> r = range_false (type);
>   else
> r = range_true_and_false (type);
> @@ -648,7 +646,7 @@ foperator_le::fold_range (irange , tree type,
>   else
>r = range_true_and_false (type);
> }
> -  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
> +  else if (op1.known_nan () || op2.known_nan ())
> r = range_false (type);
>   else
> r = range_true_and_false (type);
> @@ -742,7 +740,7 @@ foperator_gt::fold_range (irange , tree type,
>   else
>r = range_true_and_false (type);
> }
> -  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
> +  else if (op1.known_nan () || op2.known_nan ())
> r = range_false (type);
>   else
> r = range_true_and_false (type);
> @@ -844,7 +842,7 @@ foperator_ge::fold_range (irange , tree type,
>   else
>r = range_true_and_false (type);
> }
> -  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
> +  else if (op1.known_nan () || op2.known_nan ())
> r = range_false (type);
>   else
> r = range_true_and_false (type);
> @@ -927,10 +925,10 @@ 

[PATCH] Implement known/maybe fpclassify like API for frange.

2022-09-08 Thread Aldy Hernandez via Gcc-patches
This is what I have in mind for the fpclassify-like methods on the
current implementation.  I'll get to the removal of the tristates after
Cauldron.

As you mentioned, isnormal is kinda tricky, and likely to be confusing
for the user.  We can revisit it if it's important.

?? I assume maybe_inf() does not return true for NAN ??

Also, what are your thoughts on signbit?  Perhaps a method returning
true if we're sure, and the actual signbit result as a reference?

bool known_signbit (const frange , int );

How does this look?  I must say, the uses look much cleaner.
Aldy

gcc/ChangeLog:

* gimple-range-fold.cc
(fold_using_range::range_of_builtin_int_call): Use fpclassify like API.
* range-op-float.cc (finite_operand_p): Same.
(finite_operands_p): Same.
(foperator_lt::fold_range): Same.
(foperator_le::fold_range): Same.
(foperator_gt::fold_range): Same.
(foperator_ge::fold_range): Same.
(foperator_unordered::fold_range): Same.
(foperator_unordered::op1_range): Same.
(foperator_ordered::fold_range): Same.
* value-range.cc (frange::set_nan): Same.
(frange::set_signbit): Same.
(frange::union_): Same.
(frange::intersect): Same.
(frange::operator==): Same.
(frange::singleton_p): Same.
(frange::verify_range): Same.
(range_tests_nan): Same.
(range_tests_floats): Same.
* value-range.h(frange::known_finite): New.
(frange::maybe_inf): New.
(frange::known_inf): New.
(frange::maybe_nan): New.
(frange::known_nan): New.
---
 gcc/gimple-range-fold.cc |  2 +-
 gcc/range-op-float.cc| 26 ---
 gcc/value-range.cc   | 37 ++-
 gcc/value-range.h| 54 +++-
 4 files changed, 83 insertions(+), 36 deletions(-)

diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index c9c7a2ccc70..47a1f49eb36 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -1031,7 +1031,7 @@ fold_using_range::range_of_builtin_int_call (irange , 
gcall *call,
  {
if (tmp.get_signbit ().varying_p ()
// FIXME: We don't support signed NANs yet.
-   || !tmp.get_nan ().no_p ())
+   || tmp.maybe_nan ())
  return false;
if (tmp.get_signbit ().yes_p ())
  r.set_nonzero (type);
diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index 5fbbaa1fb36..0f928b6c098 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -167,7 +167,7 @@ frange_set_nan (frange , tree type)
 static inline bool
 finite_operand_p (const frange )
 {
-  return flag_finite_math_only || op1.get_nan ().no_p ();
+  return flag_finite_math_only || !op1.maybe_nan ();
 }
 
 // Return TRUE if OP1 and OP2 are known to be free of NANs.
@@ -175,9 +175,7 @@ finite_operand_p (const frange )
 static inline bool
 finite_operands_p (const frange , const frange )
 {
-  return (flag_finite_math_only
- || (op1.get_nan ().no_p ()
- && op2.get_nan ().no_p ()));
+  return flag_finite_math_only || (!op1.maybe_nan () && !op2.maybe_nan ());
 }
 
 // Floating version of relop_early_resolve that takes into account NAN
@@ -546,7 +544,7 @@ foperator_lt::fold_range (irange , tree type,
   else
r = range_true_and_false (type);
 }
-  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
+  else if (op1.known_nan () || op2.known_nan ())
 r = range_false (type);
   else
 r = range_true_and_false (type);
@@ -648,7 +646,7 @@ foperator_le::fold_range (irange , tree type,
   else
r = range_true_and_false (type);
 }
-  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
+  else if (op1.known_nan () || op2.known_nan ())
 r = range_false (type);
   else
 r = range_true_and_false (type);
@@ -742,7 +740,7 @@ foperator_gt::fold_range (irange , tree type,
   else
r = range_true_and_false (type);
 }
-  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
+  else if (op1.known_nan () || op2.known_nan ())
 r = range_false (type);
   else
 r = range_true_and_false (type);
@@ -844,7 +842,7 @@ foperator_ge::fold_range (irange , tree type,
   else
r = range_true_and_false (type);
 }
-  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
+  else if (op1.known_nan () || op2.known_nan ())
 r = range_false (type);
   else
 r = range_true_and_false (type);
@@ -927,10 +925,10 @@ foperator_unordered::fold_range (irange , tree type,
 relation_kind) const
 {
   // UNORDERED is TRUE if either operand is a NAN.
-  if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
+  if (op1.known_nan () || op2.known_nan ())
 r = range_true (type);
   // UNORDERED is FALSE if neither operand is a NAN.
-  else if (op1.get_nan ().no_p () && op2.get_nan ().no_p