Re: Adding a new thread model to GCC
On 1 October 2022 20:34:45 CEST, LIU Hao via Gcc-patches wrote: >Greetings. >The first patch is necessary because somewhere in libgfortran, `pthread_t` is >referenced. If the thread model is not `posix`, it fails to compile. One of several shortcomings mentioned already on Sun, 02 Sep 2018 15:40:28 -0700 in https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg196212.html
Re: [RFA] Avoid unnecessary load-immediate in coremark
On 9/30/22 04:47, Richard Sandiford wrote:
Jeff Law writes:
This is another minor improvement to coremark. I suspect this only
improves code size as the load-immediate was likely issuing with the ret
statement on multi-issue machines.
Basically we're failing to utilize conditional equivalences during the
post-reload CSE pass. So if a particular block is only reached when a
certain condition holds (say for example a4 == 0) and the block has an
assignment like a4 = 0, we would fail to eliminate the unnecessary
assignment.
I wasn't sure (and was too lazy to try, sorry), but: is the reason
that we fail to catch this earlier because the two uses of r4 are
entirely separate (i.e. not from the same pseudo)?
Right. Different pseudos used in the comparison vs the destination of
the assignment. If they used the same pseudo, then I would have
expected cse or gcse to pick it up.
+ /* Iterate over each incoming edge and see if they
+all have the same implicit set. */
+ FOR_EACH_EDGE (e, ei, bb->preds)
+ {
+ /* If the predecessor does not end in a conditional
+jump, then it does not have an implicit set. */
+ if (e->src != ENTRY_BLOCK_PTR_FOR_FN (cfun)
+ && !block_ends_with_condjump_p (e->src))
+ {
+ found = false;
+ break;
+ }
+
+ /* We know the predecessor ends with a conditional
+jump. Now dig into the actal form of the jump
+to potentially extract an implicit set. */
Very minor, but it looked odd to fall through for the entry block.
How about:
if (e->src == ENTRY_BLOCK_PTR_FOR_FN (cfun)
|| !block_ends_with_condjump_p (e->src))
?
Looks like a mistake to me. we don't want to process anything for a
successor of the entry block. I'll adjust and retest.
Thanks,
Jeff
Re: [RFA] Avoid unnecessary load-immediate in coremark
On 9/29/22 01:44, Richard Biener wrote: On Tue, Sep 27, 2022 at 9:54 PM Jeff Law wrote: This is another minor improvement to coremark. I suspect this only improves code size as the load-immediate was likely issuing with the ret statement on multi-issue machines. Basically we're failing to utilize conditional equivalences during the post-reload CSE pass. So if a particular block is only reached when a certain condition holds (say for example a4 == 0) and the block has an assignment like a4 = 0, we would fail to eliminate the unnecessary assignment. conditional equivalences on RTL - ick ;) That was my first reaction as well. I'm not familiar with RTL pattern matching so somebody else has to comment on that, but + /* If this is not the first time through, then +verify the source and destination match. */ + else if (dest == XEXP (cond, 0) && src == XEXP (cond, 1)) + ; shouldn't you restrict dest/src somehow? It might be a MEM? The way you create the fake insn suggests only REG_P dest are OK (not SUBREGs for example?)? You're absolutely right, as is Richard S WRT unexpected sharing. I'll adjust the patch appropriately. Should you use rtx_equal_p (not using that possibly exempts MEM, but being more explicit would be nice). Should you restrict this to MODE_INT compares? rtx_equal_p would be better, yes. I'll adjust that too. This should work regardless of hte mode type though. The key is the post-reload cse bits have to check that the pattern matches and that the constraints are satisfied when a replacement is made. My only concern would be MODE_CC stuff. I'll think a bit more about that case. Jeff
Re: [committed] More gimple const/copy propagation opportunities
On Fri, 30 Sep 2022 17:32:34 -0600
Jeff Law wrote:
> + /* This looks good from a CFG standpoint. Now look at the guts
> + of PRED. Basically we want to verify there are no PHI nodes
> + and no real statements. */
> + if (! gimple_seq_empty_p (phi_nodes (pred)))
> +return false;
So, given the below, neither DEBUG nor labels do count towards an
empty seq [coming in from any PHI that is, otherwise it's a different
thing], which is a bit surprising but well, ok. It looks at PHI IL, so
probably yes. Allegedly that's what it is. Neat if that's true.
> +
> + gimple_stmt_iterator gsi;
> + for (gsi = gsi_last_bb (pred); !gsi_end_p (gsi); gsi_prev ())
> +{
> + gimple *stmt = gsi_stmt (gsi);
> +
> + switch (gimple_code (stmt))
> + {
> + case GIMPLE_LABEL:
> + if (DECL_NONLOCAL (gimple_label_label (as_a (stmt
> + return false;
> + break;
> +
> + case GIMPLE_DEBUG:
> + break;
> +
> + default:
> + return false;
don't like, sounds odd. Are we sure there's no other garbage that can
manifest here? int meow=42;, and meow unused won't survive?, pragmas
neither or stuff ?
> + }
> +}
> +
> + return true;
> +}
> +
> /* We have finished optimizing BB, record any information implied by
> taking a specific outgoing edge from BB. */
>
> @@ -583,6 +656,62 @@ record_edge_info (basic_block bb)
>if (can_infer_simple_equiv && TREE_CODE (inverted) == EQ_EXPR)
> edge_info->record_simple_equiv (op0, op1);
> }
> +
> + /* If this block is a single block loop, then we may be able to
> + record some equivalences on the loop's exit edge. */
> + if (single_block_loop_p (bb))
> + {
> + /* We know it's a single block loop. Now look at the loop
> + exit condition. What we're looking for is whether or not
> + the exit condition is loop invariant which we can detect
> + by checking if all the SSA_NAMEs referenced are defined
> + outside the loop. */
> + if ((TREE_CODE (op0) != SSA_NAME
> +|| gimple_bb (SSA_NAME_DEF_STMT (op0)) != bb)
> + && (TREE_CODE (op1) != SSA_NAME
> + || gimple_bb (SSA_NAME_DEF_STMT (op1)) != bb))
> + {
> + /* At this point we know the exit condition is loop
> + invariant. The only way to get out of the loop is
> + if never traverses the backedge to begin with. This
s/if /if it /
> + implies that any PHI nodes create equivalances we can
that any threw me off asking for "that if any". Would have been nicer,
i think?
> + attach to the loop exit edge. */
attach it to
> + int alternative
bool
> + = (EDGE_PRED (bb, 0)->flags & EDGE_DFS_BACK) ? 1 : 0;
> +
> + gphi_iterator gsi;
> + for (gsi = gsi_start_phis (bb);
> +!gsi_end_p (gsi);
> +gsi_next ())
> + {
> + /* If the other alternative is the same as the result,
> + then this is a degenerate and can be ignored. */
> + if (dst == PHI_ARG_DEF (phi, !alternative))
> + continue;
> +
> + /* Now get the EDGE_INFO class so we can append
> + it to our list. We want the successor edge
> + where the destination is not the source of
> + an incoming edge. */
> + gphi *phi = gsi.phi ();
> + tree src = PHI_ARG_DEF (phi, alternative);
> + tree dst = PHI_RESULT (phi);
> +
> + if (EDGE_SUCC (bb, 0)->dest
> + != EDGE_PRED (bb, !alternative)->src)
by now, alternative would be easier to grok if it would have been spelled
from_backedge_p or something. IMHO.
thanks,
> + edge_info = (class edge_info *)EDGE_SUCC (bb, 0)->aux;
> + else
> + edge_info = (class edge_info *)EDGE_SUCC (bb, 1)->aux;
> +
> + /* Note that since this processing is done independently
> + of other edge equivalency processing, we may not
> + have an EDGE_INFO structure set up yet. */
> + if (edge_info == NULL)
> + edge_info = new class edge_info (false_edge);
> + edge_info->record_simple_equiv (dst, src);
> + }
> + }
> + }
> }
> }
> }
[PATCH] libstdc++: Use ///< for inline documentation
I accidentally that some variables were misdocumented when using
trailing comment for documentation. I ran a search with a relatively
simple regex[1] to look for any ///s following some code that did not
have a <, and came up with these instances only.
[1]: \s*([^ ]+\s*)+///[^<].*$
libstdc++-v3/ChangeLog:
* include/std/iostream: Use ///< for inline documentation.
* include/std/limits: Likewise.
* include/experimental/internet: Likewise.
Signed-off-by: Arsen Arsenović
---
Hey,
I just got reminded that I found some trivial documentation errors a few months
ago, and forgot to do anything about them after bringing them up on IRC. This
patch should fix that.
Thanks,
libstdc++-v3/include/experimental/internet | 2 +-
libstdc++-v3/include/std/iostream | 16
libstdc++-v3/include/std/limits| 10 +-
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/libstdc++-v3/include/experimental/internet
b/libstdc++-v3/include/experimental/internet
index 4be4bfb731e..a6b7b235087 100644
--- a/libstdc++-v3/include/experimental/internet
+++ b/libstdc++-v3/include/experimental/internet
@@ -2137,7 +2137,7 @@ namespace ip
using resolver = basic_resolver; ///< A TCP resolver.
using socket = basic_stream_socket;///< A TCP socket.
using acceptor = basic_socket_acceptor; ///< A TCP acceptor.
-using iostream = basic_socket_iostream; /// A TCP iostream.
+using iostream = basic_socket_iostream; ///< A TCP iostream.
#ifdef TCP_NODELAY
/// Disable coalescing of small segments (i.e. the Nagle algorithm).
diff --git a/libstdc++-v3/include/std/iostream
b/libstdc++-v3/include/std/iostream
index d705913f53c..83a238193ce 100644
--- a/libstdc++-v3/include/std/iostream
+++ b/libstdc++-v3/include/std/iostream
@@ -57,16 +57,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
* manual linked to above.
*/
///@{
- extern istream cin; /// Linked to standard input
- extern ostream cout; /// Linked to standard output
- extern ostream cerr; /// Linked to standard error (unbuffered)
- extern ostream clog; /// Linked to standard error (buffered)
+ extern istream cin; ///< Linked to standard input
+ extern ostream cout; ///< Linked to standard output
+ extern ostream cerr; ///< Linked to standard error (unbuffered)
+ extern ostream clog; ///< Linked to standard error (buffered)
#ifdef _GLIBCXX_USE_WCHAR_T
- extern wistream wcin;/// Linked to standard input
- extern wostream wcout; /// Linked to standard output
- extern wostream wcerr; /// Linked to standard error (unbuffered)
- extern wostream wclog; /// Linked to standard error (buffered)
+ extern wistream wcin;///< Linked to standard input
+ extern wostream wcout; ///< Linked to standard output
+ extern wostream wcerr; ///< Linked to standard error (unbuffered)
+ extern wostream wclog; ///< Linked to standard error (buffered)
#endif
///@}
diff --git a/libstdc++-v3/include/std/limits b/libstdc++-v3/include/std/limits
index 66201fa6215..a60611b1b11 100644
--- a/libstdc++-v3/include/std/limits
+++ b/libstdc++-v3/include/std/limits
@@ -166,11 +166,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*/
enum float_round_style
{
-round_indeterminate = -1,/// Intermediate.
-round_toward_zero = 0, /// To zero.
-round_to_nearest = 1, /// To the nearest representable value.
-round_toward_infinity = 2, /// To infinity.
-round_toward_neg_infinity = 3 /// To negative infinity.
+round_indeterminate = -1,///< Intermediate.
+round_toward_zero = 0, ///< To zero.
+round_to_nearest = 1, ///< To the nearest representable value.
+round_toward_infinity = 2, ///< To infinity.
+round_toward_neg_infinity = 3 ///< To negative infinity.
};
/**
--
2.37.3
Adding a new thread model to GCC
Greetings.
After some years I think it's time to put on this topic again.
This patch series is an attempt to add a new thread model basing on the mcfgthread library
(https://github.com/lhmouse/mcfgthread), which provides efficient implementations of mutexes,
condition variables, once flags, etc. for native Windows.
The first patch is necessary because somewhere in libgfortran, `pthread_t` is referenced. If the
thread model is not `posix`, it fails to compile.
The second patch implements `std::thread::hardware_concurrency()` for non-posix thread models. This
would also work for the win32 thread model if `std::thread` would be supported in the future.
The third patch adds the `mcf` thread model for GCC and its libraries. A new builtin macro
`__USING_MCFGTHREAD__` is added to indicate whether this new thread model is in effect. This grants
`std::mutex` and `std::once_flag` trivial destructors; `std::condition_variable` is a bit
unfortunate because its destructor is non-trivial, but in reality no cleanup is performed.
I have been bootstrapping GCC with the MCF thread model for more than five years. At the moment, C,
C++ and Fortran are supported. Ada is untested because I don't know how to bootstrap it. Objective-C
is not supported, because threading APIs for libobjc have not been implemented.
Please review. If there are any changes that I have to make, let me know.
--
Best regards,
LIU Hao
From c522fa74c791ee8904b5906c6e18908b56071db5 Mon Sep 17 00:00:00 2001
From: LIU Hao
Date: Fri, 27 May 2022 23:12:48 +0800
Subject: [PATCH 1/3] libgfortran: Use `__gthread_t` instead of `pthread_t`
It used to cause errors if a thread model other than `posix` was selected,
which looks like a leftover from a79878585a1c5e32bafbc6d1e73f91fd6e4293bf.
libgfortran/ChangeLog:
* io/async.h (struct async_unit): Use `__gthread_t` instead
of `pthread_t`.
---
libgfortran/io/async.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libgfortran/io/async.h b/libgfortran/io/async.h
index efd542a45e82..d57722a95e44 100644
--- a/libgfortran/io/async.h
+++ b/libgfortran/io/async.h
@@ -351,7 +351,7 @@ typedef struct async_unit
struct adv_cond work;
struct adv_cond emptysignal;
struct st_parameter_dt *pdt;
- pthread_t thread;
+ __gthread_t thread;
struct transfer_queue *head;
struct transfer_queue *tail;
--
2.37.3
From fcae3b25b859a207152927797c5ebc520ef3d61a Mon Sep 17 00:00:00 2001
From: LIU Hao
Date: Sun, 2 Oct 2022 00:57:08 +0800
Subject: [PATCH 2/3] libstdc++/thread: Implement `_GLIBCXX_NPROCS` for Windows
This makes `std::thread::hardware_concurrency()` return the number of
logical processors, instead of zero.
libstdc++-v3/ChangeLog:
* src/c++11/thread.cc (get_nprocs): Add new implementation
for native Windows targets
---
libstdc++-v3/src/c++11/thread.cc | 9 +
1 file changed, 9 insertions(+)
diff --git a/libstdc++-v3/src/c++11/thread.cc b/libstdc++-v3/src/c++11/thread.cc
index 707a4ad415b9..b39d9f4a9e29 100644
--- a/libstdc++-v3/src/c++11/thread.cc
+++ b/libstdc++-v3/src/c++11/thread.cc
@@ -68,6 +68,15 @@ static inline int get_nprocs()
#elif defined(_GLIBCXX_USE_SC_NPROC_ONLN)
# include
# define _GLIBCXX_NPROCS sysconf(_SC_NPROC_ONLN)
+#elif defined(_WIN32)
+# include
+static inline int get_nprocs()
+{
+ SYSTEM_INFO sysinfo;
+ GetSystemInfo();
+ return (int)sysinfo.dwNumberOfProcessors;
+}
+# define _GLIBCXX_NPROCS get_nprocs()
#else
# define _GLIBCXX_NPROCS 0
#endif
--
2.37.3
From d0f78f3f83d134d91b59e553b115521f8d67ef52 Mon Sep 17 00:00:00 2001
From: LIU Hao
Date: Sat, 16 Apr 2022 00:46:23 +0800
Subject: [PATCH 3/3] gcc: Add 'mcf' thread model support from mcfgthread
This patch adds the new thread model `mcf`, which implements mutexes
and condition variables with the mcfgthread library.
Source code for mcfgthread is available at
.
config/ChangeLog:
* gthr.m4 (GCC_AC_THREAD_HEADER): Add new case for `mcf` thread
model
gcc/config/ChangeLog:
* i386/mingw-mcfgthread.h: New file
* i386/mingw32.h: Add builtin macro and default libraries for
mcfgthread when thread model is `mcf`
gcc/ChangeLog:
* config.gcc: Include 'i386/mingw-mcfgthread.h' when thread model
is `mcf`
* configure.ac: Recognize `mcf` as a valid thread model
libatomic/ChangeLog:
* configure.tgt: Add new case for `mcf` thread model
libgcc/ChangeLog:
* config.host: Add new cases for `mcf` thread model
* config/i386/gthr-mcf.h: New file
* config/i386/t-mingw-mcfgthread: New file
* config/i386/t-slibgcc-cygming: Make CRT depend on threading
library, not vice versa
libstdc++-v3/ChangeLog:
* libsupc++/atexit_thread.cc (__cxa_thread_atexit): Use
implementation from mcfgthread if available
* libsupc++/guard.cc (__cxa_guard_acquire, __cxa_guard_release,
__cxa_guard_abort):
Re: [Buildroot] [PATCH] or1k: Only define TARGET_HAVE_TLS when HAVE_AS_TLS
Stafford, All, On 2022-10-01 11:35 +, Stafford Horne spake thusly: > On Thu, Sep 29, 2022 at 03:57:40PM +0100, Stafford Horne wrote: > > This was found when testing buildroot with linuxthreads enabled. In > > this case, the build passes --disable-tls to the toolchain during > > configuration. After building the OpenRISC toolchain it was still > > generating TLS code sequences and causing linker failures such as: > > > > /or1k-buildroot-linux-uclibc-gcc -o gpsd-3.24/gpsctl -lusb-1.0 > > -lm -lrt -lnsl > > /ld: /sysroot/usr/lib/libusb-1.0.so: undefined reference to > > `__tls_get_addr' > > > > This patch fixes this by disabling tls for the OpenRISC target when > > requested > > via --disable-tls. > > > > Tested-by: Yann E. MORIN > > > > gcc/ChangeLog: > > > > * config/or1k/or1k.cc (TARGET_HAVE_TLS): Only define if > > HAVE_AS_TLS is defined. > > I have pushed this upstream now. Adding buildroot patches should be easy to > do > now. That's great, thanks for fixing this! :-) I'll poke my work alter-ego on Monday to look at providing the backports for Buildroot. Regards, Yann E. MORIN. -- .-..--.. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 561 099 427 `.---: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL| v conspiracy. | '--^---^--^'
Re: [PATCH] Fix gdb printers for std::string
On Sat, 1 Oct 2022 at 11:43, François Dumont wrote:
>
> On 01/10/22 12:06, Jonathan Wakely wrote:
> > On Sat, 1 Oct 2022 at 08:20, François Dumont via Libstdc++
> > wrote:
> >> I had forgotten to re-run tests after I removed the #define
> >> _GLIBCXX_USE_CXX11_ABI 0.
> >>
> >> The comment was misleading, it could also impact output of std::list.
> >>
> >> I am also restoring the correct std::string alias for
> >> std::__cxx11::basic_string, even if with my workaround it doesn't really
> >> matter as the one for std::basic_string will be used.
> >>
> >> I also restored the printer for std::__cxx11::string typedef. Is it
> >> intentional to keep this ?
> > Yes, I kept that intentionally. There can be programs where some
> > objects still use that typedef, if those objects were compiled with
> > GCC 8.x or older.
> >
> >> libstdc++: Fix gdb pretty printers when dealing with std::string
> >>
> >> Since revision 33b43b0d8cd2de722d177ef823930500948a7487 std::string
> >> and other
> >> similar typedef are ambiguous from a gdb point of view because it
> >> matches both
> >> std::basic_string and std::__cxx11::basic_string
> >> symbols. For those
> >> typedef add a workaround to accept the substitution as long as the
> >> same regardless
> >> of __cxx11 namespace.
> > Thanks for figuring out what was going wrong here, and how to fix it.
> >
> >
> >> Also avoid to register printers for types in std::__cxx11::__8::
> >> namespace, there is
> >> no such symbols.
> >>
> >> libstdc++-v3/ChangeLog:
> >>
> >> * libstdc++-v3/python/libstdcxx/v6/printers.py
> >> (Printer.add_version): Do not add
> >> version namespace for __cxx11 symbols.
> >> (add_one_template_type_printer): Likewise.
> >> (add_one_type_printer): Likewise.
> >> (FilteringTypePrinter._recognizer.recognize): Add a
> >> workaround for std::string & al
> >> ambiguous typedef matching both std:: and std::__cxx11::
> >> symbols.
> >> (register_type_printers): Refine type registration to limit
> >> false positive in
> >> FilteringTypePrinter._recognize.recognize requiring to look
> >> for the type in gdb.
> > I don't really like this part of the change though:
>
> I'll check what you are proposing but I don't think it is necessary to
> fix the problem.
Most of my patch is an alternative way to make the filter match on
"basic_string I did this on my path to find out what was going wrong. Once I
> understood it I consider that it was just a good change to keep. If you
> think otherwise I can revert this part.
Yeah it looks like it's just an optimization to fail faster without
having to do gdb.lookup_type.
Please revert the changes to register_type_printers then, and we can
consider that part later if we want to revisit it. I'm not opposed to
making that fail-fast optimization, as long as we keep the property
that FilteringTypePrinter.match is the class template name. Maybe it
should be renamed to something other than "match" to make that clear.
The rest of the patch is OK for trunk, thanks.
> I also noted that gdb consider the filters as a filo list, not fifo. And
> I think that the 1st filters registered are the most extensively used. I
> can propose to invert all the registration if you think it worth it.
I've not noticed any performance problems with the printers, but I
have wondered how many printers is too many. That's an interesting
observation about the order they're checked. I'll talk to some of the
GDB devs to find out if they think it's something we should worry
about. Let's not try make premature optimizations until we know if it
matters.
>
>
> >
> > @@ -2105,29 +2120,29 @@ def register_type_printers(obj):
> > return
> >
> > # Add type printers for typedefs std::string, std::wstring etc.
> > -for ch in ('', 'w', 'u8', 'u16', 'u32'):
> > -add_one_type_printer(obj, 'basic_string', ch + 'string')
> > -add_one_type_printer(obj, '__cxx11::basic_string', ch + 'string')
> > +for ch in (('char', ''), ('wchar_t', 'w'), ('char8_t', 'u8'),
> > ('char16_t', 'u16'), ('char32_t', 'u32')):
> > +add_one_type_printer(obj, 'basic_string<' + ch[0], ch[1] +
> > 'string')
> > +add_one_type_printer(obj, '__cxx11::basic_string<' + ch[0],
> > ch[1] + 'string')
> >
> >
> > As the docs for FilteringTypePrinter says, the first argument is
> > supposed to be the class template name:
> >
> > class FilteringTypePrinter(object):
> > r"""
> > A type printer that uses typedef names for common template
> > specializations.
> >
> > Args:
> > match (str): The class template to recognize.
> > name (str): The typedef-name that will be used instead.
> >
> > Checks if a specialization of the class template 'match' is the same
> > type
> > as the typedef 'name', and prints it as 'name' instead.
> >
> > e.g. if an
Re: [PATCH] or1k: Only define TARGET_HAVE_TLS when HAVE_AS_TLS
On Thu, Sep 29, 2022 at 03:57:40PM +0100, Stafford Horne wrote: > This was found when testing buildroot with linuxthreads enabled. In > this case, the build passes --disable-tls to the toolchain during > configuration. After building the OpenRISC toolchain it was still > generating TLS code sequences and causing linker failures such as: > > /or1k-buildroot-linux-uclibc-gcc -o gpsd-3.24/gpsctl -lusb-1.0 -lm > -lrt -lnsl > /ld: /sysroot/usr/lib/libusb-1.0.so: undefined reference to > `__tls_get_addr' > > This patch fixes this by disabling tls for the OpenRISC target when requested > via --disable-tls. > > Tested-by: Yann E. MORIN > > gcc/ChangeLog: > > * config/or1k/or1k.cc (TARGET_HAVE_TLS): Only define if > HAVE_AS_TLS is defined. I have pushed this upstream now. Adding buildroot patches should be easy to do now. -Stafford
Re: [PATCH] testsuite: Windows paths use \ and not /
Hi,
I'm really sorry for the mess.
I did test my patch, but I just looked for the PASS/FAIL for the excess
errors and missed that there was an error with the pattern.
In the end, the patch that you pushed is much better. Thanks for fixing
the issue in my absence.
Kind regards,
Torbjörn
On 2022-09-30 23:07, Jonathan Wakely wrote:
On Fri, 30 Sept 2022 at 19:13, Jonathan Wakely via Libstdc++
wrote:
On Fri, 30 Sept 2022 at 19:07, Jonathan Wakely wrote:
On Fri, 30 Sept 2022 at 19:04, Jonathan Wakely wrote:
On Fri, 30 Sept 2022 at 18:55, Jakub Jelinek wrote:
On Fri, Sep 30, 2022 at 06:47:07PM +0100, Jonathan Wakely via Gcc-patches wrote:
On Fri, 30 Sept 2022 at 17:26, Jonathan Wakely wrote:
On Fri, 30 Sept 2022 at 17:04, Torbjörn SVENSSON
wrote:
libstdc++-v3/testsuite:
* 20_util/bind/ref_neg.cc: Prune Windows paths too.
Please CC the libstdc++ for libstdc++ patches.
OK for trunk, thanks.
I'm seeing errors now on x86_64-linux:
ERROR: 20_util/bind/ref_neg.cc: unknown dg option: /\\ for "
dg-prune-output 53 "[/\\](functional|bits/invoke.h):" "
ERROR: 20_util/bind/ref_neg.cc: unknown dg option: /\\ for "
dg-prune-output 53 "[/\\](functional|bits/invoke.h):" "
Bet it should be
// { dg-prune-output "\[/\\](functional|bits\[/\\]invoke.h):" }
or so. Completely untested.
That fixes the error, but now the regex doesn't match so there are
still excess errors. It needs to be:
// { dg-prune-output ".*\[/\\](functional|bits\[/\\]invoke.h):.*" }
Without any regex special characters, there's an implicit .* before
and after the pattern. But when you use any regex special characters
in the pattern, it stops working. I can't remember why. I figured it
out once.
It looks like just adding .* at the start is enough:
// { dg-prune-output ".*\[/\\](functional|bits\[/\\]invoke.h):" }
But that's so ugly, I'm tempted to replace that prune with something different.
I'll finish testing this and push it.
I committed this instead, with no .* in the pattern.
Re: [PATCH] Fix gdb printers for std::string
On 01/10/22 12:06, Jonathan Wakely wrote:
On Sat, 1 Oct 2022 at 08:20, François Dumont via Libstdc++
wrote:
I had forgotten to re-run tests after I removed the #define
_GLIBCXX_USE_CXX11_ABI 0.
The comment was misleading, it could also impact output of std::list.
I am also restoring the correct std::string alias for
std::__cxx11::basic_string, even if with my workaround it doesn't really
matter as the one for std::basic_string will be used.
I also restored the printer for std::__cxx11::string typedef. Is it
intentional to keep this ?
Yes, I kept that intentionally. There can be programs where some
objects still use that typedef, if those objects were compiled with
GCC 8.x or older.
libstdc++: Fix gdb pretty printers when dealing with std::string
Since revision 33b43b0d8cd2de722d177ef823930500948a7487 std::string
and other
similar typedef are ambiguous from a gdb point of view because it
matches both
std::basic_string and std::__cxx11::basic_string
symbols. For those
typedef add a workaround to accept the substitution as long as the
same regardless
of __cxx11 namespace.
Thanks for figuring out what was going wrong here, and how to fix it.
Also avoid to register printers for types in std::__cxx11::__8::
namespace, there is
no such symbols.
libstdc++-v3/ChangeLog:
* libstdc++-v3/python/libstdcxx/v6/printers.py
(Printer.add_version): Do not add
version namespace for __cxx11 symbols.
(add_one_template_type_printer): Likewise.
(add_one_type_printer): Likewise.
(FilteringTypePrinter._recognizer.recognize): Add a
workaround for std::string & al
ambiguous typedef matching both std:: and std::__cxx11::
symbols.
(register_type_printers): Refine type registration to limit
false positive in
FilteringTypePrinter._recognize.recognize requiring to look
for the type in gdb.
I don't really like this part of the change though:
I'll check what you are proposing but I don't think it is necessary to
fix the problem.
I did this on my path to find out what was going wrong. Once I
understood it I consider that it was just a good change to keep. If you
think otherwise I can revert this part.
I also noted that gdb consider the filters as a filo list, not fifo. And
I think that the 1st filters registered are the most extensively used. I
can propose to invert all the registration if you think it worth it.
@@ -2105,29 +2120,29 @@ def register_type_printers(obj):
return
# Add type printers for typedefs std::string, std::wstring etc.
-for ch in ('', 'w', 'u8', 'u16', 'u32'):
-add_one_type_printer(obj, 'basic_string', ch + 'string')
-add_one_type_printer(obj, '__cxx11::basic_string', ch + 'string')
+for ch in (('char', ''), ('wchar_t', 'w'), ('char8_t', 'u8'),
('char16_t', 'u16'), ('char32_t', 'u32')):
+add_one_type_printer(obj, 'basic_string<' + ch[0], ch[1] + 'string')
+add_one_type_printer(obj, '__cxx11::basic_string<' + ch[0],
ch[1] + 'string')
As the docs for FilteringTypePrinter says, the first argument is
supposed to be the class template name:
class FilteringTypePrinter(object):
r"""
A type printer that uses typedef names for common template specializations.
Args:
match (str): The class template to recognize.
name (str): The typedef-name that will be used instead.
Checks if a specialization of the class template 'match' is the same type
as the typedef 'name', and prints it as 'name' instead.
e.g. if an instantiation of std::basic_istream is the same type as
std::istream then print it as std::istream.
"""
With this change, the "class template" is sometimes just a string
prefix of a particular specialization, e.g. "basic_string
Re: [PATCH] Fix gdb printers for std::string
On Sat, 1 Oct 2022 at 08:20, François Dumont via Libstdc++
wrote:
>
> I had forgotten to re-run tests after I removed the #define
> _GLIBCXX_USE_CXX11_ABI 0.
>
> The comment was misleading, it could also impact output of std::list.
>
> I am also restoring the correct std::string alias for
> std::__cxx11::basic_string, even if with my workaround it doesn't really
> matter as the one for std::basic_string will be used.
>
> I also restored the printer for std::__cxx11::string typedef. Is it
> intentional to keep this ?
Yes, I kept that intentionally. There can be programs where some
objects still use that typedef, if those objects were compiled with
GCC 8.x or older.
>
> libstdc++: Fix gdb pretty printers when dealing with std::string
>
> Since revision 33b43b0d8cd2de722d177ef823930500948a7487 std::string
> and other
> similar typedef are ambiguous from a gdb point of view because it
> matches both
> std::basic_string and std::__cxx11::basic_string
> symbols. For those
> typedef add a workaround to accept the substitution as long as the
> same regardless
> of __cxx11 namespace.
Thanks for figuring out what was going wrong here, and how to fix it.
>
> Also avoid to register printers for types in std::__cxx11::__8::
> namespace, there is
> no such symbols.
>
> libstdc++-v3/ChangeLog:
>
> * libstdc++-v3/python/libstdcxx/v6/printers.py
> (Printer.add_version): Do not add
> version namespace for __cxx11 symbols.
> (add_one_template_type_printer): Likewise.
> (add_one_type_printer): Likewise.
> (FilteringTypePrinter._recognizer.recognize): Add a
> workaround for std::string & al
> ambiguous typedef matching both std:: and std::__cxx11::
> symbols.
> (register_type_printers): Refine type registration to limit
> false positive in
> FilteringTypePrinter._recognize.recognize requiring to look
> for the type in gdb.
I don't really like this part of the change though:
@@ -2105,29 +2120,29 @@ def register_type_printers(obj):
return
# Add type printers for typedefs std::string, std::wstring etc.
-for ch in ('', 'w', 'u8', 'u16', 'u32'):
-add_one_type_printer(obj, 'basic_string', ch + 'string')
-add_one_type_printer(obj, '__cxx11::basic_string', ch + 'string')
+for ch in (('char', ''), ('wchar_t', 'w'), ('char8_t', 'u8'),
('char16_t', 'u16'), ('char32_t', 'u32')):
+add_one_type_printer(obj, 'basic_string<' + ch[0], ch[1] + 'string')
+add_one_type_printer(obj, '__cxx11::basic_string<' + ch[0],
ch[1] + 'string')
As the docs for FilteringTypePrinter says, the first argument is
supposed to be the class template name:
class FilteringTypePrinter(object):
r"""
A type printer that uses typedef names for common template specializations.
Args:
match (str): The class template to recognize.
name (str): The typedef-name that will be used instead.
Checks if a specialization of the class template 'match' is the same type
as the typedef 'name', and prints it as 'name' instead.
e.g. if an instantiation of std::basic_istream is the same type as
std::istream then print it as std::istream.
"""
With this change, the "class template" is sometimes just a string
prefix of a particular specialization, e.g. "basic_stringdiff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py
b/libstdc++-v3/python/libstdcxx/v6/printers.py
index 245b6e3dbcd..6a0b8a22f1d 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -2045,25 +2045,35 @@ class FilteringTypePrinter(object):
Args:
match (str): The class template to recognize.
name (str): The typedef-name that will be used instead.
+targ1 (str, optional): The first template argument. Defaults to None.
+If arg1 is provided, only match specializations with this type
+as the first template argument, e.g. if match='basic_string'
Checks if a specialization of the class template 'match' is the same type
as the typedef 'name', and prints it as 'name' instead.
-e.g. if an instantiation of std::basic_istream is the same type as
+e.g. for match='basic_istream', name='istream', if any specialization of
+std::basic_istream is the same type as std::istream then print it as
+std::istream.
+
+e.g. for match='basic_istream', name='istream', targ1='char', if any
+specialization of std::basic_istream is the same type as
std::istream then print it as std::istream.
"""
-def __init__(self, match, name):
+def __init__(self, match, name, targ1 = None):
self.match = match
self.name = name
+self.targ1 = targ1
self.enabled = True
class _recognizer(object):
"The recognizer class for FilteringTypePrinter."
-def __init__(self, match, name):
+
Re: [PATCH] OpenACC: Fix struct-component-kind-1.c test
On Sat, Oct 01, 2022 at 12:56:59AM -0700, Julian Brown wrote: > This patch is a minimal fix for the recently-added > struct-component-kind-1.c test (which is currently failing to emit one > of the errors it expects in scan output). This fragment was erroneously > omitted from the second version of the patch posted previously: > > https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602504.html > > Re-tested with offloading to NVPTX. OK? > > Thanks, > > Julian > > 2022-10-01 Julian Brown > > gcc/ > * gimplify.cc (omp_group_base): Fix IF_PRESENT (no_create) > handling. Ok. > --- > gcc/gimplify.cc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc > index 90606088128..9e0e3429958 100644 > --- a/gcc/gimplify.cc > +++ b/gcc/gimplify.cc > @@ -9245,6 +9245,7 @@ omp_group_base (omp_mapping_group *grp, unsigned int > *chained, > case GOMP_MAP_RELEASE: > case GOMP_MAP_DELETE: > case GOMP_MAP_FORCE_ALLOC: > +case GOMP_MAP_IF_PRESENT: >if (node == grp->grp_end) > return node; > > @@ -9323,7 +9324,6 @@ omp_group_base (omp_mapping_group *grp, unsigned int > *chained, > case GOMP_MAP_FORCE_DEVICEPTR: > case GOMP_MAP_DEVICE_RESIDENT: > case GOMP_MAP_LINK: > -case GOMP_MAP_IF_PRESENT: > case GOMP_MAP_FIRSTPRIVATE: > case GOMP_MAP_FIRSTPRIVATE_INT: > case GOMP_MAP_USE_DEVICE_PTR: > -- > 2.29.2 Jakub
[PATCH] OpenACC: Fix struct-component-kind-1.c test
This patch is a minimal fix for the recently-added struct-component-kind-1.c test (which is currently failing to emit one of the errors it expects in scan output). This fragment was erroneously omitted from the second version of the patch posted previously: https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602504.html Re-tested with offloading to NVPTX. OK? Thanks, Julian 2022-10-01 Julian Brown gcc/ * gimplify.cc (omp_group_base): Fix IF_PRESENT (no_create) handling. --- gcc/gimplify.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc index 90606088128..9e0e3429958 100644 --- a/gcc/gimplify.cc +++ b/gcc/gimplify.cc @@ -9245,6 +9245,7 @@ omp_group_base (omp_mapping_group *grp, unsigned int *chained, case GOMP_MAP_RELEASE: case GOMP_MAP_DELETE: case GOMP_MAP_FORCE_ALLOC: +case GOMP_MAP_IF_PRESENT: if (node == grp->grp_end) return node; @@ -9323,7 +9324,6 @@ omp_group_base (omp_mapping_group *grp, unsigned int *chained, case GOMP_MAP_FORCE_DEVICEPTR: case GOMP_MAP_DEVICE_RESIDENT: case GOMP_MAP_LINK: -case GOMP_MAP_IF_PRESENT: case GOMP_MAP_FIRSTPRIVATE: case GOMP_MAP_FIRSTPRIVATE_INT: case GOMP_MAP_USE_DEVICE_PTR: -- 2.29.2
Re: [PATCH] Process unsigned overflow relations for plus and minus in range-ops.
On Thu, 29 Sep 2022 18:38:10 -0400
Andrew MacLeod via Gcc-patches wrote:
> diff --git a/gcc/range-op.cc b/gcc/range-op.cc
> index 9bb04c361d0..830c64bd6b9 100644
> --- a/gcc/range-op.cc
> +++ b/gcc/range-op.cc
> @@ -1305,22 +1305,123 @@ operator_plus::wi_fold (irange , tree type,
>value_range_with_overflow (r, type, new_lb, new_ub, ov_lb, ov_ub);
> }
>
> +// Given addition or subtraction, determine the possible NORMAL ranges and
> +// OVERFLOW ranges given an OFFSET range. ADD_P is true for addition.
> +// Return the relation that exists between the LHS and OP1 in order for the
> +// NORMAL range to apply.
> +// a return value of VREL_VARYING means no ranges were applicable.
Capital A in A return value
> +
> +static relation_kind
> +plus_minus_ranges (irange _ov, irange _normal, const irange ,
> + bool add_p)
> +{
> + relation_kind kind = VREL_VARYING;
> + // For now, only deal with constant adds. This could be extended to ranges
> + // when someone is so motivated.
> + if (!offset.singleton_p () || offset.zero_p ())
> +return kind;
> +
> + // Always work with a positive offset. ie a+ -2 -> a-2 and a- -2 > a+2
> + wide_int off = offset.lower_bound ();
> + if (wi::neg_p (off, SIGNED))
> +{
> + add_p = !add_p;
> + off = wi::neg (off);
> +}
> +
> + wi::overflow_type ov;
> + tree type = offset.type ();
> + unsigned prec = TYPE_PRECISION (type);
> + wide_int ub;
> + wide_int lb;
> + // calculate the normal range and relation for the operation.
> + if (add_p)
> +{
> + // [ 0 , INF - OFF]
> + lb = wi::zero (prec);
> + ub = wi::sub (wi::to_wide (vrp_val_max (type)), off, UNSIGNED, );
> + kind = VREL_GT;
> +}
> + else
> +{
> + // [ OFF, INF ]
> + lb = off;
> + ub = wi::to_wide (vrp_val_max (type));
> + kind = VREL_LT;
> +}
> + int_range<2> normal_range (type, lb, ub);
> + int_range<2> ov_range (type, lb, ub, VR_ANTI_RANGE);
> +
> + r_ov = ov_range;
> + r_normal = normal_range;
> + return kind;
> +}
> +
> +// Once op1 has been calculated by operator_plus or operator_minus, check
> +// to see if the relation passed causes any part of the calculation to
> +// be not possible. ie
> +// a_2 = b_3 + 1 with a_2 < b_3 can refine the range of b_3 to [INF, INF]
> +// and that further refines a_2 to [0, 0].
> +// R is the value of op1, OP2 is the offset being added/subtracted, REL is
> the
> +// relation between LHS relatoin OP1 and ADD_P is true for PLUS, false for
> +// MINUS.IF any adjustment can be made, R will reflect it.
s/relatoin/relation/
Excess space before the last sentense, or should this go to a new line?
> +
> +static void
> +adjust_op1_for_overflow (irange , const irange , relation_kind rel,
> + bool add_p)
> +{
> + tree type = r.type ();
> + // Check for unsigned overflow and calculate the overflow part.
> + signop s = TYPE_SIGN (type);
> + if (!TYPE_OVERFLOW_WRAPS (type) || s == SIGNED)
> +return;
> +
> + // Only work with <, <=, >, >= relations.
> + if (!relation_lt_le_gt_ge_p (rel))
> +return;
> +
> + // Get the ranges for this offset.
> + int_range_max normal, overflow;
> + relation_kind k = plus_minus_ranges (overflow, normal, op2, add_p);
> +
> + // VREL_VARYING means there are no adjustments.
> + if (k == VREL_VARYING)
> +return;
> +
> + // If the relations match use the normal range, otherwise use overflow
> range.
> + if (relation_intersect (k, rel) == k)
> +r.intersect (normal);
> + else
> +r.intersect (overflow);
> + return;
> +}
> +
> bool
> operator_plus::op1_range (irange , tree type,
> const irange ,
> const irange ,
> - relation_kind rel ATTRIBUTE_UNUSED) const
> + relation_kind rel) const
> {
> - return range_op_handler (MINUS_EXPR, type).fold_range (r, type, lhs, op2);
> + if (lhs.undefined_p ())
> +return false;
> + // Start with the default operation.
> + range_op_handler minus (MINUS_EXPR, type);
> + if (!minus)
> +return false;
> + bool res = minus.fold_range (r, type, lhs, op2);
> + // Check for a relation refinement.
> + if (res)
> +adjust_op1_for_overflow (r, op2, rel, true /* PLUS_EXPR */);
> + return res;
> }
>
> bool
> operator_plus::op2_range (irange , tree type,
> const irange ,
> const irange ,
> - relation_kind rel ATTRIBUTE_UNUSED) const
> + relation_kind rel) const
> {
> - return range_op_handler (MINUS_EXPR, type).fold_range (r, type, lhs, op1);
> + return op1_range (r, type, lhs, op1, rel);
> }
>
>
> @@ -1472,7 +1573,17 @@ operator_minus::op1_range (irange , tree type,
> const irange ,
> relation_kind rel ATTRIBUTE_UNUSED) const
You could remove ATTRIBUTE_UNUSED above for rel like you did for the
other operators.
>
Re: [PATCH] Fix gdb printers for std::string
I had forgotten to re-run tests after I removed the #define _GLIBCXX_USE_CXX11_ABI 0. The comment was misleading, it could also impact output of std::list. I am also restoring the correct std::string alias for std::__cxx11::basic_string, even if with my workaround it doesn't really matter as the one for std::basic_string will be used. I also restored the printer for std::__cxx11::string typedef. Is it intentional to keep this ? libstdc++: Fix gdb pretty printers when dealing with std::string Since revision 33b43b0d8cd2de722d177ef823930500948a7487 std::string and other similar typedef are ambiguous from a gdb point of view because it matches both std::basic_string and std::__cxx11::basic_string symbols. For those typedef add a workaround to accept the substitution as long as the same regardless of __cxx11 namespace. Also avoid to register printers for types in std::__cxx11::__8:: namespace, there is no such symbols. libstdc++-v3/ChangeLog: * libstdc++-v3/python/libstdcxx/v6/printers.py (Printer.add_version): Do not add version namespace for __cxx11 symbols. (add_one_template_type_printer): Likewise. (add_one_type_printer): Likewise. (FilteringTypePrinter._recognizer.recognize): Add a workaround for std::string & al ambiguous typedef matching both std:: and std::__cxx11:: symbols. (register_type_printers): Refine type registration to limit false positive in FilteringTypePrinter._recognize.recognize requiring to look for the type in gdb. * libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx17.cc: Remove obsolete \#define _GLIBCXX_USE_CXX11_ABI 0. * libstdc++-v3/testsuite/libstdc++-prettyprinters/simple.cc: Likewise. Adapt test to accept std::__cxx11::list. * libstdc++-v3/testsuite/libstdc++-prettyprinters/simple11.cc: Likewise. * libstdc++-v3/testsuite/libstdc++-prettyprinters/whatis.cc: Likewise. * libstdc++-v3/testsuite/libstdc++-prettyprinters/80276.cc: Likewise and remove xfail for c++20 and debug mode. * libstdc++-v3/testsuite/libstdc++-prettyprinters/libfundts.cc: Likewise. Ok to commit ? François On 28/09/22 22:42, François Dumont wrote: Sometimes substitution of basic_string by one of the std::string typeedef fails. Here is the fix. libstdc++: Fix gdb pretty printers when dealing with std::string Since revision 33b43b0d8cd2de722d177ef823930500948a7487 std::string and other similar typedef are ambiguous from a gdb point of view because it matches both std::basic_string and std::__cxx11::basic_string symbols. For those typedef add a workaround to accept the substitution as long as the same regardless of __cxx11 namespace. Also avoid to register printers for types in std::__cxx11::__8:: namespace, there is no such symbols. libstdc++-v3/ChangeLog: * libstdc++-v3/python/libstdcxx/v6/printers.py (Printer.add_version): Do not add version namespace for __cxx11 symbols. (add_one_template_type_printer): Likewise. (add_one_type_printer): Likewise. (FilteringTypePrinter._recognizer.recognize): Add a workaround for std::string & al ambiguous typedef matching both std:: and std::__cxx11:: symbols. (register_type_printers): Refine type registration to limit false positive in FilteringTypePrinter._recognize.recognize requiring to look for the type in gdb. * libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx17.cc: Remove obsolete \#define _GLIBCXX_USE_CXX11_ABI 0. * libstdc++-v3/testsuite/libstdc++-prettyprinters/simple.cc: Likewise. * libstdc++-v3/testsuite/libstdc++-prettyprinters/simple11.cc: Likewise. * libstdc++-v3/testsuite/libstdc++-prettyprinters/whatis.cc: Likewise. * libstdc++-v3/testsuite/libstdc++-prettyprinters/80276.cc: Likewise and remove xfail for c++20 and debug mode. * libstdc++-v3/testsuite/libstdc++-prettyprinters/libfundts.cc: Likewise. Tested under x86_64 linux w/o _GLIBCXX_INLINE_VERSION and w/o _GLIBCXX_DEBUG. I also tested it with my patch to use cxx11 abi in _GLIBCXX_INLINE_VERSION mode. Ok to commit ? François diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py index 245b6e3dbcd..0f966fc79a7 100644 --- a/libstdc++-v3/python/libstdcxx/v6/printers.py +++ b/libstdc++-v3/python/libstdcxx/v6/printers.py @@ -1857,7 +1857,7 @@ class Printer(object): # Add a name using _GLIBCXX_BEGIN_NAMESPACE_VERSION. def add_version(self, base, name, function): self.add(base + name, function) -if _versioned_namespace: +if _versioned_namespace and not '__cxx11' in base:
