Re: [Ada] Read directory in Ada.Directories.Start_Search rather than Get_Next_Entry
Hi Pierre-Marie, is this really a good idea? If a directory has millions of files in it (rare, but I've seen it) this may consume a lot of memory. Also, if using a slow medium like a network file system, reading the entire directory contents may take a long time. Finally, you aren't really solving the race condition, you're just making the window smaller, right? After all, if I understand right you are still using readdir, you just use it during a shorter time period. Best wishes, Duncan. On 07/01/2022 17:27, Pierre-Marie de Rodat via Gcc-patches wrote: The Ada.Directories directory search function is changed so the contents of the directory is now read in Start_Search instead of in Get_Next_Entry. Start_Search now stores the result of the directory search in the search object, with Get_Next_Entry returning results from the search object. This differs from the prior implementation where Get_Next_Entry would query the directory directly for the next item using the POSIX readdir function. The problem with building Get_Next_Entry around the readdir function is POSIX does not specify the behavior of readdir when files are added or removed from the directory being read. For example: on most systems, deleting files from the folder being read does not impact readdir. However, some systems, like RTEMS and HFS+ volumes on macOS, will return NULL instead of the next item in the directory if the current item returned by readdir is deleted. To avoid this issue, the contents of the directory is read in Start_Search and the user is given a copy of these results. Consequently, any subsequent modification to the directory does not affect the ability to iterate through the results. This approach is the same taken by the popular fts C functions. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * libgnat/a-direct.adb (Search_Data): Remove type. (Directory_Vectors): New package instantiation. (Search_State): New type. (Fetch_Next_Entry): Remove. (Close): Remove. (Finalize): Rewritten. (Full_Name): Ditto. (Get_Next_Entry): Return next entry from Search results vector rather than querying the directory directly using readdir. (Kind): Rewritten. (Modification_Time): Rewritten. (More_Entries): Use Search state cursor to determine if more entries are available for users to read. (Simple_Name): Rewritten. (Size): Rewritten. (Start_Search_Internal): Rewritten to load the contents of the directory that matches the pattern and filter into the search object. * libgnat/a-direct.ads (Search_Type): New type. (Search_Ptr): Ditto. (Directory_Entry_Type): Rewritten to support new Start_Search procedure. * libgnat/s-filatt.ads (File_Length_Attr): New function.
Re: [Ada] Improve performance of 'Image with enumeration types.
On 09/26/2017 12:17 PM, Eric Botcazou wrote: By the way, why not always do this "inlining", even when not optimizing? Because this generates more bloated code and inferior debugging experience. This is a trick question, because when you answer "because XYZ" I will then reply "but XYZ is a common reason that people disable inlining when optimizing, so shouldn't you only do it when inlining is enabled?" :) People ought not to disable inlining when optimizing though. I've seen a few projects disable inlining when optimizing because it can generate bloated code and an inferior debugging experience :) But I won't argue the point any further (that this should really be conditioned on inlining being enabled, not on optimization being enabled) as while I'm probably right in theory, in practice I doubt it will actually cause trouble for anyone. Best wishes, Duncan.
Re: [Ada] Improve performance of 'Image with enumeration types.
Hi Arno, it looks like this is in essence inlining the run-time library routine. In which case, shouldn't you only do it if inlining is enabled? For example, it seems rather odd to do this if compiling with -Os. Actually, measurements showed that this instance of inlining is a win for both performance and code size, so it???s a good candidate even for -Os. Note that we inline string concatenation routines for the same reason. thanks for explaining. I think it merits a comment in the code though. By the way, why not always do this "inlining", even when not optimizing? That's a practical trade off, based on our past experience. if it's a trade-off then there must be a down-side. What is the down-side? Best wishes, Duncan.
Re: [Ada] Improve performance of 'Image with enumeration types.
Hi Pierre-Marie, On 09/26/2017 11:30 AM, Pierre-Marie de Rodat wrote: On 09/25/2017 02:47 PM, Duncan Sands wrote: it looks like this is in essence inlining the run-time library routine. In which case, shouldn't you only do it if inlining is enabled? For example, it seems rather odd to do this if compiling with -Os. Actually, measurements showed that this instance of inlining is a win for both performance and code size, so it’s a good candidate even for -Os. Note that we inline string concatenation routines for the same reason. thanks for explaining. I think it merits a comment in the code though. By the way, why not always do this "inlining", even when not optimizing? This is a trick question, because when you answer "because XYZ" I will then reply "but XYZ is a common reason that people disable inlining when optimizing, so shouldn't you only do it when inlining is enabled?" :) Best wishes, Duncan. PS: I'm imagining XYZ is related to a better debugging experience.
Re: [Ada] Improve performance of 'Image with enumeration types.
Hi, On 09/25/2017 10:54 AM, Pierre-Marie de Rodat wrote: This patch improves the performance of the code generated by the compiler for attribute Image when applied to user-defined enumeration types and the sources are compiled with optimizations enabled. it looks like this is in essence inlining the run-time library routine. In which case, shouldn't you only do it if inlining is enabled? For example, it seems rather odd to do this if compiling with -Os. Best wishes, Duncan. No test required. Tested on x86_64-pc-linux-gnu, committed on trunk 2017-09-25 Javier Miranda * exp_imgv.adb (Is_User_Defined_Enumeration_Type): New subprogram. (Expand_User_Defined_Enumeration_Image): New subprogram. (Expand_Image_Attribute): Enable speed-optimized expansion of user-defined enumeration types when we are compiling with optimizations enabled.
Re: [Ada] Use the Monotonic Clock on Linux
Hi, On 09/25/2017 10:47 AM, Pierre-Marie de Rodat wrote: The monotonic clock epoch is set to some undetermined time in the past (typically system boot time). In order to use the monotonic clock for absolute time, the offset from a known epoch is calculated and incorporated into timed delay and sleep. --- libgnarl/s-taprop__linux.adb(revision 253134) +++ libgnarl/s-taprop__linux.adb(working copy) @@ -257,6 +266,73 @@ end if; end Abort_Handler; + -- + -- Compute_Base_Monotonic_Clock -- + -- + + function Compute_Base_Monotonic_Clock return Duration is + TS_Bef0, TS_Mon0, TS_Aft0 : aliased timespec; + TS_Bef, TS_Mon, TS_Aft : aliased timespec; + Bef, Mon, Aft : Duration; + Res_B, Res_M, Res_A : Interfaces.C.int; + begin + Res_B := clock_gettime + (clock_id => OSC.CLOCK_REALTIME, tp => TS_Bef0'Unchecked_Access); + pragma Assert (Res_B = 0); + Res_M := clock_gettime + (clock_id => OSC.CLOCK_RT_Ada, tp => TS_Mon0'Unchecked_Access); + pragma Assert (Res_M = 0); + Res_A := clock_gettime + (clock_id => OSC.CLOCK_REALTIME, tp => TS_Aft0'Unchecked_Access); + pragma Assert (Res_A = 0); + + for I in 1 .. 10 loop + -- Guard against a leap second which will cause CLOCK_REALTIME + -- to jump backwards. In the extrenmely unlikely event we call + -- clock_gettime before and after the jump the epoch result will + -- be off slightly. + -- Use only results where the tv_sec values match for the sake + -- of convenience. + -- Also try to calculate the most accurate + -- epoch by taking the minimum difference of 10 tries. + + Res_B := clock_gettime + (clock_id => OSC.CLOCK_REALTIME, tp => TS_Bef'Unchecked_Access); + pragma Assert (Res_B = 0); + Res_M := clock_gettime + (clock_id => OSC.CLOCK_RT_Ada, tp => TS_Mon'Unchecked_Access); + pragma Assert (Res_M = 0); + Res_A := clock_gettime + (clock_id => OSC.CLOCK_REALTIME, tp => TS_Aft'Unchecked_Access); + pragma Assert (Res_A = 0); + + if (TS_Bef0.tv_sec /= TS_Aft0.tv_sec and then + TS_Bef.tv_sec = TS_Aft.tv_sec) +-- The calls to clock_gettime before the loop were no good. +or else +(TS_Bef0.tv_sec = TS_Aft0.tv_sec and then + TS_Bef.tv_sec = TS_Aft.tv_sec and then +(TS_Aft.tv_nsec - TS_Bef.tv_nsec < + TS_Aft0.tv_nsec - TS_Bef0.tv_nsec)) +-- The most recent calls to clock_gettime were more better. were more better -> were better Best wishes, Duncan. + then +TS_Bef0.tv_sec := TS_Bef.tv_sec; +TS_Bef0.tv_nsec := TS_Bef.tv_nsec; +TS_Aft0.tv_sec := TS_Aft.tv_sec; +TS_Aft0.tv_nsec := TS_Aft.tv_nsec; +TS_Mon0.tv_sec := TS_Mon.tv_sec; +TS_Mon0.tv_nsec := TS_Mon.tv_nsec; + end if; + end loop; + + Bef := To_Duration (TS_Bef0); + Mon := To_Duration (TS_Mon0); + Aft := To_Duration (TS_Aft0); + + return Bef / 2 + Aft / 2 - Mon; + -- Distribute the division to avoid potential type overflow someday. + end Compute_Base_Monotonic_Clock; + -- -- Lock_RTS -- --
Re: [patch] Restore cross-language inlining into Ada
Hi Eric, On 23/01/16 10:25, Eric Botcazou wrote: I think we was inlining them with LTO until I installed the patch. Most of time DECL_STRUCT_FUNCTION == NULL for WPA and thus the original check testing the flags was disabled. We did not update the EH coddegen during inlining, so probably we just did not produce non-call EH for these. OK, we may have inlined them after all... My understanding of the new code is that we will still inline them if the Ada callee doesn't use EH, which is good enough in my opinion. it would be nice to also inline if the caller doesn't use EH even if the callee does, for example when calling Ada from C. Best wishes, Duncan.
Re: [Ada] More efficient code generated for object overlays
Hi Arnaud, On 12/11/15 12:06, Arnaud Charlet wrote: This change refines the use of the "volatile hammer" to implement the advice given in RM 13.3(19) by disabling it for object overlays altogether. relying instead on the ref-all aliasing property of reference types to achieve the desired effect. This will generate better code for object overlays, for example the following function should now make no memory accesses at all on 64-bit platforms when compiled at -O2 or above: this is great! When doing tricks to improve performance I've several times resorted to address overlays, forgetting about the "volatile hammer", only to rediscover it for the N'th time due to the poor performance and the horrible code generated. Best wishes, Duncan.
Re: [Ada] Correct some anmolies in the handling of Atomic
Hi Arnaud, Index: exp_util.adb === --- exp_util.adb(revision 223476) +++ exp_util.adb(working copy) @@ -204,6 +204,13 @@ when others => null; end case; + -- Nothing to do for the identifier in an object renaming declaration, + -- the renaming itself does not need atomic syncrhonization. syncrhonization -> synchronization Ciao, Duncan.
Re: [Ada] Out parameters of a null-excluding access type in entries.
Hi Arnaud,
On 29/07/14 16:02, Arnaud Charlet wrote:
If a procedure or entry has an formal out-parameter of a null-excluding access
type, there is no check applied to the actual before the call. This patch
removes a spurious access check on such parameters on entry calls.
Compiling and executing p.adb must yield;
Procedure version did not raise exception
Entry version did not raise exception
---
with Ada.Text_IO; use Ada.Text_IO;
procedure P is
type Integer_Access is access all Integer;
An_Integer : aliased Integer;
procedure Procedure_Version (A : out not null Integer_Access) is
begin
A := An_Integer'Access;
end Procedure_Version;
protected Object is
entry Entry_Version (A : out not null Integer_Access);
end Object;
protected body Object is
entry Entry_Version (A : out not null Integer_Access) when True is
Junk : integer := 0;
this variable "Junk" seems useless.
begin
A := An_Integer'Access;
end Entry_Version;
end Object;
A : Integer_Access;
begin
A := null;
Procedure_Version (A);
Put_Line ("Procedure version did not raise exception");
A := null;
Object.Entry_Version (A);
Put_Line ("Entry version did not raise exception");
end;
Ciao, Duncan.
Re: [Ada] Imported C++ exceptions
Hi Arnaud, On 14/10/13 15:29, Arnaud Charlet wrote: It is now possible to import C++ exceptions and to handle it. ... Index: exp_prag.adb === --- exp_prag.adb(revision 203544) +++ exp_prag.adb(working copy) @@ -575,6 +575,64 @@ if No (Init_Call) and then Present (Expression (Parent (Def_Id))) then Set_Expression (Parent (Def_Id), Empty); end if; + elsif Ekind (Def_Id) = E_Exception +and then Convention (Def_Id) = Convention_CPP + then + + -- Import a C++ convention should this comment say "Import a C++ exception"? Ciao, Duncan.
Re: [GOOGLE] More strict checking for call args
Hi Dehao,
On 31/05/13 00:47, Dehao Chen wrote:
This patch makes more strict check of call args to make sure the
number of args match.
Bootstrapped and passed regression tests.
did you thoroughly test Fortran? The Fortran front-end has long had an
unfortunate tendency to eg declare a function as taking 4 int arguments,
but in the call pass it one argument (an array of length 4, consisting
of ints). It would be great if all such nastiness has been fixed. There
are also a few cases in which it declares a builtin as taking, say, an
int,float pair, but passes a float,int pair in the call. I fixed a couple
of instances of this a while back, but I still have one outstanding patch.
Ciao, Duncan.
OK for google branches?
Thanks,
Dehao
Index: gcc/gimple-low.c
===
--- gcc/gimple-low.c (revision 199414)
+++ gcc/gimple-low.c (working copy)
@@ -254,9 +254,13 @@ gimple_check_call_args (gimple stmt, tree fndecl)
&& !fold_convertible_p (DECL_ARG_TYPE (p), arg)))
return false;
}
+ if (p != NULL)
+ return false;
}
else if (parms)
{
+ if (list_length (parms) - nargs != 1)
+ return false;
for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
{
tree arg;
Re: PATCH: PR plugins/56754 some missing plugin headers during installation in gcc 4.8
Hi Jakub, I actually committed this patch to mainline earlier today, as it is trivial, enables my own plugin (dragonegg) to compile against gcc-4.8, and according to the PR makes some other plugins work with gcc-4.8 too. I will backport it to the gcc-4.8 branch if no-one objects. But maybe you are objecting? On 21/05/13 17:09, Jakub Jelinek wrote: On Sat, Mar 30, 2013 at 03:17:59PM +0100, Magnus Granberg wrote: This patch readd TARGET_H that was removed with revision 188166 IPA_PROP_H is in use by PLUGIN_HEADERS and did depend on GIMPLE_H that did have TARGET_H before it was removed and it was not added to IPA_PROP_H or PLUGIN_HEADERS. See the bug for more info. 2013-03-30 Magnus Granberg Two spaces before <, instead of just one. I had corrected this one already in the version I committed. PR plugins/56754 * Makefile.in (PLUGIN_HEADERS): Add TARGET_H Missing dot at the end of line, plus it should be $(TARGET_H) instead of TARGET_H. I missed these however. I will correct the changelog if you are otherwise OK with the commit. Where has it been tested? In addition to what Jack mentioned, x86-64 ubuntu 13.04. Ciao, Duncan. --- a/gcc/Makefile.in 2013-02-08 10:07:49.0 +0100 +++ b/gcc/Makefile.in 2013-03-28 03:43:53.343390945 +0100 @@ -4597,7 +4597,7 @@ PLUGIN_HEADERS = $(TREE_H) $(CONFIG_H) $ $(C_PRAGMA_H) $(CPPLIB_H) $(FUNCTION_H) \ cppdefault.h flags.h $(MD5_H) params.def params.h prefix.h tree-inline.h \ $(GIMPLE_PRETTY_PRINT_H) realmpfr.h \ - $(IPA_PROP_H) $(RTL_H) $(TM_P_H) $(CFGLOOP_H) $(EMIT_RTL_H) version.h + $(IPA_PROP_H) $(TARGET_H) $(RTL_H) $(TM_P_H) $(CFGLOOP_H) $(EMIT_RTL_H) version.h # generate the 'build fragment' b-header-vars s-header-vars: Makefile Jakub
Re: [Ada] Lock-free implementation of protected objects
Hi Arnaud, --- s-atopri.adb(revision 189768) +++ s-atopri.adb(working copy) @@ -31,6 +31,58 @@ package body System.Atomic_Primitives is + -- + -- Lock_Free_Read_8 -- + -- ... + -- + -- Lock_Free_Read_16 -- + -- ... + -- + -- Lock_Free_Read_32 -- + -- There's a bunch or wrong comment formatting (notice how "" lines aren't always long enough except for the _8 case). Ciao, Duncan.
Re: [Ada] Ease interface with builtins that returns void *
PS: That said, I have to admit that using void* for builtins does cover the most important cases.
Re: [Ada] Ease interface with builtins that returns void *
Hi Tristan, Ah, what you want is the use of 'void *' for System.Address. We didn't choose that because the semantic of System.Address (which includes arithmetic on the whole address space) doesn't match the void * one. void* arithmetic of this kind exists, it's a gcc extension to C :) The issue is not void * vs char *, but the fact that the C standard has restriction on pointer arithmetic. I see, -fno-strict-overflow would be needed to get System.Address modulo arithmetic semantics if it was always turned into void*, which would then presumably pessimize other code. However this isn't really relevant to whether Address formal parameters should always be turned into void* or not. Ciao, Duncan. But, you can try to implement this scheme by modifying the runtime. I don't know if this is a small work or not. It crashes the front-end, so it's not trivial. :-)
Re: [Ada] Ease interface with builtins that returns void *
Hi Tristan, Ah, what you want is the use of 'void *' for System.Address. We didn't choose that because the semantic of System.Address (which includes arithmetic on the whole address space) doesn't match the void * one. void* arithmetic of this kind exists, it's a gcc extension to C :) But, you can try to implement this scheme by modifying the runtime. I don't know if this is a small work or not. It crashes the front-end, so it's not trivial. Ciao, Duncan.
Re: [Ada] Ease interface with builtins that returns void *
Hi Tristan, indeed, for two years already. Is there any reason not to do this for all functions, rather than just limiting it to builtins? I don't understand what do you mean. We need to do this implicit conversion for builtins because they are known by the compiler. Which other functions (that aren't builtins) are you referring to ? all of them! First off, the LLVM optimizers do a better job if an argument of a user defined function that is really a pointer is declared as such, rather than declared as an integer then cast to a pointer before being used. I don't know if the GCC optimizers are sensitive to this too. Also, the LLVM optimizers recognize some standard library functions that the gcc optimizers do not, but fail to recognize them when called from Ada because they have the wrong prototype: an integer rather than a pointer argument. Finally I would argue that as System.Address is really a pointer, playing pretty much exactly the same role as void* in C, it is more philosophically correct to express it as a void*. That said, it should probably just be declared as a pointer in the System package rather than doing all this mucking around in the gcc interface. Ciao, Duncan.
Re: [Ada] Ease interface with builtins that returns void *
Hi Tristan, On 16/07/12 15:17, Tristan Gingold wrote: On Jul 16, 2012, at 3:16 PM, Duncan Sands wrote: Hi Arnaud, The natural way to import a builtin that returns void * is to use System.Address in Ada, which is in fact an integral type. how about doing this for formal arguments too and not just the return type? Formal arguments were already handled. indeed, for two years already. Is there any reason not to do this for all functions, rather than just limiting it to builtins? Ciao, Duncan. Tristan. This would improve optimization by LLVM of calls to standard library functions since the optimizers bail out when they see an "int" parameter where normally there would be a void* (or other pointer type). Ciao, Duncan. Addressed by this patch, which makes it possible to e.g. compile: with System; procedure Btins1 is function Frame_Address (Level : Integer) return System.Address; pragma Import (Intrinsic, Frame_Address, "__builtin_frame_address"); Ptr : System.Address; pragma Volatile (Ptr); begin Ptr := Frame_Address (0); end; Tested on x86_64-pc-linux-gnu, committed on trunk 2012-07-16 Tristan Gingold * gcc-interface/decl.c (intrin_return_compatible_p): Map Address to void *.
Re: [Ada] Ease interface with builtins that returns void *
Hi Arnaud, The natural way to import a builtin that returns void * is to use System.Address in Ada, which is in fact an integral type. how about doing this for formal arguments too and not just the return type? This would improve optimization by LLVM of calls to standard library functions since the optimizers bail out when they see an "int" parameter where normally there would be a void* (or other pointer type). Ciao, Duncan. Addressed by this patch, which makes it possible to e.g. compile: with System; procedure Btins1 is function Frame_Address (Level : Integer) return System.Address; pragma Import (Intrinsic, Frame_Address, "__builtin_frame_address"); Ptr : System.Address; pragma Volatile (Ptr); begin Ptr := Frame_Address (0); end; Tested on x86_64-pc-linux-gnu, committed on trunk 2012-07-16 Tristan Gingold * gcc-interface/decl.c (intrin_return_compatible_p): Map Address to void *.
Re: [Patch 4.6] In system.h, wrap include of C++ header in 'extern C++'
Hi, If ENABLE_BUILD_WITH_CXX is defined, then GCC itself is built with C++, and we want a C++ signature for functions. If it is not defined, then GCC itself is not built with C++, and we want (and must have) a C signature. I suppose we would decide that fancy_abort always uses a C signature, but that seems odd. Ian I guess the issue is when people care only about C plugins, yet fancy_abort get implicitly exported with a C++ linkage. I suspect this goes back to the eternal question: what do we consider as part of the public GCC public API (no, Basile, I am not suggesting to have the same discussion again.) if the following are to hold (1) fancy_abort is declared in system.h (2) system.h should not be wrapped in extern "C" when included from a plugin, (3) it should be valid to include it from plugins compiled as C or as C++, (4) fancy_abort should use the same linkage as GCC, i.e. C when GCC built as C, C++ when built as C++ (aka ENABLE_BUILD_WITH_CXX). then something like the following seems inevitable: #ifdef ENABLE_BUILD_WITH_CXX #ifdef __cplusplus extern void fancy_abort(const char *, int, const char *) ATTRIBUTE_NORETURN; #else extern void _Z11fancy_abortPKciS0_(const char *, int, const char *) ATTRIBUTE_NORETURN; #endif #else #ifdef __cplusplus extern "C" void fancy_abort(const char *, int, const char *) ATTRIBUTE_NORETURN; #else extern void fancy_abort(const char *, int, const char *) ATTRIBUTE_NORETURN; #endif #endif That's pretty nasty. But to avoid the nastiness one of (1) - (4) needs to be dropped. Which one? Ciao, Duncan.
Re: [Patch 4.6] In system.h, wrap include of C++ header in 'extern C++'
Hi Gabriel,
Richard just reminded me that we have two fancy_aborts.
Could you tell which one your code is indirectly using?
the one installed as plugin/include/system.h, which seems to be
gcc/include/system.h.
OK. I think that declaration has to have the C language spec.
Would you prepare a patch for that?
you mean: wrap the fancy_abort declaration in system.h in 'extern C'?
Sure, I will prepare a patch.
Best wishes, Duncan.
It is used for example in tree.h here:
/* Advance to the next argument. */
static inline void
function_args_iter_next (function_args_iterator *i)
{
gcc_assert (i->next != NULL_TREE);
i->next = TREE_CHAIN (i->next);
}
Best wishes, Duncan.
Re: [Patch 4.6] In system.h, wrap include of C++ header in 'extern C++'
Hi Gabriel,
Richard just reminded me that we have two fancy_aborts.
Could you tell which one your code is indirectly using?
the one installed as plugin/include/system.h, which seems to be
gcc/include/system.h. It is used for example in tree.h here:
/* Advance to the next argument. */
static inline void
function_args_iter_next (function_args_iterator *i)
{
gcc_assert (i->next != NULL_TREE);
i->next = TREE_CHAIN (i->next);
}
Best wishes, Duncan.
Re: [Patch 4.6] In system.h, wrap include of C++ header in 'extern C++'
Hi Gabriel, it defines fancy_abort. Not wrapping system.h in extern C results in undefined symbol: _Z11fancy_abortPKciS0_ when loading the plugin. If you want fancy_abort to have a C language specification, that is what you should declare as such. my code isn't using fancy_abort directly, it is including GCC headers that use gcc_assert (which turns into fancy_abort). Ciao, Duncan.
Re: [Patch 4.6] In system.h, wrap include of C++ header in 'extern C++'
Hi Richard, As system.h is supposed to only include system headers and do nothing else it has to be prepared to be included from C++ already, so no extern "C" wrapping should be necessary for it. it defines fancy_abort. Not wrapping system.h in extern C results in undefined symbol: _Z11fancy_abortPKciS0_ when loading the plugin. Ciao, Duncan.
Re: [Patch 4.6] In system.h, wrap include of C++ header in 'extern C++'
Hi Richard,
Uh, I don't think we should do that. Why do we include cstring here anyways?
Ian - you added this include in rev. 167764, I don't think that was "proper".
But I'm not sure wrapping a system.h include inside extern "C" from a C++
plugin is proper either ...
since the plugin needs to call GCC routines, and GCC is built as C, it has to
wrap at least some GCC headers in "extern C" to avoid mangling of the names
of those GCC routines (otherwise you can't load the plugin because the linker
will look for the mangled names in GCC and not find them). But perhaps you
know a trick to avoid the name mangling problem? It is true that maybe via
a careful dance it is possible to not wrap system.h in "extern C" - I will
give it a go.
Ciao, Duncan.
Thanks,
Richard.
Thanks, Duncan.
Index: gcc/system.h
===
--- gcc/system.h(revision 188518)
+++ gcc/system.h(working copy)
@@ -191,7 +191,9 @@
#endif
#ifdef __cplusplus
+extern "C++" {
# include
+}
#endif
/* Some of glibc's string inlines cause warnings. Plus we'd rather
[Patch 4.6] In system.h, wrap include of C++ header in 'extern C++'
My plugin is written in C++. When including headers from gcc-4.6 it wraps them
in 'extern "C"' to prevent name mangling. Some of the plugin headers include
gcc/system.h which includes the C++ header cstring if it detects the use of a
C++ compiler. As a result cstring routines included this way end up wrapped in
'extern C', while those included directly from C++ aren't 'extern C'. This
doesn't worry g++, but clang gets upset, erroring out with a complaint about
multiple inconsistent declarations of memchr and friends. Is the following
patch OK to apply to gcc-4.6? And is it in principle OK to apply to gcc-4.7
(I didn't test it there yet)? It would be useful if gcc-4.7 is compiled as
C.
Thanks, Duncan.
Index: gcc/system.h
===
--- gcc/system.h(revision 188518)
+++ gcc/system.h(working copy)
@@ -191,7 +191,9 @@
#endif
#ifdef __cplusplus
+extern "C++" {
# include
+}
#endif
/* Some of glibc's string inlines cause warnings. Plus we'd rather
Re: [google] Hide all uses of __float128 from Clang (issue6195066)
Hi Simon, Hide all uses of __float128 from Clang. Brackets _GLIBCXX_USE_FLOAT128 with #ifndef __clang__. Clang does not currently support the __float128 builtin, and so will fail to process libstdc++ headers that use it. if one day clang gets support for this type, won't this still turn everything off? Is it possible to test the compiler on some small program using __float128, and turn off use of __float128 if the compiler barfs? Ciao, Duncan.
Re: [Ada] Do not pass -Werror during linking
Hi Eric, Can you try to extract a testcase (assuming it's just a single case?). We shouldn't warn for layout-compatible types (but we may do so if for example struct nesting differs). It's more basic than that: for example, we map pointers on the C side to addresses (integers) on the Ada side. is having Address be an integer useful any more? Nowadays it should be possible to declare Address to be an access type with no associated storage pool. Even nicer might be to have it be turned into void* when lowering to GCC types. After all, Address is really used much like void*; see how GNAT declares malloc for example. Both of these possibilities would probably play better with GCC's middle end type system, which considers integers to be different to pointers. Ciao, Duncan. PS: I first thought of this when I noticed (when using the dragonegg plugin to compile Ada) that some of LLVM's optimizations bail out when they see integers being used where they expect a pointer. I tried tweaking the declaration of Address to be an access type as mentioned above, but unfortunately that crashes the compiler pretty quickly.
Re: [Ada] Entity list of for loop for enumeration with rep gets truncated
Hi Arnaud, --- exp_ch5.adb (revision 179894) +++ exp_ch5.adb (working copy) @@ -3458,6 +3458,20 @@ Statements => Statements (N, End_Label => End_Label (N))); + + -- The loop parameter's entity must be removed from the loop + -- scope's entity list, since itw will now be located in the typo: itw -> it Ciao, Duncan.
Re: [Ada] Speed up build of gnatools
Hi Arnaud, Now that gnatmake supports -j0, it's possible to speed up the build of gnattools during GNAT build by using gnatmake -j0 instead of gnatmake. This is useful since gnattools is the only target which isn't parallelized in the Makefile before this change. this means using as many processes as there are CPUs, right? It seems pretty dubious to me to use more processes than the user maybe asked for. For example I have to restrict the number of CPUs used when building GCC to less than I have since otherwise my machine overheats and turns itself off. Is there some way to get at the -j level the user passed to the top-level make and use that? Ciao, Duncan. Tested on x86_64-linux-gnu, committed on trunk. 2011-09-06 Arnaud Charlet * gcc-interface/Makefile.in (common-tools, gnatmake-re, gnatlink-re): Speed up by using -j0. -- Index: gcc-interface/Makefile.in === --- gcc-interface/Makefile.in (revision 178566) +++ gcc-interface/Makefile.in (working copy) @@ -2336,7 +2336,7 @@ endif common-tools: - $(GNATMAKE) -c -b $(ADA_INCLUDES) \ + $(GNATMAKE) -j0 -c -b $(ADA_INCLUDES) \ --GNATBIND="$(GNATBIND)" --GCC="$(CC) $(ALL_ADAFLAGS)" \ gnatchop gnatcmd gnatkr gnatls gnatprep gnatxref gnatfind gnatname \ gnatclean -bargs $(ADA_INCLUDES) $(GNATBIND_FLAGS) @@ -2375,16 +2375,18 @@ $(GNATLINK) -v vxaddr2line -o $@ --GCC="$(GCC_LINK)" targext.o $(CLIB) gnatmake-re: link.o targext.o - $(GNATMAKE) $(ADA_INCLUDES) -u sdefault --GCC="$(CC) $(MOST_ADA_FLAGS)" - $(GNATMAKE) -c $(ADA_INCLUDES) gnatmake --GCC="$(CC) $(ALL_ADAFLAGS)" + $(GNATMAKE) -j0 $(ADA_INCLUDES) -u sdefault --GCC="$(CC) $(MOST_ADA_FLAGS)" + $(GNATMAKE) -j0 -c $(ADA_INCLUDES) gnatmake --GCC="$(CC) $(ALL_ADAFLAGS)" $(GNATBIND) $(ADA_INCLUDES) $(GNATBIND_FLAGS) gnatmake $(GNATLINK) -v gnatmake -o ../../gnatmake$(exeext) \ --GCC="$(GCC_LINK)" $(TOOLS_LIBS) # Note the use of the "mv" command in order to allow gnatlink to be linked with # with the former version of gnatlink itself which cannot override itself. -gnatlink-re: link.o targext.o - $(GNATMAKE) -c $(ADA_INCLUDES) gnatlink --GCC="$(CC) $(ALL_ADAFLAGS)" +# gnatlink-re cannot be run at the same time as gnatmake-re, hence the +# dependency +gnatlink-re: link.o targext.o gnatmake-re + $(GNATMAKE) -j0 -c $(ADA_INCLUDES) gnatlink --GCC="$(CC) $(ALL_ADAFLAGS)" $(GNATBIND) $(ADA_INCLUDES) $(GNATBIND_FLAGS) gnatlink $(GNATLINK) -v gnatlink -o ../../gnatlinknew$(exeext) \ --GCC="$(GCC_LINK)" $(TOOLS_LIBS)
Re: Vector shuffling
Hi Artem,
On 31/08/11 10:27, Artem Shinkarov wrote:
On Wed, Aug 31, 2011 at 12:51 AM, Chris Lattner wrote:
On Aug 30, 2011, at 10:01 AM, Artem Shinkarov wrote:
The patch at the moment lacks of some examples, but mainly it works
fine for me. It would be nice if i386 gurus could look into the way I
am doing the expansion.
Middle-end parts seems to be more or less fine, they have not changed
much from the previous time.
+@code{__builtin_shuffle (vec, mask)} and
+@code{__builtin_shuffle (vec0, vec1, mask)}. Both functions construct
the latter would be __builtin_shuffle2.
Why??
That was the syntax we agreed on that elegantly handles both cases in one place.
If you're going to add vector shuffling builtins, you might consider adding the
same builtin that clang has for compatibility:
http://clang.llvm.org/docs/LanguageExtensions.html#__builtin_shufflevector
It should be straight-forward to map it into the same IR.
-Chris
Chris
I am trying to use OpenCL syntax here which says that the mask for
shuffling is a vector. Also I didn't really get from the clang
description if the indexes could be non-constnants? If not, then I
have a problem here, because I want to support this.
probably it maps directly to the LLVM shufflevector instruction, see
http://llvm.org/docs/LangRef.html#i_shufflevector
That requires the shuffle mask to be constant.
Ciao, Duncan.
Re: [Ada] Detect useless assignments to parts of objects
Hi Arnaud, this is a great feature. How does it handle unchecked unions? Will it warn if you write to a field but only read the value via a different field? Ciao, Duncan. GNAT did not issue a warning when assigning to a part of an object, and not referencing the object later on. Now it does so in some cases, similarly to the existing warnings on assignment to elementary objects. On the code below, GNAT now issues warnings: $ gcc -c -gnatwa assign.adb assign.adb:8:05: warning: useless assignment to "X", value never referenced assign.adb:9:04: warning: useless assignment to "Y", value never referenced --- procedure Assign is type T is record U : Integer; end record; X : T := T'(U => 0); Y : array (1..2) of Integer := (others => 0); begin X.U := X.U + 1; Y (2) := Y (1); end Assign; Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-04 Yannick Moy * checks.adb (Apply_Float_Conversion_Check): correct a typo where Lo_OK was used instead of Hi_OK, which could cause a read of an uninitialized value later on. Detected while working on the new warning. * exp_ch9.adb (Expand_N_Entry_Declaration): remove useless assignment to local variable. * sem_ch5.adb (Analyze_Assignment): set the last assignment component in more cases, in order to detect more unreferenced values. * sem_util.adb, sem_util.ads (Get_Enclosing_Object): return enclosing object for expression, if any.
Re: [Ada] Fix bugs with volatile and components of aggregate types
Hi Eric, This is the usual problem of volatile accesses not preserved under (heavy) optimization. In Ada, we can put pragma Volatile on components of composite types without putting it on the enclosing type itself, if T is a non-volatile composite type with volatile components, and O is an object of type T, are the optimizers allowed to remove the assignment "O := O"? Ciao, Duncan.
Re: Backport the fix for PR47714 to the 4.5 branch
On 31/05/11 08:39, Jakub Jelinek wrote: On Tue, May 31, 2011 at 08:35:35AM +0200, Duncan Sands wrote: The following patch backports the one-line fix for PR47714 from the 4.6 branch to the 4.5 branch. I hit this while working on the dragonegg plugin. OK to apply? Yes. Thanks. Applied as revision 174467. Ciao, Duncan.
Backport the fix for PR47714 to the 4.5 branch
The following patch backports the one-line fix for PR47714 from the 4.6 branch
to the 4.5 branch. I hit this while working on the dragonegg plugin. OK to
apply?
Ciao, Duncan.
Index: gcc/cp/method.c
===
--- gcc/cp/method.c (revision 173485)
+++ gcc/cp/method.c (working copy)
@@ -374,6 +374,7 @@
DECL_CONTEXT (x) = thunk_fndecl;
SET_DECL_RTL (x, NULL_RTX);
DECL_HAS_VALUE_EXPR_P (x) = 0;
+ TREE_ADDRESSABLE (x) = 0;
t = x;
}
a = nreverse (t);
Index: gcc/cp/ChangeLog
===
--- gcc/cp/ChangeLog(revision 173485)
+++ gcc/cp/ChangeLog(working copy)
@@ -1,3 +1,11 @@
+2011-05-31 Duncan Sands
+
+ Backported from 4.6 branch
+ 2011-03-09 Martin Jambor
+
+ PR tree-optimization/47714
+ * method.c (use_thunk): Clear addressable flag of thunk arguments.
+
2011-04-27 Jason Merrill
PR c++/48046
Index: gcc/testsuite/ChangeLog
===
--- gcc/testsuite/ChangeLog (revision 173485)
+++ gcc/testsuite/ChangeLog (working copy)
@@ -1,3 +1,11 @@
+2011-05-31 Duncan Sands
+
+ Backported from 4.6 branch
+ 2011-03-09 Martin Jambor
+
+ PR tree-optimization/47714
+ * g++.dg/torture/pr47714.C: New test.
+
2011-05-05 Jason Merrill
* g++.dg/init/new30.C: New.
Index: gcc/testsuite/g++.dg/torture/pr47714.C
===
--- gcc/testsuite/g++.dg/torture/pr47714.C (revision 0)
+++ gcc/testsuite/g++.dg/torture/pr47714.C (revision 0)
@@ -0,0 +1,16 @@
+struct A { virtual ~A () {} };
+struct B { virtual ~B () {} };
+struct C { virtual const A *foo (int) const = 0; };
+struct E : public B, public A { };
+struct F : public C
+{
+ virtual const E *foo (int) const;
+};
+void bar (int &);
+
+const E *
+F::foo (int x) const
+{
+ bar (x);
+ return __null;
+}
Re: [PATCH] Do not crash in array_type_nelts if TYPE_MIN_VALUE is null
On 09/04/11 17:45, Richard Guenther wrote: On Sat, Apr 9, 2011 at 1:22 PM, Duncan Sands wrote: I recently changed the dragonegg plugin to use array_type_nelts, and just got sent a Fortran testcase that shows that array_type_nelts can crash on array types coming from Fortran. The array type in question has TYPE_DOMAIN set, with TYPE_MIN_VALUE equal to 1 (because the array is indexed from 1) but no TYPE_MAX_VALUE (because the array length is not known). Here's a patch that fixes array_type_nelts. Unfortunately I don't have a testcase that shows the issue without the use of the dragonegg plugin. Tested by bootstrapping mainline and running the testsuite with gcc-4.5. OK to apply on mainline and the 4.5 and 4.6 branches? Ok. Thanks - applied (mainline commit 172227). Ciao, Duncan. Thanks, Richard. Ciao, Duncan. Index: gcc/tree.c === --- gcc/tree.c (revision 172166) +++ gcc/tree.c (working copy) @@ -2462,6 +2462,10 @@ min = TYPE_MIN_VALUE (index_type); max = TYPE_MAX_VALUE (index_type); + /* TYPE_MAX_VALUE may not be set if the array has unknown length. */ + if (!max) +return error_mark_node; + return (integer_zerop (min) ? max : fold_build2 (MINUS_EXPR, TREE_TYPE (max), max, min)); Index: gcc/ChangeLog === --- gcc/ChangeLog (revision 172166) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,7 @@ +2011-04-08 Duncan Sands + + * tree.c (array_type_nelts): Bail out if TYPE_MAX_VALUE not set. + 2011-04-08 Anatoly Sokolov * doc/tm.texi.in (ASM_OUTPUT_BSS): Remove documentation.
[PATCH] Do not crash in array_type_nelts if TYPE_MIN_VALUE is null
I recently changed the dragonegg plugin to use array_type_nelts, and just got sent a Fortran testcase that shows that array_type_nelts can crash on array types coming from Fortran. The array type in question has TYPE_DOMAIN set, with TYPE_MIN_VALUE equal to 1 (because the array is indexed from 1) but no TYPE_MAX_VALUE (because the array length is not known). Here's a patch that fixes array_type_nelts. Unfortunately I don't have a testcase that shows the issue without the use of the dragonegg plugin. Tested by bootstrapping mainline and running the testsuite with gcc-4.5. OK to apply on mainline and the 4.5 and 4.6 branches? Ciao, Duncan. Index: gcc/tree.c === --- gcc/tree.c (revision 172166) +++ gcc/tree.c (working copy) @@ -2462,6 +2462,10 @@ min = TYPE_MIN_VALUE (index_type); max = TYPE_MAX_VALUE (index_type); + /* TYPE_MAX_VALUE may not be set if the array has unknown length. */ + if (!max) +return error_mark_node; + return (integer_zerop (min) ? max : fold_build2 (MINUS_EXPR, TREE_TYPE (max), max, min)); Index: gcc/ChangeLog === --- gcc/ChangeLog (revision 172166) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,7 @@ +2011-04-08 Duncan Sands + + * tree.c (array_type_nelts): Bail out if TYPE_MAX_VALUE not set. + 2011-04-08 Anatoly Sokolov * doc/tm.texi.in (ASM_OUTPUT_BSS): Remove documentation.
Re: PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode.
Hi Basile, By the way, this patch is enough for the MELT plugin (which probably is the only plugin needing GTY). the dragonegg plugin also uses GTY, but only in a very mild way. Ciao, Duncan.
Re: [PATCH, Fortran] Correct declaration of frexp and friends
Hi Tobias, here's a gcc-4.5 patch which fixes: (1) the comment for fntype[2], (2) the prototypes for the frexp family and (3) the prototypes for the scalbn family. I checked all uses of all of the function types declared here and all the rest seem to be correct. I also took a look at an example using scalbn and it looks like calls to scalbn pass arguments in the right order (i.e. consistent with the fixed prototype). I'm running the testsuite now. OK to apply to the 4.5 branch if it passes testing? OK. (Contrary to the now fixed frexp, it seems to only affect < 4.6.) yes, scalbn was already fixed in gcc-4.6 and later. Thanks for checking and fixing these bugs! No problem, thanks for approving! Committed as revision 171979. Ciao, Duncan.
Re: [PATCH, Fortran] Correct declaration of frexp and friends
Hi Tobias, I do, so that's not a problem. By the way I just noticed that the arguments to the scalbn functions also seem to be the wrong way round: here's a gcc-4.5 patch which fixes: (1) the comment for fntype[2], (2) the prototypes for the frexp family and (3) the prototypes for the scalbn family. I checked all uses of all of the function types declared here and all the rest seem to be correct. I also took a look at an example using scalbn and it looks like calls to scalbn pass arguments in the right order (i.e. consistent with the fixed prototype). I'm running the testsuite now. OK to apply to the 4.5 branch if it passes testing? Index: f95-lang.c === --- f95-lang.c (revision 171972) +++ f95-lang.c (working copy) @@ -646,19 +646,20 @@ /* type (*) (type, type) */ tmp = tree_cons (NULL_TREE, type, tmp); fntype[1] = build_function_type (type, tmp); - /* type (*) (int, type) */ + /* type (*) (type, int) */ tmp = tree_cons (NULL_TREE, integer_type_node, void_list_node); tmp = tree_cons (NULL_TREE, type, tmp); fntype[2] = build_function_type (type, tmp); /* type (*) (void) */ fntype[3] = build_function_type (type, void_list_node); /* type (*) (type, &int) */ - tmp = tree_cons (NULL_TREE, type, void_list_node); - tmp = tree_cons (NULL_TREE, build_pointer_type (integer_type_node), tmp); + tmp = tree_cons (NULL_TREE, build_pointer_type (integer_type_node), + void_list_node); + tmp = tree_cons (NULL_TREE, type, tmp); fntype[4] = build_function_type (type, tmp); /* type (*) (type, int) */ - tmp = tree_cons (NULL_TREE, type, void_list_node); - tmp = tree_cons (NULL_TREE, integer_type_node, tmp); + tmp = tree_cons (NULL_TREE, integer_type_node, void_list_node); + tmp = tree_cons (NULL_TREE, type, tmp); fntype[5] = build_function_type (type, tmp); }
Re: [PATCH, Fortran] Correct declaration of frexp and friends
Hi Tobias,
Pong. It helps to send Fortran patches also to fortran@ ...
indeed :)
On 30/03/11 16:43, Duncan Sands wrote:
While working on the dragonegg plugin I noticed that the Fortran front-end
declares frexp with the parameters the wrong way round. Instead of
double frexp(double x, int *exp);
it is declared as
double frexp(int *exp, double x);
OK to apply on mainline and the 4.5 and 4.6 branches?
OK and thanks for the patch. Do you have an GCC SVN account?
I do, so that's not a problem. By the way I just noticed that the arguments to
the scalbn functions also seem to be the wrong way round:
gfc_define_builtin ("__builtin_scalbnl", mfunc_longdouble[5],
BUILT_IN_SCALBNL, "scalbnl",
ATTR_CONST_NOTHROW_LEAF_LIST);
gfc_define_builtin ("__builtin_scalbn", mfunc_double[5],
BUILT_IN_SCALBN, "scalbn", ATTR_CONST_NOTHROW_LEAF_LIST);
gfc_define_builtin ("__builtin_scalbnf", mfunc_float[5],
BUILT_IN_SCALBNF, "scalbnf",
ATTR_CONST_NOTHROW_LEAF_LIST);
but
/* type (*) (int, type) */
fntype[5] = build_function_type_list (type,
integer_type_node, type, NULL_TREE);
so it looks like you get scalbn(int, double) and not scalbn(double, int) etc.
If you agree that they are the wrong way round I will fix this too.
Ciao, Duncan.
Tobias
2011-03-30 Duncan Sands
* f95-lang.c (build_builtin_fntypes): Swap frexp parameter types.
- /* type (*) (&int, type) */
- fntype[4] = build_function_type_list (type,
+ /* type (*) (type, &int) */
+ fntype[4] = build_function_type_list (type, type,
build_pointer_type (integer_type_node),
- type,
NULL_TREE);
Re: [PATCH, Fortran] Correct declaration of frexp and friends
Ping? On 30/03/11 16:43, Duncan Sands wrote: While working on the dragonegg plugin I noticed that the Fortran front-end declares frexp with the parameters the wrong way round. Instead of double frexp(double x, int *exp); it is declared as double frexp(int *exp, double x); This is fairly harmless but might as well be fixed, so here is a patch (as far as I can see fntype[4] is only used in declaring the frexp family of functions). Bootstraps and has no impact on the Fortran testsuite (tested on mainline). OK to apply on mainline and the 4.5 and 4.6 branches? Proposed fortran/Changelog entry: 2011-03-30 Duncan Sands * f95-lang.c (build_builtin_fntypes): Swap frexp parameter types. Index: gcc/fortran/f95-lang.c === --- gcc/fortran/f95-lang.c (revision 171716) +++ gcc/fortran/f95-lang.c (working copy) @@ -695,10 +695,9 @@ type, integer_type_node, NULL_TREE); /* type (*) (void) */ fntype[3] = build_function_type_list (type, NULL_TREE); - /* type (*) (&int, type) */ - fntype[4] = build_function_type_list (type, + /* type (*) (type, &int) */ + fntype[4] = build_function_type_list (type, type, build_pointer_type (integer_type_node), - type, NULL_TREE); /* type (*) (int, type) */ fntype[5] = build_function_type_list (type,
[PATCH, Fortran] Correct declaration of frexp and friends
While working on the dragonegg plugin I noticed that the Fortran front-end declares frexp with the parameters the wrong way round. Instead of double frexp(double x, int *exp); it is declared as double frexp(int *exp, double x); This is fairly harmless but might as well be fixed, so here is a patch (as far as I can see fntype[4] is only used in declaring the frexp family of functions). Bootstraps and has no impact on the Fortran testsuite (tested on mainline). OK to apply on mainline and the 4.5 and 4.6 branches? Proposed fortran/Changelog entry: 2011-03-30 Duncan Sands * f95-lang.c (build_builtin_fntypes): Swap frexp parameter types. Index: gcc/fortran/f95-lang.c === --- gcc/fortran/f95-lang.c (revision 171716) +++ gcc/fortran/f95-lang.c (working copy) @@ -695,10 +695,9 @@ type, integer_type_node, NULL_TREE); /* type (*) (void) */ fntype[3] = build_function_type_list (type, NULL_TREE); - /* type (*) (&int, type) */ - fntype[4] = build_function_type_list (type, + /* type (*) (type, &int) */ + fntype[4] = build_function_type_list (type, type, build_pointer_type (integer_type_node), -type, NULL_TREE); /* type (*) (int, type) */ fntype[5] = build_function_type_list (type,
Re: [Ada] Implement switches to reorder record components in gigi
Hi Eric, Not clear what kind of answer you're expecting. To cancel the previous define? I somehow failed to see the define. Sorry for the noise. Ciao, Duncan.
Re: [Ada] Implement switches to reorder record components in gigi
Hi Eric, this looks like a nice improvement. I noticed this mysterious undef though - what is it for? +#undef MOVE_FROM_FIELD_LIST_TO Ciao, Duncan.
