C++ PATCH to reduced_constant_expression for partially-initialized objects
A few months back I queued this patch to bring back for GCC 8. Unfortunately I don't remember the context that it came up in, but it affects for instance cases of self-assignment, which can't have a constant value if there is no previous initialization. Tested x86_64-pc-linux-gnu, applying to trunk. commit 8c5592e0e31ec60bb6b075fca14face587e3084b Author: Jason Merrill Date: Tue Apr 11 16:29:37 2017 -0400 * constexpr.c (reduced_constant_expression_p): If CONSTRUCTOR_NO_IMPLICIT_ZERO, check that all fields are initialized. diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index a5692fb..2d2f3b8 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -1732,15 +1732,30 @@ reduced_constant_expression_p (tree t) case CONSTRUCTOR: /* And we need to handle PTRMEM_CST wrapped in a CONSTRUCTOR. */ - tree elt; unsigned HOST_WIDE_INT idx; - FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (t), idx, elt) + tree idx, val, field; unsigned HOST_WIDE_INT i; + if (CONSTRUCTOR_NO_IMPLICIT_ZERO (t)) + field = next_initializable_field (TYPE_FIELDS (TREE_TYPE (t))); + else + field = NULL_TREE; + FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (t), i, idx, val) { - if (!elt) + if (!val) /* We're in the middle of initializing this element. */ return false; - if (!reduced_constant_expression_p (elt)) + if (!reduced_constant_expression_p (val)) return false; + if (field) + { + if (idx != field) + return false; + field = next_initializable_field (DECL_CHAIN (field)); + } } + if (field) + return false; + else if (CONSTRUCTOR_NO_IMPLICIT_ZERO (t)) + /* All the fields are initialized. */ + CONSTRUCTOR_NO_IMPLICIT_ZERO (t) = false; return true; default:
Re: [PATCH] Remove output_die_symbol
On Tue, Aug 22, 2017 at 1:13 PM, Richard Biener wrote: > > This addresses one postponed feedback from the early LTO debug time > by simply removing output_die_symbol from output_die as we have no > ! die->comdat_type_p symbols left apart from the CU one produced by > LTO debug which I added the guard for. > > Bootstrapped and tested on x86_64-unknown-linux-gnu, ok? OK.
Re: [C++ Patch] PR 70621 ("[6/7/8 Regression] ICE on invalid code at -O1 and above on x86_64-linux-gnu in record_reference, at cgraphbuild.c:64")
OK. On Tue, Aug 29, 2017 at 11:33 AM, Paolo Carlini wrote: > Hi, > > in this error recovery regression, we ICE only when optimizing, while > building the cgraph. Avoiding the reported problem seems easy, just check > the return value of duplicate_decls in start_decl and immediately return > back error_mark_node. However, while working on the issue, I noticed > something slightly more interesting, IMO: we have, after the relevant > duplicate_decls call: > > - if (decl_spec_seq_has_spec_p (declspecs, ds_constexpr) > - && !DECL_DECLARED_CONSTEXPR_P (field)) > -error ("%qD declared % outside its class", > field); > > which I propose to remove. In fact - something I didn't really know - for > well formed user code, duplicate_decls, near the end, does some memcpys > which mean that its second argument (would be 'field' in the start_decl > section we are looking at) is adjusted to have a DECL_DECLARED_CONSTEXPR_P > consistent with its first argument. That's of course because we want to > accept snippets like: > > struct A > { > static const int x; > }; > > constexpr int A::x = 0; > > In turn that means the error above is issued only when something went wrong > in duplicate_decls in the first place. Thus the above error seems at least > verbose. However, here in start_decl we are only handling VAR_P (decl), thus > I don't think the message above even makes sense and could be misleading, > given snippets like the above. Therefore, I propose to remove the diagnostic > entirely, which overall also simplifies a patch dealing with c++/70621. The > below passes testing as-is on x86_64-linux. > > Thanks, Paolo. > > >
[patch, fortran, RFC] warn about out-of-bounds errors in DO loops
Hello world, the attached patch warns about certain cases where out-of-bound array accesses can be detected at compile time. This was inspired by an out-of-bound access in Polyhedron. A preliminary version of this patch has already found one error in the testsuite. The problem is what to warn for. Cases like real, dimension(10) :: a do i=1,11 if (somecondition) a(i) = 42. end do could be valid if somecondition is false for i=11. What I did was to check if the subscript reference was - warn for all cases with the new option -Wdo-subscript-extra, included in -Wextra - not warn if an expression is found in an if or select case statement inside the do loop for -Wdo-subscript, included in -Wall. The patch also checks for slightly complicated expressions like i*i - the only condition is that it should evaluate to a constant if the loop variable is inserted. Only constant bounds are checked. See the test cases for some more details. So, what do you think, especially about the choice of options and warning levels? Regards Thomas Index: frontend-passes.c === --- frontend-passes.c (Revision 251375) +++ frontend-passes.c (Arbeitskopie) @@ -39,6 +39,8 @@ static void optimize_minmaxloc (gfc_expr **); static bool is_empty_string (gfc_expr *e); static void doloop_warn (gfc_namespace *); +static int do_intent (gfc_expr **); +static int do_subscript (gfc_expr **); static void optimize_reduction (gfc_namespace *); static int callback_reduction (gfc_expr **, int *, void *); static void realloc_strings (gfc_namespace *); @@ -98,10 +100,19 @@ /* Keep track of DO loop levels. */ -static vec doloop_list; +typedef struct { + gfc_code *c; + int branch_level; +} do_t; +static vec doloop_list; static int doloop_level; +/* Keep track of if and select case levels. */ + +static int if_level; +static int select_level; + /* Vector of gfc_expr * to keep track of DO loops. */ struct my_struct *evec; @@ -133,6 +144,8 @@ change. */ doloop_level = 0; + if_level = 0; + select_level = 0; doloop_warn (ns); doloop_list.release (); int w, e; @@ -2231,6 +2244,7 @@ gfc_formal_arglist *f; gfc_actual_arglist *a; gfc_code *cl; + do_t loop, *lp; co = *c; @@ -2244,9 +2258,12 @@ case EXEC_DO: if (co->ext.iterator && co->ext.iterator->var) - doloop_list.safe_push (co); + loop.c = co; else - doloop_list.safe_push ((gfc_code *) NULL); + loop.c = NULL; + + loop.branch_level = if_level + select_level; + doloop_list.safe_push (loop); break; case EXEC_CALL: @@ -2265,9 +2282,10 @@ while (a && f) { - FOR_EACH_VEC_ELT (doloop_list, i, cl) + FOR_EACH_VEC_ELT (doloop_list, i, lp) { gfc_symbol *do_sym; + cl = lp->c; if (cl == NULL) break; @@ -2282,14 +2300,14 @@ "value inside loop beginning at %L as " "INTENT(OUT) argument to subroutine %qs", do_sym->name, &a->expr->where, - &doloop_list[i]->loc, + &(doloop_list[i].c->loc), co->symtree->n.sym->name); else if (f->sym->attr.intent == INTENT_INOUT) gfc_error_now ("Variable %qs at %L not definable inside " "loop beginning at %L as INTENT(INOUT) " "argument to subroutine %qs", do_sym->name, &a->expr->where, - &doloop_list[i]->loc, + &(doloop_list[i].c->loc), co->symtree->n.sym->name); } } @@ -2304,17 +2322,268 @@ return 0; } -/* Callback function for functions checking that we do not pass a DO variable - to an INTENT(OUT) or INTENT(INOUT) dummy variable. */ +/* Callback function to warn about different things within DO loops. */ static int do_function (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED, void *data ATTRIBUTE_UNUSED) { + + int errors; + + if (doloop_list.length () == 0) +return 0; + + if ((*e)->expr_type == EXPR_FUNCTION) +do_intent (e); + +#if 0 + printf("warn_do_subscript = %d, warn_do_subscript_extra = %d" + "cond = %d\n", + warn_do_subscript, warn_do_subscript_extra, + !(warn_do_subscript || warn_do_subscript_extra)); +#endif + if (!(warn_do_subscript || warn_do_subscript_extra)) +return 0; + + gfc_get_errors (NULL, &errors); + if (errors) +return 0; + + if ((*e)->expr_type == EXPR_VARIABLE) +do_subscript (e); + + return 0; +} + +typedef struct +{ + gfc_symbol *sym; + mpz_t val; +} insert_index_t; + +/* Callback function - if the expression is the variable in data->sym, + replace it with a constant from data->val. */ + +static int +callback_insert_index (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED, + void *data) +{ + insert_index_t *d; + gfc_expr *ex, *n; + + ex = (*e); + if (ex->expr_type != EXPR_VARIABLE) +return 0; + + d = (insert_index_t *) data; + if (ex->symtree->n.sym != d->sym) +return 0; + + n = gfc_get_constant_expr (BT_INTEGER, ex->ts.kind, &ex->w
Re: [PATCH][PR sanitizer/77631] Support separate debug info in libbacktrace
On Sat, Jul 29, 2017 at 1:42 PM, Denis Khalikov wrote: > > Hello Ian, > thanks for review. > I've updated the patch, can you please take a look. Apologies again for the length of time it took to reply. I've had a hard time understanding the patch. It's quite likely that I don't understand how it works, but it seems to pass the same file descriptor to process_elf_header twice. It seems to look for debug files with the buildid in places where they will not be found. It seems to work out the file name a second time, even though the file name must already be known. I eventually just wrote my own implementation. Could you try this patch and see if it works for your cases? The patch is against current mainline. Thanks. Ian Index: Makefile.am === --- Makefile.am (revision 251948) +++ Makefile.am (working copy) @@ -122,6 +122,16 @@ ttest_LDADD = libbacktrace.la endif HAVE_PTHREAD +if HAVE_OBJCOPY_DEBUGLINK + +TESTS += dtest + +dtest: btest + $(OBJCOPY) --only-keep-debug btest btest.debug + $(OBJCOPY) --strip-debug --add-gnu-debuglink=btest.debug btest dtest + +endif HAVE_OBJCOPY_DEBUGLINK + endif NATIVE # We can't use automake's automatic dependency tracking, because it Index: Makefile.in === --- Makefile.in (revision 251948) +++ Makefile.in (working copy) @@ -86,6 +86,7 @@ target_triplet = @target@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) @NATIVE_TRUE@am__append_1 = btest stest edtest @HAVE_PTHREAD_TRUE@@NATIVE_TRUE@am__append_2 = ttest +@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_3 = dtest subdir = . DIST_COMMON = README ChangeLog $(srcdir)/Makefile.in \ $(srcdir)/Makefile.am $(top_srcdir)/configure \ @@ -217,6 +218,7 @@ MAKEINFO = @MAKEINFO@ MKDIR_P = @MKDIR_P@ NM = @NM@ NMEDIT = @NMEDIT@ +OBJCOPY = @OBJCOPY@ OBJDUMP = @OBJDUMP@ OBJEXT = @OBJEXT@ OTOOL = @OTOOL@ @@ -343,7 +345,7 @@ libbacktrace_la_LIBADD = \ $(ALLOC_FILE) libbacktrace_la_DEPENDENCIES = $(libbacktrace_la_LIBADD) -TESTS = $(check_PROGRAMS) +TESTS = $(check_PROGRAMS) $(am__append_3) @NATIVE_TRUE@btest_SOURCES = btest.c testlib.c @NATIVE_TRUE@btest_CFLAGS = $(AM_CFLAGS) -g -O @NATIVE_TRUE@btest_LDADD = libbacktrace.la @@ -799,6 +801,10 @@ uninstall-am: @NATIVE_TRUE@ cat $(srcdir)/edtest2.c > tmp-edtest2_build.c @NATIVE_TRUE@ $(SHELL) $(srcdir)/../move-if-change tmp-edtest2_build.c edtest2_build.c @NATIVE_TRUE@ echo timestamp > $@ + +@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@dtest: btest +@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@ $(OBJCOPY) --only-keep-debug btest btest.debug +@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@ $(OBJCOPY) --strip-debug --add-gnu-debuglink=btest.debug btest dtest alloc.lo: config.h backtrace.h internal.h backtrace.lo: config.h backtrace.h internal.h btest.lo: (INCDIR)/filenames.h backtrace.h backtrace-supported.h Index: configure === --- configure (revision 251948) +++ configure (working copy) @@ -604,6 +604,9 @@ LTLIBOBJS LIBOBJS NATIVE_FALSE NATIVE_TRUE +HAVE_OBJCOPY_DEBUGLINK_FALSE +HAVE_OBJCOPY_DEBUGLINK_TRUE +OBJCOPY HAVE_PTHREAD_FALSE HAVE_PTHREAD_TRUE PTHREAD_CFLAGS @@ -746,7 +749,8 @@ CFLAGS LDFLAGS LIBS CPPFLAGS -CPP' +CPP +OBJCOPY' # Initialize some variables set by options. @@ -1396,6 +1400,7 @@ Some influential environment variables: CPPFLAGSC/C++/Objective C preprocessor flags, e.g. -I if you have headers in a nonstandard directory CPP C preprocessor + OBJCOPY location of objcopy Use these variables to override the choices made by `configure' or to help it to find libraries and programs with nonstandard names/locations. @@ -11136,7 +11141,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 11139 "configure" +#line 11144 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -11242,7 +11247,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 11245 "configure" +#line 11250 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -12761,6 +12766,69 @@ else fi + +# Extract the first word of "objcopy", so it can be a program name with args. +set dummy objcopy; ac_word=$2 +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5 +$as_echo_n "checking for $ac_word... " >&6; } +if test "${ac_cv_prog_OBJCOPY+set}" = set; then : + $as_echo_n "(cached) " >&6 +else + if test -n "$OBJCOPY"; then + ac_cv_prog_OBJCOPY="$OBJCOPY" # Let the user override the test. +else +as_save_IFS=$IFS; IFS=$PATH_SEPARATOR +for as_dir in $PATH +do + IFS=$as_save_IFS + test -z "$as_dir" && as_dir=. +for ac_exec_ext in '' $ac_executable_extensions; do + if { test -f "$as_dir/$ac_word$ac_exec_ext" && $as
Re: Ping on MIPS specific patch adds `short_call` synonym for `near` attribute
bootstrap and regtested on mips64el-unknown-linux-gnu. On Sat, Sep 9, 2017 at 2:34 PM, Simon Atanasyan wrote: > Hi Paul, > > Probably the patch in the original message is broken somehow. Anyway I > attach updated patch to this mail. > > $ svn up > Updating '.': > At revision 251918. > $ patch -p0 < ~/add_short_call_attribute.patch > patching file gcc/config/mips/mips.c > patching file gcc/doc/extend.texi > patching file gcc/testsuite/gcc.target/mips/near-far-1.c > patching file gcc/testsuite/gcc.target/mips/near-far-2.c > patching file gcc/testsuite/gcc.target/mips/near-far-3.c > patching file gcc/testsuite/gcc.target/mips/near-far-4.c > > On Sat, Sep 09, 2017 at 11:26:57AM +0800, Paul Hua wrote: >> I would like to testing your patch on mips64el target. but your patch >> can't apply to current trunk. >> >> On Thu, Sep 7, 2017 at 7:22 PM, Simon Atanasyan wrote: >> > This is a ping for a small MIPS specific patch adds `short_call` >> > synonym for existing `near` attribute. The patch has not been approved >> > or commented on. >> > >> > Add 'short_call' attribute for MIPS targets. >> > https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01280.html > > -- > Simon Atanasyan
[PATCH] PR target/82166: Update preferred stack boundary for leaf functions
preferred_stack_boundary may not be the minimum stack alignment. For leaf functions without TLS access, max_used_stack_slot_alignment may be smaller. We should update preferred_stack_boundary for leaf functions. Tested on i686 and x86-64. OK for trunk? H.J. --- gcc/ PR target/82166 * config/i386/i386.c (ix86_finalize_stack_frame_flags): Properly compute the minimum stack alignment. Also update preferred stack boundary when main () is a leaf function. gcc/testsuite/ PR target/82166 * gcc.target/i386/pr82166.c: New test. --- gcc/config/i386/i386.c | 17 - gcc/testsuite/gcc.target/i386/pr82166.c | 14 ++ 2 files changed, 26 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr82166.c diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 4e93cc1043b..03338e422be 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -14257,11 +14257,12 @@ ix86_finalize_stack_frame_flags (void) unsigned int incoming_stack_boundary = (crtl->parm_stack_boundary > ix86_incoming_stack_boundary ? crtl->parm_stack_boundary : ix86_incoming_stack_boundary); + unsigned int stack_alignment += (crtl->is_leaf && !ix86_current_function_calls_tls_descriptor + ? crtl->max_used_stack_slot_alignment + : crtl->stack_alignment_needed); unsigned int stack_realign -= (incoming_stack_boundary - < (crtl->is_leaf && !ix86_current_function_calls_tls_descriptor - ? crtl->max_used_stack_slot_alignment - : crtl->stack_alignment_needed)); += (incoming_stack_boundary < stack_alignment); bool recompute_frame_layout_p = false; if (crtl->stack_realign_finalized) @@ -14306,7 +14307,9 @@ ix86_finalize_stack_frame_flags (void) HARD_FRAME_POINTER_REGNUM); /* The preferred stack alignment is the minimum stack alignment. */ - unsigned int stack_alignment = crtl->preferred_stack_boundary; + if (stack_alignment > crtl->preferred_stack_boundary) + stack_alignment = crtl->preferred_stack_boundary; + bool require_stack_frame = false; FOR_EACH_BB_FN (bb, cfun) @@ -14349,6 +14352,10 @@ ix86_finalize_stack_frame_flags (void) = incoming_stack_boundary; crtl->stack_alignment_needed = incoming_stack_boundary; + /* Also update preferred_stack_boundary for leaf +functions. */ + crtl->preferred_stack_boundary + = incoming_stack_boundary; } } else diff --git a/gcc/testsuite/gcc.target/i386/pr82166.c b/gcc/testsuite/gcc.target/i386/pr82166.c new file mode 100644 index 000..8bc63e15231 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr82166.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-Os" } */ + +void foo (void); +int a, b, c; +int +main (void) +{ + int j; + for (; c;) +a = b; + for (; j;) +foo (); +} -- 2.13.5
Improve int<->FP conversions
This patch improves the latency of code by eliminating two FP <-> integer register transfers. An example: float f1(float x) { int y = x; return (float)y; } Trunk generates: f1: fcvtzs w0, s0 scvtf s0, w0 ret With the patch we can use the neon scalar instructions and eliminate the two FP <-> integer register transfes. f1: fcvtzs s0, s0 scvtf s0, s0 ret Bootstrapped and tested on aarch64-linux-gnu. Okay for trunk? 2017-09-02 Michael Collison * config/aarch64/aarch64.md(_trunc>2): New pattern. (_trunchf2: New pattern. (_trunc2: New pattern. * config/aarch64/iterators.md (wv): New mode attribute. (vf, VF): New mode attributes. (vgp, VGP): New mode attributes. (s): Update attribute with SImode and DImode prefixes. * testsuite/gcc.target/aarch64/fix_trunc1.c: New testcase. pr6527.patch Description: pr6527.patch
Re: std::forward_list optim for always equal allocator
On 08/09/2017 18:19, Jonathan Wakely wrote: On 28/08/17 21:09 +0200, François Dumont wrote: Hi Any news for this patch ? It does remove a constructor: -_Fwd_list_impl(const _Node_alloc_type& __a) -: _Node_alloc_type(__a), _M_head() It was already unused before the patch. Do you think it has ever been used and so do I need to restore it ? I eventually restore the _M_head() in _Fwd_list_impl constructors cause IMO it is the inline init of _M_next in _Fwd_list_node_base which should be removed. But I remember Jonathan that you didn't want to do so because gcc was not good enough in detecting usage of uninitialized variables, is it still true ? Why should it be removed? When user declare a container iterator like that: std::forward_list::iterator it; There is no reason to initialize it with a null node pointer. It is just an uninitialized iterator which is invalid to use except to initialize it. I once proposed to do the same simplification for the unordered containers _Hash_node_base. But you said that detection of the usage of uninitialized variable is not good enough in gcc to leave variables uninitialized this way.
Re: Improve int<->FP conversions
On Sun, Sep 10, 2017 at 9:50 PM, Michael Collison wrote: > This patch improves the latency of code by eliminating two FP <-> integer > register transfers. > > An example: > > float f1(float x) > { > int y = x; > return (float)y; > } > > Trunk generates: > > f1: > fcvtzs w0, s0 > scvtf s0, w0 > ret > > With the patch we can use the neon scalar instructions and eliminate the two > FP <-> integer register transfes. > > f1: > fcvtzs s0, s0 > scvtf s0, s0 > ret > > Bootstrapped and tested on aarch64-linux-gnu. Okay for trunk? > > 2017-09-02 Michael Collison > > * config/aarch64/aarch64.md(_trunc>2): > New pattern. > (_trunchf2: New pattern. > (_trunc2: New pattern. You might want to set the simd and fp attr on these patterns but other than I think this patch looks good (though I cannot approve it). Thanks, Andrew > * config/aarch64/iterators.md (wv): New mode attribute. > (vf, VF): New mode attributes. > (vgp, VGP): New mode attributes. > (s): Update attribute with SImode and DImode prefixes. > * testsuite/gcc.target/aarch64/fix_trunc1.c: New testcase.
Re: std::forward_list optim for always equal allocator
2017-09-11 7:12 GMT+02:00 François Dumont : > When user declare a container iterator like that: > > std::forward_list::iterator it; > > There is no reason to initialize it with a null node pointer. It is just an > uninitialized iterator which is invalid to use except to initialize it. While that is correct, for every forward iterator (and std::forward_list::iterator meets these requirements), it is also required that a value-initialized iterator can be compared against other initialized iterators, so this reduces the amount of freedom to define a default constructor for such iterators even when used to default-initialize. This is not meant as a showstopper argument, since I have not fully understood of what you are planning, but just a reminder. - Daniel