Re: [Patch, Fortran, F03] PR 80046: Explicit interface required: pointer argument
Hi Janus, The patch is OK for trunk. Thanks Paul On 7 April 2017 at 17:51, Janus Weil wrote: > ping! > > 2017-03-29 22:25 GMT+02:00 Janus Weil : >> Hi all, >> >> here is a patch that enhances the diagnostics for procedure-pointer >> assignments, so that procedure-pointer components that need an >> explicit interface are correctly rejected. >> >> Regtests cleanly on x86_64-linux-gnu. Ok for trunk? >> >> Cheers, >> Janus >> >> >> 2017-03-29 Janus Weil >> >> PR fortran/80046 >> * expr.c (gfc_check_pointer_assign): Check if procedure pointer >> components in a pointer assignment need an explicit interface. >> >> 2017-03-29 Janus Weil >> >> PR fortran/80046 >> * gfortran.dg/proc_ptr_comp_48.f90: New test case. -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [Patch, fortran] PR34360 (Comment 28) - ICE when assigning item of a derived-component to a pointer
Dear Paul, Your patch fixes the tests in pr34640 comments 20 and 28 (I didn’t test the variants in comment 27) and in pr57733. The tests in pr34640 in comments 0, 3, and 5, as well in all the other duplicates still fail. Thanks for working on the issue, Dominique
Re: [Patch, fortran] PR34360 (Comment 28) - ICE when assigning item of a derived-component to a pointer
The original test in pr51218 is also miscomputed with the patch: Before t: Program received signal SIGSEGV: Segmentation fault - invalid memory reference. Dominique > Le 9 avr. 2017 à 16:41, Dominique d'Humières a écrit : > > Dear Paul, > > Your patch fixes the tests in pr34640 comments 20 and 28 (I didn’t test the > variants in comment 27) and in pr57733. > The tests in pr34640 in comments 0, 3, and 5, as well in all the other > duplicates still fail. > > Thanks for working on the issue, > > Dominique >
Re: [hsa] Integrate into the existing accelerator framework, libgomp plugin
On Wed, Apr 05, 2017 at 09:36:44AM +0200, Thomas Schwinge wrote: > Hi! > > On Tue, 9 Jun 2015 16:43:57 +0200, Martin Jambor wrote: > > [hsa libgomp plugin] > > > I'm looking forward to any comments and suggestions, meanwhile I have > > committed the patch to the branch as r224284. > > Commenting better late than never? ;-) > > Is there a specific reason why you're not using the standard > GOMP_PLUGIN_debug interface guarded by the GOMP_DEBUG environment > variable, and instead essentially re-implement that functionality guarded > by a new HSA_DEBUG environment variable? (I might be talked into > creating the obvious patch.) No, no reason at all, I did not now about GOMP_PLUGIN_debug. You're welcome to create the patch (or I will do that after I wade through my email and other backlogs). Thanks for pointing it out, Martin
Re: [HSA, PATCH] Load an HSA runtime via dlopen mechanism
Hi, On Wed, Apr 05, 2017 at 10:11:34AM +0200, Thomas Schwinge wrote: > ... > > I ran into a case where the libgomp hsa plugin wouldn't load. The > following patch helped me to quickly diagnose and then fix that. OK for > trunk? Yes, thanks. Martin > > commit 54099202eb88464530dd3a55709c9afb85766ee0 > Author: Thomas Schwinge > Date: Wed Apr 5 09:58:02 2017 +0200 > > libgomp hsa plugin: debug output for HSA runtime library loading failure > > libgomp/ > * plugin/plugin-hsa.c (DLSYM_FN, init_hsa_runtime_functions): > Debug output for failure. > --- > libgomp/plugin/plugin-hsa.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git libgomp/plugin/plugin-hsa.c libgomp/plugin/plugin-hsa.c > index 9cc243d..90ca247 100644 > --- libgomp/plugin/plugin-hsa.c > +++ libgomp/plugin/plugin-hsa.c > @@ -491,14 +491,14 @@ static struct hsa_context_info hsa_context; > #define DLSYM_FN(function) \ >hsa_fns.function##_fn = dlsym (handle, #function); \ >if (hsa_fns.function##_fn == NULL) \ > -return false; > +goto dl_fail; > > static bool > init_hsa_runtime_functions (void) > { >void *handle = dlopen (hsa_runtime_lib, RTLD_LAZY); >if (handle == NULL) > -return false; > +goto dl_fail; > >DLSYM_FN (hsa_status_string) >DLSYM_FN (hsa_agent_get_info) > @@ -530,6 +530,10 @@ init_hsa_runtime_functions (void) >DLSYM_FN (hsa_ext_program_destroy) >DLSYM_FN (hsa_ext_program_finalize) >return true; > + > + dl_fail: > + HSA_DEBUG ("while loading %s: %s\n", hsa_runtime_lib, dlerror ()); > + return false; > } > > /* Find kernel for an AGENT by name provided in KERNEL_NAME. */ > > > Grüße > Thomas
Re: [C++ PATCH] New warning for extra semicolons after in-class function definitions
On 7 Apr, David Malcolm wrote: > On Fri, 2017-04-07 at 21:55 +0200, Volker Reichelt wrote: >> Hi, >> >> with the following patch I suggest to add a diagnostic for extra >> semicolons after in-class member function definitions: >> >> struct A >> { >> A() {}; >> void foo() {}; >> friend void bar() {}; >> }; >> >> Although they are allowed in the C++ standard, people (including me) >> often like to get rid of them for stylistic/consistency reasons. >> In fact clang has a warning -Wextra-semi for this. >> >> Also in GCC (almost exactly 10 years ago) there was a patch >> https://gcc.gnu.org/ml/gcc-cvs/2007-03/msg00841.html >> to issue a pedwarn (which had to be reverted as GCC would reject >> valid >> code because of the pedwarn). >> >> Instead of using pewarn the patch below adds a new warning (named >> like >> clang's) to warn about these redundant semicolons. >> Btw, clang's warning message "extra ';' after member function >> definition" >> is slightly incorrect because it is also emitted for friend functions >> which are not member-functions. That's why I suggest a different >> wording: >> >> Wextra-semi.C:3:9: warning: extra ';' after in-class function >> definition [-Wextra-semi] >> A() {}; >>^ >> Wextra-semi.C:4:16: warning: extra ';' after in-class function >> definition [-Wextra-semi] >> void foo() {}; >> ^ >> Wextra-semi.C:5:23: warning: extra ';' after in-class function >> definition [-Wextra-semi] >> friend void bar() {}; >> ^ >> >> Bootstrapped and regtested on x86_64-pc-linux-gnu. >> >> OK for stage 1, once GCC 7 has branched? >> Regards, >> Volker >> >> >> 2017-04-07 Volker Reichelt >> >> * c.opt (Wextra-semi): New C++ warning flag. >> >> Index: gcc/c-family/c.opt >> === >> --- gcc/c-family/c.opt (revision 246752) >> +++ gcc/c-family/c.opt (working copy) >> @@ -504,6 +504,10 @@ >> C ObjC C++ ObjC++ Warning >> ; in common.opt >> >> +Wextra-semi >> +C++ Var(warn_extra_semi) Warning >> +Warn about semicolon after in-class function definition. >> + >> Wfloat-conversion >> C ObjC C++ ObjC++ Var(warn_float_conversion) Warning LangEnabledBy(C >> ObjC C++ ObjC++,Wconversion) >> Warn for implicit type conversions that cause loss of floating point >> precision. >> >> 2017-04-07 Volker Reichelt >> >> * parser.c (cp_parser_member_declaration): Add warning for >> extra semicolon after in-class function definition. >> >> Index: gcc/cp/parser.c >> === >> --- gcc/cp/parser.c (revision 246752) >> +++ gcc/cp/parser.c (working copy) >> @@ -23386,7 +23386,11 @@ >> token = cp_lexer_peek_token (parser->lexer); >> /* If the next token is a semicolon, consume it. >> */ >> if (token->type == CPP_SEMICOLON) >> - cp_lexer_consume_token (parser->lexer); >> + { >> + cp_lexer_consume_token (parser->lexer); >> + warning (OPT_Wextra_semi, "extra %<;%> " >> + "after in-class function definition"); >> + } > > Thanks for posting this. > > I'm not a C++ maintainer, but I like the idea (though the patch is > missing at least a doc/invoke.texi change). You're right. I had the nagging feeling that I forgot something. I added a respective section to doc/invoke.texi after -Wempty-body and -Wenum-compare. > A small improvement to this would be to emit a deletion fix-it hint > about the redundant token (so that IDEs have a change of fixing it > easily). > > This could be done something like this: > > location_t semicolon_loc >= cp_lexer_consume_token (parser->lexer)->location; > gcc_rich_location richloc (semicolon_loc); > richloc.add_fixit_remove (); > warning_at_richloc (&richloc, OPT_Wextra_semi, > "extra %<;%> after in-class function > definition"); > That's a nice suggestion. I modified the patch accordingly. >> goto out; >> } >> else >> >> 2017-04-07 Volker Reichelt >> >> * g++.dg/warn/Wextra-semi.C: New test. >> >> Index: gcc/testsuite/g++.dg/warn/Wextra-semi.C >> === >> --- gcc/testsuite/g++.dg/warn/Wextra-semi.C 2017-04-07 >> +++ gcc/testsuite/g++.dg/warn/Wextra-semi.C 2017-04-07 >> @@ -0,0 +1,8 @@ >> +// { dg-options "-Wextra-semi" } >> + >> +struct A >> +{ >> + A() {}; // { dg-warning "after in-class function >> definition" } >> + void foo() {}; // { dg-warning "after in-class function >> definition" } >> + friend void bar() {};// { dg-warning "after in-class >> function definition" } >> +}; >> > > If you implement the fix-it idea, then you can add > -
[PATCH] avoid using types wider than int for width and precision (PR 80364)
Bug 80364 - sanitizer detects signed integer overflow in gimple-ssa-sprintf.c, points out a runtime error (integer overflow when negating LONG_MIN) triggered by the undefined saniziter for an invalid test case recently added to the test suite. The test case passes an argument of type long as the width and precision specified by the asterisk where the argument is required to be an int. The attached fix avoids this error by using the expected type (int) rather than the type of actual argument. This is a 7.0 P1 regression so I'm looking for approval to commit the patch to the trunk. Martin PR middle-end/80364 - sanitizer detects signed integer overflow in gimple-ssa-sprintf.c gcc/ChangeLog: PR middle-end/80364 * gimple-ssa-sprintf.c (get_int_range): Use the specified type when recursing, not that of the argument. gcc/testsuite/ChangeLog: PR middle-end/80364 * gcc.dg/tree-ssa/builtin-sprintf-warn-16.c: New test. diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c index 2474391..fbcf5c7 100644 --- a/gcc/gimple-ssa-sprintf.c +++ b/gcc/gimple-ssa-sprintf.c @@ -961,10 +961,10 @@ get_int_range (tree arg, tree type, HOST_WIDE_INT *pmin, HOST_WIDE_INT *pmax, /* True if the argument's range cannot be determined. */ bool unknown = true; - type = TREE_TYPE (arg); + tree argtype = TREE_TYPE (arg); if (TREE_CODE (arg) == SSA_NAME - && TREE_CODE (type) == INTEGER_TYPE) + && TREE_CODE (argtype) == INTEGER_TYPE) { /* Try to determine the range of values of the integer argument. */ wide_int min, max; @@ -972,11 +972,11 @@ get_int_range (tree arg, tree type, HOST_WIDE_INT *pmin, HOST_WIDE_INT *pmax, if (range_type == VR_RANGE) { HOST_WIDE_INT type_min - = (TYPE_UNSIGNED (type) - ? tree_to_uhwi (TYPE_MIN_VALUE (type)) - : tree_to_shwi (TYPE_MIN_VALUE (type))); + = (TYPE_UNSIGNED (argtype) + ? tree_to_uhwi (TYPE_MIN_VALUE (argtype)) + : tree_to_shwi (TYPE_MIN_VALUE (argtype))); - HOST_WIDE_INT type_max = tree_to_uhwi (TYPE_MAX_VALUE (type)); + HOST_WIDE_INT type_max = tree_to_uhwi (TYPE_MAX_VALUE (argtype)); *pmin = min.to_shwi (); *pmax = max.to_shwi (); @@ -1004,6 +1004,9 @@ get_int_range (tree arg, tree type, HOST_WIDE_INT *pmin, HOST_WIDE_INT *pmax, *pmin = *pmax = -*pmin; else { + /* Make sure signed overlow is avoided. */ + gcc_assert (*pmin != HOST_WIDE_INT_MIN); + HOST_WIDE_INT tmp = -*pmin; *pmin = 0; if (*pmax < tmp) diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-16.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-16.c new file mode 100644 index 000..92112b5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-16.c @@ -0,0 +1,166 @@ +/* PR middle-end/80364 - sanitizer detects signed integer overflow + in gimple-ssa-sprintf.c + { dg-do compile } + { dg-options "-O2 -Wall -Wformat-overflow=1 -ftrack-macro-expansion=0" } + { dg-require-effective-target int32plus } */ + +typedef __SIZE_TYPE__ size_t; +typedef __WCHAR_TYPE__ wchar_t; + +void sink (void*); +void* get_value (void); + +/* Return a random width as type T. */ +#define W(T) *(T*)get_value () + +/* Return a random precision as type T. */ +#define P(T) *(T*)get_value () + +/* Return a random value as type T. */ +#define V(T) *(T*)get_value () + +extern char buf[1]; + +/* Test convenience macro. */ +#define T(fmt, ...) \ + __builtin_sprintf (buf + 1, fmt, __VA_ARGS__); \ + sink (buf) + +typedef signed char schar_t; +typedef unsigned char uchar_t; +typedef signed shortsshort_t; +typedef unsigned short ushort_t; +typedef signed int sint_t; +typedef unsigned intuint_t; +typedef signed long slong_t; +typedef unsigned long ulong_t; +typedef signed long longsllong_t; +typedef unsigned long long ullong_t; + +#if __SIZEOF_INT128__ +typedef __int128_t sint128_t; +typedef __uint128_t uint128_t; +#else +/* When __int128_t is not available, repeat the same tests with long long. + This is to avoid having to guard the tests below and to avoid making + the dg-warning directives conditional. */ +typedef signed long longsint128_t; +typedef unsigned long long uint128_t; +#endif + +void test_width_cst (void) +{ + T ("%*i", W (schar_t), 1); /* { dg-warning "between 1 and 128 " } */ + T ("%*i", W (uchar_t), 12);/* { dg-warning "between 2 and 255 " } */ + + T ("%*i", W (sshort_t), 123); /* { dg-warning "between 3 and 32768 " } */ + T ("%*i", W (ushort_t), 1234); /* { dg-warning "between 4 and 65535 " } */ + + T ("%*i", W (sint_t), 12345); /* { dg-warning "between 5 and 2147483648" } */ + T ("%*i", W (uint_t), 123456); /* { dg-warning "between 6 and 2147483648" } */ + + /* Exercise calls with invalid arguments (to verify there is no ICE). */ + T ("%*li", W (slong_t), 1234567L); /* { dg-warning "between 7 and 2147483648" }
Re: [wwwdocs] Document C++ null pointer constant changes in gcc-7/porting_to.html
On 04/07/2017 03:25 AM, Jonathan Wakely wrote: This issue caused a lot of build failures during the GCC mass rebuilds for Fedora, but isn't in the porting to guide yet. Is this accurate and clear enough for casual readers? Index: htdocs/gcc-7/porting_to.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-7/porting_to.html,v retrieving revision 1.13 diff -u -r1.13 porting_to.html --- htdocs/gcc-7/porting_to.html6 Apr 2017 17:12:16 - 1.13 +++ htdocs/gcc-7/porting_to.html7 Apr 2017 09:25:06 - @@ -118,6 +118,39 @@ with GCC 7 and some are compiled with older releases. +Null pointer constants + + +When compiling as C++11 or later, GCC 7 follows the revised definition of a +null pointer constant. This means conversions to pointers from other +types of constant (such as character literals and boolean literals) will now +be rejected. + Nit pick: s/will now be rejected/are now rejected/ since "now" means we are describing GCC's current behavior, not future behavior. :-) -Sandra
Re: [PATCH] Add _mm512_{,mask_}reduce_*_* intrinsics (PR target/80324)
Hi Jakib, On 07 Apr 16:52, Jakub Jelinek wrote: > Hi! > > This patch is slightly larger, so I haven't included it in the patch I've > sent a few minutes ago. > > I've looked at godbolt for what ICC generates for these and picked sequences > that generate approx. as good code as that. For > min_epi64/max_epi64/min_epu64/max_epu64 there is a slight complication that > in AVX512F there is only _mm512_{min,max}_ep{i,u}64 but not the _mm256_ or > _mm_ ones, so we need to perform 512-bit operations all the time rather than > perform extractions, 256-bit operation, further extractions and then 128-bit > operations. > > Seems we need to teach our permutation code further instructions, e.g. > typedef long long V __attribute__((vector_size (64))); > typedef int W __attribute__((vector_size (64))); > W f0 (W x) { > return __builtin_shuffle (x, (W) { 8, 9, 10, 11, 12, 13, 14, 15, 0, 1, 2, > 3, 4, 5, 6, 7 }); > } > V f1 (V x) { > return __builtin_shuffle (x, (V) { 4, 5, 6, 7, 0, 1, 2, 3 }); > } > generate unnecessarily bad code (could use vpshufi64x2 instruction), > guess that can be resolved for GCC8. > > Tested with > make -j272 -k check-gcc RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} > i386.exp' > on KNL, will bootstrap/regtest on my Haswell-E next, ok for trunk > if that passes? Patch is OK for trunk, thanks for implementing those intrinsics! -- K
Re: [PATCH] avoid using types wider than int for width and precision (PR 80364)
On Sun, Apr 09, 2017 at 05:06:58PM -0600, Martin Sebor wrote: > PR middle-end/80364 - sanitizer detects signed integer overflow in > gimple-ssa-sprintf.c > > gcc/ChangeLog: > > PR middle-end/80364 > * gimple-ssa-sprintf.c (get_int_range): Use the specified type when > recursing, not that of the argument. > > gcc/testsuite/ChangeLog: > > PR middle-end/80364 > * gcc.dg/tree-ssa/builtin-sprintf-warn-16.c: New test. > > diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c > index 2474391..fbcf5c7 100644 > --- a/gcc/gimple-ssa-sprintf.c > +++ b/gcc/gimple-ssa-sprintf.c > @@ -961,10 +961,10 @@ get_int_range (tree arg, tree type, HOST_WIDE_INT > *pmin, HOST_WIDE_INT *pmax, >/* True if the argument's range cannot be determined. */ >bool unknown = true; > > - type = TREE_TYPE (arg); > + tree argtype = TREE_TYPE (arg); > >if (TREE_CODE (arg) == SSA_NAME > - && TREE_CODE (type) == INTEGER_TYPE) > + && TREE_CODE (argtype) == INTEGER_TYPE) > { > /* Try to determine the range of values of the integer argument. */ > wide_int min, max; I see that as a step in the right direction, but it doesn't seem to be complete fix. > @@ -972,11 +972,11 @@ get_int_range (tree arg, tree type, HOST_WIDE_INT > *pmin, HOST_WIDE_INT *pmax, > if (range_type == VR_RANGE) > { > HOST_WIDE_INT type_min > - = (TYPE_UNSIGNED (type) > -? tree_to_uhwi (TYPE_MIN_VALUE (type)) > -: tree_to_shwi (TYPE_MIN_VALUE (type))); > + = (TYPE_UNSIGNED (argtype) > +? tree_to_uhwi (TYPE_MIN_VALUE (argtype)) > +: tree_to_shwi (TYPE_MIN_VALUE (argtype))); > > - HOST_WIDE_INT type_max = tree_to_uhwi (TYPE_MAX_VALUE (type)); > + HOST_WIDE_INT type_max = tree_to_uhwi (TYPE_MAX_VALUE (argtype)); For the start, consider what happens if argtype is __int128_t or __uint128_t or unsigned int here. For the first two, those tree_to_uhwi or tree_to_shwi will just ICE above (if you have VR_RANGE for those, shouldn't be hard to write testcase). So likely you need to ignore range info for invalid args with precision greater than that of integer_type_node (i.e. INT_TYPE_SIZE). unsigned int is I think not UB when passed to var-args and read as int, it is just implementation defined how is it converted into int (I could be wrong). In that case (i.e. TYPE_UNSIGNED and precision equal to INT_TYPE_SIZE) I think you need to either handle it like you do only if the max is smaller or equal than INT_MAX and punt to full int range otherwise, or need to take the unsigned -> int conversion into account. I think passing unsigned int arg bigger than __INT_MAX__ is not well defined, because that means negative precision in the end. And, if you never change type, so type == integer_type_node, you might consider not passing that argument at all, the behavior is hardcoded for that type anyway (assumption that it fits into shwi or uhwi etc.). Perhaps in addition to the type_min/type_max check you do to determine knownrange follow it by similar range check for integer_type_node range and don't set unknown to false if it is not within integer_type_node range? The TYPE_PRECISION <= INT_TYPE_SIZE check will be still needed not to trigger the ICEs. Jakub