[PATCH 2/2] [C++] pr31397 - implement -Wsuggest-override
From: Trevor Saunders
Hi,
the idea in the pr of providing a warning that tells people where they can add
override seems reasonable to me.
bootstrapped + regtested x86_64-unknown--linux-gnu (new test passes), ok?
Trev
c-family/
* c.opt (Wsuggest-override): New option.
cp/
* class.c (check_for_override): Warn when a virtual function is an
override not marked override.
gcc/
* doc/invoke.texi: Document -Wsuggest-override.
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 85dcb98..259b520 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -574,6 +574,11 @@ Wsuggest-attribute=format
C ObjC C++ ObjC++ Var(warn_suggest_attribute_format) Warning
Warn about functions which might be candidates for format attributes
+Wsuggest-override
+C++ ObjC++ Var(warn_override) Warning
+Suggest that the override keyword be used when the declaration of a virtual
+function overrides another.
+
Wswitch
C ObjC C++ ObjC++ Var(warn_switch) Warning LangEnabledBy(C ObjC C++
ObjC++,Wall)
Warn about enumerated switches, with no default, missing a case
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 16279df..515f33f 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -2777,6 +2777,10 @@ check_for_override (tree decl, tree ctype)
{
DECL_VINDEX (decl) = decl;
overrides_found = true;
+ if (warn_override && DECL_VIRTUAL_P (decl) && !DECL_OVERRIDE_P (decl)
+ && !DECL_DESTRUCTOR_P (decl))
+ warning_at(DECL_SOURCE_LOCATION (decl), OPT_Wsuggest_override,
+ "%q+D can be marked override", decl);
}
if (DECL_VIRTUAL_P (decl))
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 89edddb..8741e8e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -275,7 +275,7 @@ Objective-C and Objective-C++ Dialects}.
-Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol
-Wstrict-aliasing=n @gol -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
-Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol
--Wsuggest-final-types @gol -Wsuggest-final-methods @gol
+-Wsuggest-final-types @gol -Wsuggest-final-methods @gol -Wsuggest-override @gol
-Wmissing-format-attribute @gol
-Wswitch -Wswitch-default -Wswitch-enum -Wswitch-bool -Wsync-nand @gol
-Wsystem-headers -Wtrampolines -Wtrigraphs -Wtype-limits -Wundef @gol
@@ -4255,6 +4255,10 @@ effective with link time optimization, where the
information about the class
hiearchy graph is more complete. It is recommended to first consider suggestins
of @option{-Wsuggest-final-types} and then rebuild with new annotations.
+@item -Wsuggest-override
+Warn about overriding virtual functions that are not marked with the override
+keyword.
+
@item -Warray-bounds
@opindex Wno-array-bounds
@opindex Warray-bounds
diff --git a/gcc/testsuite/g++.dg/warn/Wsuggest-override.C
b/gcc/testsuite/g++.dg/warn/Wsuggest-override.C
new file mode 100644
index 000..929d365
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wsuggest-override.C
@@ -0,0 +1,21 @@
+// { dg-do compile }
+// { dg-options "-std=c++11 -Wsuggest-override" }
+struct A
+{
+ A();
+ virtual ~A();
+ virtual void f();
+ virtual int bar();
+ operator int();
+ virtual operator float();
+};
+
+struct B : A
+{
+ B();
+ virtual ~B();
+ virtual void f(); // { dg-warning "can be marked override" }
+virtual int bar() override;
+operator int();
+virtual operator float(); // { dg-warning "can be marked override" }
+};
--
2.1.3
[PATCH 1/2] [C++] Remove tree_list from warn_hidden
From: Trevor Saunders
Hi,
I'll claim this is kind of a bug fix in so much as excessive gc allocation and
use of tree_list is a bug.
bootstrapped + regtested x86_64-unknown-linux-gnu, ok?
Trev
cp/
* class.c (warn_hidden): Use auto_vec instead of tree_list to
hold base_fndecls.
(get_basefndecls): Adjust.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index c83c8ad..16279df 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -129,7 +129,7 @@ vec *local_classes;
static tree get_vfield_name (tree);
static void finish_struct_anon (tree);
static tree get_vtable_name (tree);
-static tree get_basefndecls (tree, tree);
+static void get_basefndecls (tree, tree, vec *);
static int build_primary_vtable (tree, tree);
static int build_secondary_vtable (tree);
static void finish_vtbls (tree);
@@ -2717,16 +2717,16 @@ modify_all_vtables (tree t, tree virtuals)
/* Get the base virtual function declarations in T that have the
indicated NAME. */
-static tree
-get_basefndecls (tree name, tree t)
+static void
+get_basefndecls (tree name, tree t, vec *base_fndecls)
{
tree methods;
- tree base_fndecls = NULL_TREE;
int n_baseclasses = BINFO_N_BASE_BINFOS (TYPE_BINFO (t));
int i;
/* Find virtual functions in T with the indicated NAME. */
i = lookup_fnfields_1 (t, name);
+ bool found_decls = false;
if (i != -1)
for (methods = (*CLASSTYPE_METHOD_VEC (t))[i];
methods;
@@ -2736,20 +2736,20 @@ get_basefndecls (tree name, tree t)
if (TREE_CODE (method) == FUNCTION_DECL
&& DECL_VINDEX (method))
- base_fndecls = tree_cons (NULL_TREE, method, base_fndecls);
+ {
+ base_fndecls->safe_push (method);
+ found_decls = true;
+ }
}
- if (base_fndecls)
-return base_fndecls;
+ if (found_decls)
+return;
for (i = 0; i < n_baseclasses; i++)
{
tree basetype = BINFO_TYPE (BINFO_BASE_BINFO (TYPE_BINFO (t), i));
- base_fndecls = chainon (get_basefndecls (name, basetype),
- base_fndecls);
+ get_basefndecls (name, basetype, base_fndecls);
}
-
- return base_fndecls;
}
/* If this declaration supersedes the declaration of
@@ -2811,7 +2811,6 @@ warn_hidden (tree t)
tree fn;
tree name;
tree fndecl;
- tree base_fndecls;
tree base_binfo;
tree binfo;
int j;
@@ -2820,19 +2819,18 @@ warn_hidden (tree t)
have the same name. Figure out what name that is. */
name = DECL_NAME (OVL_CURRENT (fns));
/* There are no possibly hidden functions yet. */
- base_fndecls = NULL_TREE;
+ auto_vec base_fndecls;
/* Iterate through all of the base classes looking for possibly
hidden functions. */
for (binfo = TYPE_BINFO (t), j = 0;
BINFO_BASE_ITERATE (binfo, j, base_binfo); j++)
{
tree basetype = BINFO_TYPE (base_binfo);
- base_fndecls = chainon (get_basefndecls (name, basetype),
- base_fndecls);
+ get_basefndecls (name, basetype, &base_fndecls);
}
/* If there are no functions to hide, continue. */
- if (!base_fndecls)
+ if (!base_fndecls.exists ())
continue;
/* Remove any overridden functions. */
@@ -2842,28 +2840,27 @@ warn_hidden (tree t)
if (TREE_CODE (fndecl) == FUNCTION_DECL
&& DECL_VINDEX (fndecl))
{
- tree *prev = &base_fndecls;
-
- while (*prev)
/* If the method from the base class has the same
signature as the method from the derived class, it
has been overridden. */
- if (same_signature_p (fndecl, TREE_VALUE (*prev)))
- *prev = TREE_CHAIN (*prev);
- else
- prev = &TREE_CHAIN (*prev);
+ for (size_t k = 0; k < base_fndecls.length (); k++)
+ if (base_fndecls[k]
+ && same_signature_p (fndecl, base_fndecls[k]))
+ base_fndecls[k] = NULL_TREE;
}
}
/* Now give a warning for all base functions without overriders,
as they are hidden. */
- while (base_fndecls)
- {
- /* Here we know it is a hider, and no overrider exists. */
- warning (OPT_Woverloaded_virtual, "%q+D was hidden", TREE_VALUE
(base_fndecls));
- warning (OPT_Woverloaded_virtual, " by %q+D", fns);
- base_fndecls = TREE_CHAIN (base_fndecls);
- }
+ size_t k;
+ tree base_fndecl;
+ FOR_EACH_VEC_ELT (base_fndecls, k, base_fndecl)
+ if (base_fndecl)
+ {
+ /* Here we know it is a hider, and no overrider exists. */
+ warning (OPT_Woverloaded_virtual, "%q+D was hidden", base_fndecl);
+ warning (OPT_Woverloaded_virtual, " by %q+D", fns);
+ }
}
}
--
2.1.3
Re: [PATCH 04/08] PR jit/63854: Remove xstrdup from ipa/cgraph fprintf calls
On Wed, Nov 26, 2014 at 09:49:52AM -0500, David Malcolm wrote:
> On Wed, 2014-11-26 at 09:13 +0100, Uros Bizjak wrote:
> > Hello!
> >
> > > cgraph*.c and ipa-*.c use xstrdup on strings when dumping them via
> > > fprintf, leaking all of the duplicated buffers.
> > >
> > > Is/was there a reason for doing this?
> >
> > Yes, please see [1] and PR 53136 [2]. As said in [1]:
> >
> > "There is a problem with multiple calls of cgraph_node_name in fprintf
> > dumps. Please note that C++ uses caching in
> > cxx_printable_name_internal (aka LANG_HOOKS_DECL_PRINTABLE_NAME), so
> > when cxx_printable_name_internal is called multiple times from printf
> > (i.e. fprintf "%s/%i -> %s/%i"), it can happen that the first string
> > gets evicted by the second call, before fprintf is fully evaluated."
> >
> > > Taking them out fixes these leaks (seen when dumping is enabled):
> >
> > But you will get "Invalid read of size X" instead.
> >
> > The patch at [1] fixed these, but introduced memory leaks, which were
> > tolerable at the time:
> >
> > "I think that small memory leak is tolerable here (the changes are
> > exclusively in the dump code), and follows the same approach as in
> > java frontend."
> >
> > It seems that these assumptions are not valid anymore.
> >
> > [1] https://gcc.gnu.org/ml/gcc-patches/2012-04/msg01904.html
> > [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53136
>
> Aha! Thanks.
>
> I was only running under valgrind when testing the jit code, and the jit
> code looks like a frontend to the rest of GCC, it doesn't provide an
> implementation of LANG_HOOKS_DECL_PRINTABLE_NAME. I ran the rest of the
> gcc testsuite outside of gcc. So perhaps the duplications of the
> transient decl strings is still needed, and my patch would undo that
> fix. Oops.
>
> I'm about to disappear for a US holiday, but maybe a good solution here
> is to invent something like a "xstrdup_for_dump" function like this:
>
> /* When using fprintf (or similar), problems can arise with
> transient generated strings. Many string-generation APIs
> only support one result being alive at once (e.g. by
> returning a pointer to a statically-allocated buffer).
>
> If there is more than one generated string within one
> fprintf call: the first string gets evicted or overwritten
> by the second, before fprintf is fully evaluated.
> See e.g. PR/53136.
>
> This function provides a workaround for this, by providing
> a simple way to create copies of these transient strings,
> without the need to have explicit cleanup:
>
> fprintf (dumpfile, "string 1: %s string 2:%s\n",
> xstrdup_for_dump (EXPR_1),
> xstrdup_for_dump (EXPR_2));
>
> The copied string is eventually freed, from code within
> toplev::finalize. */
>
> extern const char *
> xstrdup_for_dump (const char *transient_str);
>
> We could then use this instead of xstrdup at these callsites, which
> would document what's going on.
>
> (I'm not in love with the name, but hopefully the idea is clear).
>
> This could be implemented either:
>
> (A) in terms of the "long-term allocator" idea here:
> https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02420.html
> which uses a obstack on the gcc::context, eventually freeing the
> allocations from toplev::finalize (although that's currently only called
> from libgccjit.so, but we could call it from cc1 etc when running under
> valgrind).
>
> (B) or in terms of ggc_xstrdup, with the assumption that no GC can occur
> in anything within a fprintf call.
>
> That would document what's going on, ensure that we don't have
> use-after-free issues, and stop memory leak warnings from valgrind
> concerning these duplicates.
>
> Thoughts?
"bleh"? ;-) however I'm not sure I have a better idea, but here are
some other options.
- have lang_hooks_decl_printable_name return something like
auto_ptr so you'd write
fprintf ("blah: %s", hooks.printable_name (t).get ());
- depending on who all calls this function we could also replace the
static cache with a hash map from trees to strings.
Trev
> Dave
>
Re: [PATCH, ARM, libgcc] New aeabi_idiv function for armv6-m
OK applying to arm/embedded-4_9-branch, though you still need
maintainer approval into trunk.
- Joey
On Wed, Nov 26, 2014 at 11:43 AM, Hale Wang wrote:
> Hi,
>
> This patch ports the aeabi_idiv routine from Linaro Cortex-Strings
> (https://git.linaro.org/toolchain/cortex-strings.git), which was contributed
> by ARM under Free BSD license.
>
> The new aeabi_idiv routine is used to replace the one in
> libgcc/config/arm/lib1funcs.S. This replacement happens within the Thumb1
> wrapper. The new routine is under LGPLv3 license.
>
> The main advantage of this version is that it can improve the performance of
> the aeabi_idiv function for Thumb1. This solution will also increase the
> code size. So it will only be used if __OPTIMIZE_SIZE__ is not defined.
>
> Make check passed for armv6-m.
>
> OK for trunk?
>
> Thanks,
> Hale Wang
>
> libgcc/ChangeLog:
>
> 2014-11-26 Hale Wang
>
> * config/arm/lib1funcs.S: Add new wrapper.
>
> ===
> diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
> index b617137..de66c81 100644
> --- a/libgcc/config/arm/lib1funcs.S
> +++ b/libgcc/config/arm/lib1funcs.S
> @@ -306,34 +306,12 @@ LSYM(Lend_fde):
> #ifdef __ARM_EABI__
> .macro THUMB_LDIV0 name signed
> #if defined(__ARM_ARCH_6M__)
> - .ifc \signed, unsigned
> - cmp r0, #0
> - beq 1f
> - mov r0, #0
> - mvn r0, r0 @ 0x
> -1:
> - .else
> - cmp r0, #0
> - beq 2f
> - blt 3f
> +
> + push{r0, lr}
> mov r0, #0
> - mvn r0, r0
> - lsr r0, r0, #1 @ 0x7fff
> - b 2f
> -3: mov r0, #0x80
> - lsl r0, r0, #24 @ 0x8000
> -2:
> - .endif
> - push{r0, r1, r2}
> - ldr r0, 4f
> - adr r1, 4f
> - add r0, r1
> - str r0, [sp, #8]
> - @ We know we are not on armv4t, so pop pc is safe.
> - pop {r0, r1, pc}
> - .align 2
> -4:
> - .word __aeabi_idiv0 - 4b
> + bl SYM(__aeabi_idiv0)
> + pop {r1, pc}
> +
> #elif defined(__thumb2__)
> .syntax unified
> .ifc \signed, unsigned
> @@ -927,7 +905,158 @@ LSYM(Lover7):
> add dividend, work
>.endif
> LSYM(Lgot_result):
> -.endm
> +.endm
> +
> +#if defined(__prefer_thumb__) && !defined(__OPTIMIZE_SIZE__)
> +.macro BranchToDiv n, label
> + lsr curbit, dividend, \n
> + cmp curbit, divisor
> + blo \label
> +.endm
> +
> +.macro DoDiv n
> + lsr curbit, dividend, \n
> + cmp curbit, divisor
> + bcc 1f
> + lsl curbit, divisor, \n
> + sub dividend, dividend, curbit
> +
> +1: adc result, result
> +.endm
> +
> +.macro THUMB1_Div_Positive
> + mov result, #0
> + BranchToDiv #1, LSYM(Lthumb1_div1)
> + BranchToDiv #4, LSYM(Lthumb1_div4)
> + BranchToDiv #8, LSYM(Lthumb1_div8)
> + BranchToDiv #12, LSYM(Lthumb1_div12)
> + BranchToDiv #16, LSYM(Lthumb1_div16)
> +LSYM(Lthumb1_div_large_positive):
> + mov result, #0xff
> + lsl divisor, divisor, #8
> + rev result, result
> + lsr curbit, dividend, #16
> + cmp curbit, divisor
> + blo 1f
> + asr result, #8
> + lsl divisor, divisor, #8
> + beq LSYM(Ldivbyzero_waypoint)
> +
> +1: lsr curbit, dividend, #12
> + cmp curbit, divisor
> + blo LSYM(Lthumb1_div12)
> + b LSYM(Lthumb1_div16)
> +LSYM(Lthumb1_div_loop):
> + lsr divisor, divisor, #8
> +LSYM(Lthumb1_div16):
> + Dodiv #15
> + Dodiv #14
> + Dodiv #13
> + Dodiv #12
> +LSYM(Lthumb1_div12):
> + Dodiv #11
> + Dodiv #10
> + Dodiv #9
> + Dodiv #8
> + bcs LSYM(Lthumb1_div_loop)
> +LSYM(Lthumb1_div8):
> + Dodiv #7
> + Dodiv #6
> + Dodiv #5
> +LSYM(Lthumb1_div5):
> + Dodiv #4
> +LSYM(Lthumb1_div4):
> + Dodiv #3
> +LSYM(Lthumb1_div3):
> + Dodiv #2
> +LSYM(Lthumb1_div2):
> + Dodiv #1
> +LSYM(Lthumb1_div1):
> + sub divisor, dividend, divisor
> + bcs 1f
> + cpy divisor, dividend
> +
> +1: adc result, result
> + cpy dividend, result
> + RET
> +
> +LSYM(Ldivbyzero_waypoint):
> + b LSYM(Ldiv0)
> +.endm
> +
> +.macro THUMB1_Div_Negative
> + lsr result, divisor, #31
> + beq 1f
> + neg divisor, divisor
> +
> +1: asr curbit, dividend, #32
> + bcc 2f
> + neg dividend, dividend
> +
> +2: eor curbit, result
> + mov result, #0
> + cpy ip, curbit
> + BranchToDiv #4, LSYM(Lthumb1_div_negative4)
> + BranchToDiv #8, LSYM(Lthumb1_div_negative8)
> +LSYM(Lthumb1_div_large):
> + mov result, #0xfc
> + lsl
[PATCH, committed] skip alignof2.C testcase on AIX
The AIX ABI unfortunately does not use natural alignment. Type
"double" is 4 word aligned. However, the alignment of structs with a
first member of double are bumped to doubleword alignment. This
exactly conflicts with the testcase.
Thanks, David
* g++.dg/ext/alignof2.C: xfail-run-if on AIX.
Index: ext/alignof2.C
===
--- ext/alignof2.C (revision 218086)
+++ ext/alignof2.C (working copy)
@@ -3,6 +3,7 @@
// wrong for some fields.
// { dg-do run }
+// { dg-xfail-run-if "AIX ABI increases struct alignment for first
member double" { powerpc-ibm-aix* } { "*" } { "" } }
extern "C" void abort();
[tree-type] Split decl_attribute into type and decl specific versions.
First independent infrastructure patch for the branch. These sort of
patches I will commit to the branch without any tree-type specific
changes so I can maintain it as a patch to submit in stage 1 independent
of the overall larger change. I'll make the tree-type specific changes
last.
decl_attribute can be applied to either a type or a decl. This separates
that function into decl_attributes() and type_attributes() (adding
checking asserts for good measure), and changes the calling locations.
It was not an enjoyable function to split. ugg. Hope the rest aren't
like this :-)
Bootstraps, passes regression testing and build all targets.The next
part will change the attribute_spec structure to have a separate handler
for types and for decls and change all the locations as handlers as need
be. Easier to test by keeping these changes apart from each other.
checked into tree-type branch as revision 218105
Andrew
* attribs.c (process_attribute_spec): New. Factored from
decl_attribute to return a valid attribute spec.
(finalize_type_attribute): New. Factored from decl_attribute to process
attributes for types after error checking.
(type_attributes): New. Factored type specific portions of
decl_attributes.
(decl_attributes): Moved all type processing to other functions.
* attribs.h (type_attributes): Export.
* c/c-decl.c (groktypename, grokdeclarator, finish_struct,
finish_enum): Call type_attributes.
* c-family/c-common.c (handle_tm_attribute): Call type_attributes.
* cp/decl.c (grokdeclarator): Call type_attributes.
* cp/decl2.c (cplus_decl_attributes): Call type_attributes when node is
a type.
* ada/gcc-interface/utils.c (finish_record_type): Call type_attribute.
(process_attributes): Call type_attribute or decl_attribute as needed.
* config/i386/i386.c (ix86_handle_tm_regparm_attribute): Call
type_attribute or decl_attribute as required.
(ix86_init_tm_builtins ): Call type_attributes.
Index: attribs.c
===
*** attribs.c (revision 217787)
--- attribs.c (working copy)
*** get_attribute_namespace (const_tree attr
*** 354,368
return get_identifier ("gnu");
}
/* Process the attributes listed in ATTRIBUTES and install them in *NODE,
!which is either a DECL (including a TYPE_DECL) or a TYPE. If a DECL,
!it should be modified in place; if a TYPE, a copy should be created
!unless ATTR_FLAG_TYPE_IN_PLACE is set in FLAGS. FLAGS gives further
!information, in the form of a bitwise OR of flags in enum attribute_flags
!from tree.h. Depending on these flags, some attributes may be
!returned to be applied at a later stage (for example, to apply
!a decl attribute to the declaration rather than to its type). */
tree
decl_attributes (tree *node, tree attributes, int flags)
--- 354,622
return get_identifier ("gnu");
}
+ /* Lookup the spec for attribute A using FLAGS, and issue any warnings or
+ errors as appropriate. Return NULL or a valid spec. */
+
+ static const struct attribute_spec *
+ process_attribute_spec (tree a, int flags)
+ {
+ tree name = get_attribute_name (a);
+ tree ns = get_attribute_namespace (a);
+ tree args = TREE_VALUE (a);
+ const struct attribute_spec *spec = lookup_scoped_attribute_spec (ns, name);
+
+ if (spec == NULL)
+ {
+ if (!(flags & (int) ATTR_FLAG_BUILT_IN))
+ {
+ if (ns == NULL_TREE || !cxx11_attribute_p (a))
+ warning (OPT_Wattributes, "%qE attribute directive ignored",
+ name);
+ else
+ warning (OPT_Wattributes,
+ "%<%E::%E%> scoped attribute directive ignored",
+ ns, name);
+ }
+ return NULL;
+ }
+ else if (list_length (args) < spec->min_length
+ || (spec->max_length >= 0
+ && list_length (args) > spec->max_length))
+ {
+ error ("wrong number of arguments specified for %qE attribute",
+ name);
+ return NULL;
+ }
+ gcc_assert (is_attribute_p (spec->name, name));
+ return spec;
+ }
+
+ /* Perform the type specific processing of type_attribute. This is factored
+out to allow decl_attributes() to process underlying types when necessary.
+In those cases, DECL_NODE is passed in so the decl can have re-layout
+called. SPEC is the already verified attribute spec structure. Otherwise
+the parameters match those of type_attribute. */
+
+ static tree
+ finalize_type_attribute (tree *node, const struct attribute_spec *spec,
+ tree a, tree returned_attrs, int flags,
+ tree *decl_node = NULL)
+ {
+ bool no_add_attrs = 0;
+ int fn_ptr_quals = 0;
+ tree fn_ptr_tmp = NULL_TREE;
+ tree name = get_attribute_name (a);
+ tree args = TREE_VALUE (a);
+ tree *anode = node;
+
+ gcc_checking_assert (TYPE_P (*node));
+
+ if (spec->function_type_required && TREE_CODE (*anode) != FUNCTION_TYPE
+ && TREE_CODE (*anode) != METHOD_TYPE)
+ {
+ if (TREE_CO
Re: [PATCH v3] gcc/ubsan.c: Use 'pretty_print' for 'pretty_name' to avoid memory overflow
If necessary to add related test case, please let me know. Thanks. Send from Lenovo A788t. Jakub Jelinek wrote: >On Mon, Nov 24, 2014 at 04:28:10PM +0800, Chen Gang wrote: >> On 11/24/14 15:41, Jakub Jelinek wrote: >> > On Sun, Nov 23, 2014 at 09:13:27AM +0800, Chen Gang wrote: >> >> [...] >> >> >> + else >> >> + pp_wide_int(&pretty_name, >> >> + wi::add (wi::to_widest (TYPE_MAX_VALUE (dom)), 1), >> >> + TYPE_SIGN (TREE_TYPE (dom))); >> > >> > Space still missing before ( (and reindenting the following 2 lines). >> > >> >> Oh, thanks, if necessary to send patch v4, please let me know. > >No, just fix it up before checking in. > > Jakub
Re: [PATCH v4] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value
OK, I shall send related test case within two weeks (2014-12-10). Thanks. Send from Lenovo A788t. Joseph Myers wrote: >On Thu, 27 Nov 2014, Chen Gang wrote: > >> The original length 18 is not enough for HOST_WIDE_INT printing, need >> use 20 instead of. >> >> Also need additional bytes for printing related prefix and suffix, and >> give a related check. >> >> It passes testsuite under fedora 20 x86_64-unknown-linux-gnu. >> >> 2014-11-27 Chen Gang >> >> * c-family/c-cppbuiltin.c (builtin_define_with_int_value): Let >> buffer enough to print host wide integer value. > >OK. Properly implementing the (-9223372036854775807LL-1) and similar >cases (when the value is the least int, long or long long on the target) >can be a followup fix. > >-- >Joseph S. Myers >jos...@codesourcery.com
Re: patch to fix PR63897
On Mon, Nov 24, 2014 at 4:23 PM, H.J. Lu wrote: > On Fri, Nov 21, 2014 at 1:32 PM, Vladimir Makarov wrote: >> The following patch fixes PR63897. The details can be found on >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63897 >> >> The patch was successfully bootstrapped and tested on x86 and x86-64. >> >> Committed as rev. 217947. >> >> 2014-11-21 Vladimir Makarov >> >> PR target/63897 >> * lra-lives.c (mark_regno_live, mark_regno_dead): Remove last >> argument. >> (process_bb_lives): Rename dead_insn_p on remove_p >> and global_live_info_p on dead_insn_p. Calculate local live info >> unconditionally. Remove last argument in calls mark_regno_live and >> mark_regno_dead. Reorganize body of EXECUTE_IF_SET_IN_BITMAP. >> (lra_create_live_ranges): Rename to lra_create_live_ranges_1. >> Return bool. Rename global_live_info_p on dead_insn_p. Return >> flag of live info change. >> (lra_create_live_ranges): New. > > This caused: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64061 This also caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64087 -- H.J.
Re: [wwwdocs] Add news item about JIT
On Tuesday 2014-11-11 20:17, David Malcolm wrote: > The attached patch adds a news entry about the JIT merge to the > frontpage of the website. Yes. Except you may want to add your name to the entry as we have in some older examples. Kudos where kudos is due. :-) Oh, and gcc/doc/contrib.texi probably ought to be updated as well. Gerald
Re: [Patch,wwwdoc]Update 5.0 change for ARM new core cortex-m7
On Wednesday 2014-11-26 11:17, Terry Guo wrote: > This patch will document support and tuning for Cortex-M7 in GCC 5.0 > changes. Is it ok to commit? Looks good to me. Thanks, Gerald
Re: [patch] Excessive alignment in ix86_data_alignment
On 09 Oct 08:25, H.J. Lu wrote: > On Thu, Oct 9, 2014 at 1:37 AM, Uros Bizjak wrote: > > On Thu, Oct 9, 2014 at 10:25 AM, Kirill Yukhin > > wrote: > >> On 08 Oct 23:02, Petr Murzin wrote: > >>> Hi, > >>> I have measured performance impact on Haswell platform according to this > >>> input: > >>> https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00978.html > > Kirill, please mention: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61296 > > in your ChangeLog. > > > What about older processors? > > Kirill, please collect data on Nehelam/Westmere, Sandybrigde/Ivybride > and Silvermont. > > > The optimization was introduced well before Haswell for then current > > processors, and it was based on the recommendation from Intel > > optimization guide. If this optimization doesn't apply for new > > processors, then tune option should be introduced and set accordingly. > > > > I believe the original excessive alignment was introduced by cut/paste > from > > https://gcc.gnu.org/git/?p=gcc.git;a=patch;h=ed45e834f305d1f2709bf200a13d5beebc2fcfee > > to improve x86 FP performance, which might be partially copied from > CONSTANT_ALIGNMENT: > > https://gcc.gnu.org/git/?p=gcc.git;a=patch;h=f7d6703c5d83fc9fb06246d6eb49e9b61098045c > > > -- > H.J. Hi, Here is the info about code and data size. CODE SEGMENTS SLM Benchmark | Trunk | Trunk w/ patch | Diff, % soplex | 1970381| 1969805| -0.0292329250028 hmmer | 1244935| 1244775| -0.0128520766144 Xalan | 6464124| 6448572| -0.240589444138 gcc | 5613433| 5611001| -0.0433246464329 gamess | 16775109 | 16744869 | -0.180267085001 h264ref | 2004787| 2003155| -0.0814051567573 libquantum | 915693 | 915693 | 0.0 astar | 956881 | 956849 | -0.0033441984949 gobmk | 3000807| 3000263| -0.0181284567785 namd| 1525077| 1525045| -0.00209825471107 lbm | 777141 | 777141 | 0.0 milc| 1151431| 1151303| -0.066018632 sjeng | 994163 | 993555 | -0.0611569732529 GemsFDTD| 2361733| 2360773| -0.0406481172935 leslie3d| 1474453| 1473909| -0.0368950383634 bwaves | 1076385| 1075777| -0.0564853653665 mcf | 778835 | 778835 | 0.0 gromacs | 2348746| 2348106| -0.0272485828608 wrf | 7742741| 7739637| -0.040089162223 cactusADM | 2820689| 2820049| -0.0226894918227 zeusmp | 1676605| 1675997| -0.0362637592039 sphinx | 1379854| 1379822| -0.00231908593228 omnetpp | 2872183| 2871511| -0.0233968378756 dealII | 2514404| 2512324| -0.0827233809682 povray | 3212579| 3211395| -0.0368551248078 perlbench | 3123490| 3122178| -0.0420042964761 calculix| 3914026| 3909866| -0.106284424273 bzip2 | 887974 | 887910 | -0.00720741823522 tonto | 7213573| 7206981| -0.0913832853705 SNB and IVB Benchmark | Trunk | Trunk w/ patch | Diff, % soplex | 1969317| 1968741| -0.029248719226 hmmer | 1252815| 1252655| -0.0127712391694 Xalan | 6517380| 6501828| -0.238623495945 gcc | 5634249| 5631817| -0.0431645814731 gamess | 16555117 | 16525197 | -0.18072961973 h264ref | 2062507| 2060779| -0.0837815338324 libquantum | 915085 | 915085 | 0.0 astar | 957025 | 956993 | -0.00334369530577 gobmk | 3010535| 3009991| -0.0180698779453 namd| 1505805| 1505773| -0.00212510916088 lbm | 775733 | 775733 | 0.0 milc| 1140567| 1140471| -0.00841686634805 sjeng | 997235 | 996627 | -0.0609685781185 GemsFDTD| 2451645| 2450653| -0.0404626281537 leslie3d| 1479125| 1478581| -0.0367785008028 bwaves | 1072497| 1071953| -0.0507227526044 mcf | 778803 | 778803 | 0.0 gromacs | 2322770| 2322130| -0.0275533091955 wrf | 8092645| 8090693| -0.0241206675938 cactusADM | 2808633| 2807993| -0.0227868860047 zeusmp | 1733717| 1733077| -0.0369149059506 sphinx | 1388206| 1388174| -0.00230513338798 omnetpp | 2875179| 2874475| -0.0244854320374 dealII | 2527560| 2525576| -0.078494674706 povray | 3164931| 3163971| -0.0303324148299 perlbench | 3123690| 3122346| -0.0430260365145 calculix| 3905210| 3901114| -
Re: [PATCH] Fix middle-end/64061, ICE in gen_rtx_SUBREG
On Wed, Nov 26, 2014 at 2:00 PM, Andrew Pinski wrote: > Hi, > The problem here is lra_substitute_pseudo calls gen_rtx_SUBREG with > a VOIDmode (const_int) argument but really it should not be calling > gen_rtx_SUBREG directly instead it should be using > gen_lowpart_if_possible. This patch fixes that and adds a testcase > that had happened on x86_64. I forgot to say I ran into this issue first on aarch64-linux-gnu while building libgo. Thanks, Andrew > > OK? I bootstrapped and tested this on both aarch64-linux-gnu and > x86_64-linux-gnu with no regression on either. > > Thanks, > Andrew Pinski > > ChangeLog: > * lra.c (lra_substitute_pseudo): Use gen_lowpart_if_possible > instead of gen_rtx_SUBREG/gen_lowpart_SUBREG. > > testsuite/ChangeLog: > > * gcc.dg/pr64061.c: New testcase.
[PATCH] Fix middle-end/64061, ICE in gen_rtx_SUBREG
Hi,
The problem here is lra_substitute_pseudo calls gen_rtx_SUBREG with
a VOIDmode (const_int) argument but really it should not be calling
gen_rtx_SUBREG directly instead it should be using
gen_lowpart_if_possible. This patch fixes that and adds a testcase
that had happened on x86_64.
OK? I bootstrapped and tested this on both aarch64-linux-gnu and
x86_64-linux-gnu with no regression on either.
Thanks,
Andrew Pinski
ChangeLog:
* lra.c (lra_substitute_pseudo): Use gen_lowpart_if_possible
instead of gen_rtx_SUBREG/gen_lowpart_SUBREG.
testsuite/ChangeLog:
* gcc.dg/pr64061.c: New testcase.
Index: testsuite/gcc.dg/pr64061.c
===
--- testsuite/gcc.dg/pr64061.c (revision 0)
+++ testsuite/gcc.dg/pr64061.c (revision 0)
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -g -fno-dce -fno-tree-dce" } */
+extern void *buf;
+
+extern void bar (void);
+
+int
+foo (int i)
+{
+ int j = 0;
+ if (__builtin_setjmp (buf) == 0)
+{
+ while (1)
+ {
+ j = 1;
+ bar ();
+ }
+}
+ return j ? i : 0;
+}
Index: lra.c
===
--- lra.c (revision 218071)
+++ lra.c (working copy)
@@ -1807,13 +1807,7 @@ lra_substitute_pseudo (rtx *loc, int old
machine_mode inner_mode = GET_MODE (new_reg);
if (mode != inner_mode)
- {
- if (GET_MODE_SIZE (mode) >= GET_MODE_SIZE (inner_mode)
- || ! SCALAR_INT_MODE_P (inner_mode))
- new_reg = gen_rtx_SUBREG (mode, new_reg, 0);
- else
- new_reg = gen_lowpart_SUBREG (mode, new_reg);
- }
+ new_reg = gen_lowpart_if_possible (mode, new_reg);
*loc = new_reg;
return true;
}
Re: [PATCH 3/5] combine: add regno field to LOG_LINKS
On Wed, Nov 26, 2014 at 11:14:36AM -0700, Jeff Law wrote: > >Are you talking about create_log_links? There can be no duplicates there > >(anymore), that would be multiple defs of the same reg in the same insn, > >indeed. > Yes, I was referring to the code in create_log_links. You dropped the > check for duplicate links. It caught my eye when reading the changes, > but then I realized the check may no longer be necessary. > > Hmm, what about an insn that has two destinations, which happen to be > upper and lower SUBREGs of a pseudo. Would that create duplicate links > after your change? Yes it would. And I'm not so certain distribute_log_links handles that situation gracefully. Rats. IMNSHO such RTL should be invalid (it can always be written simpler as one SET); but there seems to be no such rule. I'll add the check back. Wouldn't it be lovely if we could just use DF here... Segher
C++ PATCH for partial specialization of variable templates
Another member of the C++ committee playing with our new support for
variable templates noted that we didn't allow partial specialization of
them. Fixed thus.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit d85e489036f70718a12063345db0894797252938
Author: Jason Merrill
Date: Wed Nov 26 12:57:36 2014 -0500
Allow partial specialization of variable templates.
* cp-tree.h (TINFO_USED_TEMPLATE_ID): New.
* decl.c (duplicate_decls): Copy it.
* error.c (dump_decl) [TEMPLATE_ID_EXPR]: Handle variables.
* parser.c (cp_parser_decltype_expr): Do call finish_id_expression
on template-ids.
* pt.c (register_specialization): Remember variable template insts.
(instantiate_template_1): Find the matching partial specialization.
(check_explicit_specialization): Allow variable partial specialization.
(process_partial_specialization): Likewise.
(push_template_decl_real): Likewise.
(more_specialized_partial_spec): Rename from more_specialized_class.
(most_specialized_partial_spec): Rename from most_specialized_class.
(get_partial_spec_bindings): Rename from get_class_bindings.
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f1064e9..edd1d5d 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -99,6 +99,7 @@ c-common.h, not after.
QUALIFIED_NAME_IS_TEMPLATE (in SCOPE_REF)
DECLTYPE_FOR_INIT_CAPTURE (in DECLTYPE_TYPE)
CONSTRUCTOR_NO_IMPLICIT_ZERO (in CONSTRUCTOR)
+ TINFO_USED_TEMPLATE_ID (in TEMPLATE_INFO)
2: IDENTIFIER_OPNAME_P (in IDENTIFIER_NODE)
ICS_THIS_FLAG (in _CONV)
DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (in VAR_DECL)
@@ -801,6 +802,12 @@ typedef struct qualified_typedef_usage_s qualified_typedef_usage_t;
#define FNDECL_HAS_ACCESS_ERRORS(NODE) \
(TINFO_HAS_ACCESS_ERRORS (DECL_TEMPLATE_INFO (NODE)))
+/* Non-zero if this variable template specialization was specified using a
+ template-id, so it's a partial or full specialization and not a definition
+ of the member template of a particular class specialization. */
+#define TINFO_USED_TEMPLATE_ID(NODE) \
+ (TREE_LANG_FLAG_1 (TEMPLATE_INFO_CHECK (NODE)))
+
struct GTY(()) tree_template_info {
struct tree_common common;
vec *typedefs_needing_access_checking;
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 4ce4645..455097e 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -2137,7 +2137,14 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend)
DECL_LANG_SPECIFIC (newdecl)->u.min.u2 =
DECL_LANG_SPECIFIC (olddecl)->u.min.u2;
if (DECL_TEMPLATE_INFO (newdecl))
- new_template_info = DECL_TEMPLATE_INFO (newdecl);
+ {
+ new_template_info = DECL_TEMPLATE_INFO (newdecl);
+ if (DECL_TEMPLATE_INSTANTIATION (olddecl)
+ && DECL_TEMPLATE_SPECIALIZATION (newdecl))
+ /* Remember the presence of explicit specialization args. */
+ TINFO_USED_TEMPLATE_ID (DECL_TEMPLATE_INFO (olddecl))
+ = TINFO_USED_TEMPLATE_ID (new_template_info);
+ }
DECL_TEMPLATE_INFO (newdecl) = DECL_TEMPLATE_INFO (olddecl);
}
/* Only functions have these fields. */
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index 7d79771..5dcc149 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -1212,7 +1212,9 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags)
tree args = TREE_OPERAND (t, 1);
if (is_overloaded_fn (name))
- name = DECL_NAME (get_first_fn (name));
+ name = get_first_fn (name);
+ if (DECL_P (name))
+ name = DECL_NAME (name);
dump_decl (pp, name, flags);
pp_cxx_begin_template_argument_list (pp);
if (args == error_mark_node)
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index b8e182a..d1cd63f 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -12175,7 +12175,6 @@ cp_parser_decltype_expr (cp_parser *parser,
if (expr
&& expr != error_mark_node
- && TREE_CODE (expr) != TEMPLATE_ID_EXPR
&& TREE_CODE (expr) != TYPE_DECL
&& (TREE_CODE (expr) != BIT_NOT_EXPR
|| !TYPE_P (TREE_OPERAND (expr, 0)))
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 29fb2e1..8e71fcb 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -129,7 +129,7 @@ static int unify (tree, tree, tree, tree, int, bool);
static void add_pending_template (tree);
static tree reopen_tinst_level (struct tinst_level *);
static tree tsubst_initializer_list (tree, tree);
-static tree get_class_bindings (tree, tree, tree, tree);
+static tree get_partial_spec_bindings (tree, tree, tree, tree);
static tree coerce_template_parms (tree, tree, tree, tsubst_flags_t,
bool, bool);
static tree coerce_innermost_template_parms (tree, tree, tree, tsubst_flags_t,
@@ -173,7 +173,7 @@ static tree tsubst_template_arg (tree, tree, tsubst_flags_t, tree);
static tree tsubst_template_args (tree, tree, tsubst_flags_t, tree);
static tree tsubst_template_parms (tree, tree, tsubst_flags_t);
static void regenerate_decl_from_template (tree, tree);
-static tree most_specialized_class (tree
[PATCH 3/4] Merge the ./config directory between GCC and Binutils
This pulls in the missing Binutils pieces into the ./config
directory. Contains these missing Binutils changes:
2014-08-14 Alan Modra
* plugins.m4: Test for dlfcn.h or windows.h here to set default
for --enable-plugins. Report error if someone tries to enable
plugins on a host we don't support.
2014-08-19 Alan Modra
* plugins.m4 (AC_PLUGINS): If plugins are enabled, add -ldl to
LIBS via AC_SEARCH_LIBS.
For this commit:
2014-11-26 Jan-Benedict Glaw
* config/plugins.m4: Merge from Binutils.
---
config/plugins.m4 | 26 ++
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/config/plugins.m4 b/config/plugins.m4
index 7ee8412..513c690 100644
--- a/config/plugins.m4
+++ b/config/plugins.m4
@@ -1,11 +1,21 @@
AC_DEFUN([AC_PLUGINS],
[
-AC_ARG_ENABLE([plugins],
-AS_HELP_STRING([--enable-plugins], [Enable support for plugins (defaults no)]),
-[case "${enableval}" in
- yes | "") plugins=yes ;;
- no) plugins=no ;;
- *) plugins=yes ;;
- esac],
-[plugins=no])
+ maybe_plugins=no
+ AC_CHECK_HEADERS([dlfcn.h], [maybe_plugins=yes], [], [AC_INCLUDES_DEFAULT])
+ AC_CHECK_HEADERS([windows.h], [maybe_plugins=yes], [], [AC_INCLUDES_DEFAULT])
+
+ AC_ARG_ENABLE([plugins],
+AS_HELP_STRING([--enable-plugins], [Enable support for plugins]),
+[case "${enableval}" in
+ no) plugins=no ;;
+ *) plugins=yes
+ if test "$maybe_plugins" != "yes" ; then
+ AC_MSG_ERROR([Building with plugin support requires a host that
supports dlopen.])
+fi ;;
+ esac],
+[plugins=$maybe_plugins]
+ )
+ if test "$plugins" = "yes"; then
+AC_SEARCH_LIBS([dlopen], [dl])
+ fi
])
--
1.8.3.2
signature.asc
Description: Digital signature
[PATCH 4/4] Regenerate ./configure
2014-11-26 Jan-Benedict Glaw
* configure: Regenerate.
---
ChangeLog | 4
configure | 14 --
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index f5ce738..4c8adff 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2014-11-26 Jan-benedict Glaw
+
+ * configure: Regenerate.
+
2014-11-26 Jan-Benedict Glaw
* configure.ac: Merge with Binutils.
diff --git a/configure b/configure
index 96225b5..9e6684c 100755
--- a/configure
+++ b/configure
@@ -3656,6 +3656,10 @@ case "${target}" in
;;
*-*-rtems*)
noconfigdirs="$noconfigdirs target-libgloss"
+# this is not caught below because this stanza matches earlier
+case $target in
+ or1k*-*-*) noconfigdirs="$noconfigdirs gdb" ;;
+esac
;;
# The tpf target doesn't support gdb yet.
*-*-tpf*)
@@ -3831,7 +3835,7 @@ case "${target}" in
microblaze*)
noconfigdirs="$noconfigdirs gprof"
;;
- mips*-sde-elf* | mips*-mti-elf*)
+ mips*-sde-elf* | mips*-mti-elf* | mips*-img-elf*)
if test x$with_newlib = xyes; then
noconfigdirs="$noconfigdirs gprof"
fi
@@ -3854,10 +3858,16 @@ case "${target}" in
mips*-*-*)
noconfigdirs="$noconfigdirs gprof"
;;
+ nds32*-*-*)
+noconfigdirs="$noconfigdirs gdb"
+;;
nvptx*-*-*)
# nvptx is just a compiler
noconfigdirs="$noconfigdirs target-libssp target-libstdc++-v3
target-libobjc"
;;
+ or1k*-*-*)
+noconfigdirs="$noconfigdirs gdb"
+;;
sh-*-* | sh64-*-*)
case "${target}" in
sh*-*-elf)
@@ -6801,7 +6811,7 @@ case "${target}" in
spu-*-*)
target_makefile_frag="config/mt-spu"
;;
- mips*-sde-elf* | mips*-mti-elf*)
+ mips*-sde-elf* | mips*-mti-elf* | mips*-img-elf*)
target_makefile_frag="config/mt-sde"
;;
mipsisa*-*-elfoabi*)
--
1.8.3.2
signature.asc
Description: Digital signature
[PATCH 1/4] Merge toplevel configure.ac bits from Binutils
GCC and Binutils had a common ./configure.ac after this commit:
commit 08d081652f50df83f7e9768f8dbb7a99b0df50a2
Author: sandra
Date: Wed May 14 23:20:59 2014 +
2014-05-14 Sandra Loosemore
* configure.ac (target_makefile_frag): Set for nios2-*-elf*.
* configure: Regenerated.
config/
* mt-nios2-elf: New file.
Based on that file version, all GCC and Binutils commits to this file were
applied. This pulls in the following missing Binutils changes to configure.ac:
2014-07-21 Joel Sherrill
Disable gdb for or1k*-*-* until supported
* configure.ac (or1k*-*-*): Disable gdb.
* configure: Regenerated.
2014-07-27 Joel Sherrill
GDB not supported for or1k*-*-rtems*
* configure.ac (or1k*-*-rtems*): gdb not supported. The ordering
of the stanzas results in this not being caught by or1k*-*-* later.
* configure. Regenerated.
2014-09-06 Kuan-Lin Chen
* configure: Disable gdb for nds32*-*-* until supported.
* configure.ac: Disable gdb for nds32*-*-* until supported.
2014-09-12 Andrew Bennett
* configure.ac: Add mips*-img-elf* target triple.
* configure: Regenerate.
For this patch:
2014-11-26 Jan-Benedict Glaw
* configure.ac: Merge with Binutils.
---
ChangeLog| 4
configure.ac | 14 --
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 52b7bc8..f5ce738 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2014-11-26 Jan-Benedict Glaw
+
+ * configure.ac: Merge with Binutils.
+
2014-11-26 Tobias Burnus
* Makefile.def: Make more dependent on mpfr, mpc and isl.
diff --git a/configure.ac b/configure.ac
index b27fb1d..fd1bdf0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1010,6 +1010,10 @@ case "${target}" in
;;
*-*-rtems*)
noconfigdirs="$noconfigdirs target-libgloss"
+# this is not caught below because this stanza matches earlier
+case $target in
+ or1k*-*-*) noconfigdirs="$noconfigdirs gdb" ;;
+esac
;;
# The tpf target doesn't support gdb yet.
*-*-tpf*)
@@ -1185,7 +1189,7 @@ case "${target}" in
microblaze*)
noconfigdirs="$noconfigdirs gprof"
;;
- mips*-sde-elf* | mips*-mti-elf*)
+ mips*-sde-elf* | mips*-mti-elf* | mips*-img-elf*)
if test x$with_newlib = xyes; then
noconfigdirs="$noconfigdirs gprof"
fi
@@ -1208,10 +1212,16 @@ case "${target}" in
mips*-*-*)
noconfigdirs="$noconfigdirs gprof"
;;
+ nds32*-*-*)
+noconfigdirs="$noconfigdirs gdb"
+;;
nvptx*-*-*)
# nvptx is just a compiler
noconfigdirs="$noconfigdirs target-libssp target-libstdc++-v3
target-libobjc"
;;
+ or1k*-*-*)
+noconfigdirs="$noconfigdirs gdb"
+;;
sh-*-* | sh64-*-*)
case "${target}" in
sh*-*-elf)
@@ -2454,7 +2464,7 @@ case "${target}" in
spu-*-*)
target_makefile_frag="config/mt-spu"
;;
- mips*-sde-elf* | mips*-mti-elf*)
+ mips*-sde-elf* | mips*-mti-elf* | mips*-img-elf*)
target_makefile_frag="config/mt-sde"
;;
mipsisa*-*-elfoabi*)
--
1.8.3.2
signature.asc
Description: Digital signature
[PATCH 2/4] Merge ./config/ChangeLog with missing entries from Binutils
2014-11-26 Jan-Benedict Glaw * config/ChangeLog: Merge entries from Binutils --- config/ChangeLog | 15 +++ 1 file changed, 15 insertions(+) diff --git a/config/ChangeLog b/config/ChangeLog index ab3a773..2cbc885 100644 --- a/config/ChangeLog +++ b/config/ChangeLog @@ -26,11 +26,22 @@ * isl.m4 (ISL_CHECK_VERSION): Check link of isl library for cross_compiling. +2014-08-19 Alan Modra + + * plugins.m4 (AC_PLUGINS): If plugins are enabled, add -ldl to + LIBS via AC_SEARCH_LIBS. + 2014-08-18 Roman Gareev * cloog.m4: Remove the path to isllibs from clooglibs. * isl.m4: Add paths to islinc, isllibs. +2014-08-14 Alan Modra + + * plugins.m4: Test for dlfcn.h or windows.h here to set default + for --enable-plugins. Report error if someone tries to enable + plugins on a host we don't support. + 2014-07-26 Uros Bizjak PR target/47230 @@ -45,6 +56,10 @@ PR target/43538 * mt-gnu: Don't reset CXXFLAGS_FOR_TARGET. +2013-12-07 Mike Frysinger + + * acinclude.m4: Remove +x file mode. + 2013-11-29 Marek Polacek * bootstrap-ubsan.mk (POSTSTAGE1_LDFLAGS): Remove -lpthread -ldl. -- 1.8.3.2 signature.asc Description: Digital signature
Re: Merging configure bits with Binutils
On Mon, 2014-11-24 12:43:40 -0700, Jeff Law wrote: [Merging top-level configury files from Binutils] > It's pretty isolated stuff (in terms of what clauses are being > changed in the configure bits), so I'm not too worried about > breaking anything. We can reach out to Joel to ensure the RTEMS > bits are correct and those were, IMHO, the most important. > > I think we're early enough in stage3 that this can go forward. So I rebased the patches, regenerated ./configure and will commit the following set of patches soon. MfG, JBG -- Jan-Benedict Glaw jbg...@lug-owl.de +49-172-7608481 Signature of: They that give up essential liberty to obtain temporary safety, the second : deserve neither liberty nor safety. (Ben Franklin) signature.asc Description: Digital signature
Re: [C/C++ PATCH] Don't convert RHS of a shift-expression to int (PR c/63862)
On Wed, Nov 26, 2014 at 10:20:39PM +0100, Richard Biener wrote: > Well, if you want to aggressively prune unused bits then you could > "back-propagate" the shift count value-range. > > And note that all the frontend shorten-optimizations should move to the > middle-end. > > That said, instead of casting to int we could cast to unsigned char... As long as we don't have > 256 bitsize integers, yes, for the purposes of GIMPLE. Though, as with type demotion, then it might be worthwhile to try to promote it to wider type if the shift instructions operate on wider shift count modes than QImode. Jakub
RE: [patch, arm] align saved FP regs on stack
Hi, Sandra. FWIW, I tried this patch on A15 Juno with Coremark and any difference, if any, between specifying this option and not was below 1%. Cheers, -- Evandro Menezes Austin, TX > -Original Message- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-ow...@gcc.gnu.org] On > Behalf Of Sandra Loosemore > Sent: Friday, November 14, 2014 18:47 > To: GCC Patches > Cc: Chris Jones; Joshua Conner > Subject: [patch, arm] align saved FP regs on stack > > On ARM targets, the stack is aligned to an 8-byte boundary, but when > saving/restoring the VFP coprocessor registers in the function > prologue/epilogue, it is possible for the 8-byte values to end up at > locations that are 4-byte aligned but not 8-byte aligned. This can result in > a performance penalty on micro-architectures that are optimized for well- > aligned data, especially when such a misalignment may result in cache line > splits within a single access. This patch detects when at least one > coprocessor register value needs to be saved and adds some additional padding > to the stack at that point if necessary to align it to an 8-byte boundary. > I've re-used the existing logic to try pushing a 4-byte scratch register and > only fall back to an explicit stack adjustment if that fails. > > NVIDIA found that an earlier version of this patch (benchmarked with > SPECint2k and SPECfp2k on an older version of GCC) gave measurable > improvements on their Tegra K1 64-bit processor, aka "Denver". We aren't > sure what other ARM processors might benefit from the extra alignment, so > we've given it its own command-line option instead of tying it to -mtune. > > I did some hand-testing of this patch on small test cases to verify that the > expected alignment was happening, but it seemed to me that the expected > assembly-language patterns were likely too fragile to be hard-wired into a > test case. I also ran regression tests both with and without the switch set > so it doesn't break other things. OK to commit? > > -Sandra
Re: [C/C++ PATCH] Don't convert RHS of a shift-expression to int (PR c/63862)
On November 26, 2014 8:33:52 PM CET, Jakub Jelinek wrote:
>On Wed, Nov 26, 2014 at 02:25:54PM -0500, Jason Merrill wrote:
>> How about converting the rhs to unsigned int if it is already
>unsigned?
>
>That is fine. I'm just worried about the casts to wider types.
>So perhaps just promote and cast to int if the promoted type is
>signed or unsigned if it is unsigned?
>
>E.g. if you have
>long long foo (long long x, long long y, long long z)
>{
> return x << (y * z);
>}
>and long long multiplication requires say a libcall, while
>int * int can be expanded inline, using long long for shift
>count penalizes the code unnecessarily. Ditto for
>long long foo (void) __attribute__((const));
>int
>bar (int x)
>{
> return x << (foo () & ~0xLL);
>}
>and similar. I know, we are running here back into the sadly :(
>still missing type demotion and promotion passes, but changing this
>now when those passes are missing can only hurt code.
Well, if you want to aggressively prune unused bits then you could
"back-propagate" the shift count value-range.
And note that all the frontend shorten-optimizations should move to the
middle-end.
That said, instead of casting to int we could cast to unsigned char...
Richard.
>
> Jakub
Re: [C/C++ PATCH] Don't convert RHS of a shift-expression to int (PR c/63862)
On November 26, 2014 8:25:54 PM CET, Jason Merrill wrote: >How about converting the rhs to unsigned int if it is already unsigned? Does the standard say so? I think not. IMHO it's not the front ends business to do this. And I don't see how it helps either. Richard. >Jason
Re: [PATCH v4] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value
On Thu, 27 Nov 2014, Chen Gang wrote: > The original length 18 is not enough for HOST_WIDE_INT printing, need > use 20 instead of. > > Also need additional bytes for printing related prefix and suffix, and > give a related check. > > It passes testsuite under fedora 20 x86_64-unknown-linux-gnu. > > 2014-11-27 Chen Gang > > * c-family/c-cppbuiltin.c (builtin_define_with_int_value): Let > buffer enough to print host wide integer value. OK. Properly implementing the (-9223372036854775807LL-1) and similar cases (when the value is the least int, long or long long on the target) can be a followup fix. -- Joseph S. Myers jos...@codesourcery.com
[PATCH v4] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value
The original length 18 is not enough for HOST_WIDE_INT printing, need
use 20 instead of.
Also need additional bytes for printing related prefix and suffix, and
give a related check.
It passes testsuite under fedora 20 x86_64-unknown-linux-gnu.
2014-11-27 Chen Gang
* c-family/c-cppbuiltin.c (builtin_define_with_int_value): Let
buffer enough to print host wide integer value.
---
gcc/c-family/c-cppbuiltin.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
index c571d1b..92bc06d 100644
--- a/gcc/c-family/c-cppbuiltin.c
+++ b/gcc/c-family/c-cppbuiltin.c
@@ -1366,14 +1366,22 @@ static void
builtin_define_with_int_value (const char *macro, HOST_WIDE_INT value)
{
char *buf;
- size_t mlen = strlen (macro);
- size_t vlen = 18;
- size_t extra = 2; /* space for = and NUL. */
-
- buf = (char *) alloca (mlen + vlen + extra);
- memcpy (buf, macro, mlen);
- buf[mlen] = '=';
- sprintf (buf + mlen + 1, HOST_WIDE_INT_PRINT_DEC, value);
+ size_t vlen = 20; /* maximize value length: -9223372036854775807 */
+ size_t extra = 6; /* space for =, NUL, (, ), and L L. */
+
+ gcc_assert (wi::fits_to_tree_p (value, long_long_integer_type_node));
+
+ buf = (char *) alloca (strlen (macro) + vlen + extra);
+
+ sprintf (buf, "%s=%s" HOST_WIDE_INT_PRINT_DEC "%s%s",
+ macro,
+ value < 0 ? "(" : "",
+ value,
+ wi::fits_to_tree_p (value, integer_type_node)
+ ? ""
+ : wi::fits_to_tree_p (value, long_integer_type_node)
+? "L" : "LL",
+ value < 0 ? ")" : "");
cpp_define (parse_in, buf);
}
--
1.9.3
Re: [PATCH] Allow -fsanitize=thread without -pie
Looks good to me.
Thanks!
On Wed, Nov 26, 2014 at 10:43 PM, Jakub Jelinek wrote:
> On Fri, Nov 21, 2014 at 04:20:44PM +0400, Dmitry Vyukov wrote:
>> Yes, I think it's the way to go.
>> I've just committed the following revision to clang that removes -pie
>> when compiling with tsan:
>> http://llvm.org/viewvc/llvm-project?view=revision&revision=222526
>> The tests in llvm tree pass with this change.
>
> Ok, here it is. I think -static still doesn't work, for static linking
> symbols might not be interposed as libtsan wants, so I've changed it
> to match what we do for -fsanitize=address.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2014-11-26 Jakub Jelinek
>
> * gcc.c (SANITIZER_SPEC): Don't error on -fsanitize=thread
> without -pie or -shared, error on -fsanitize=thread -static instead.
>
> * lib/tsan-dg.exp (check_effective_target_fsanitize_thread,
> tsan_init): Don't use -fPIE or -pie.
>
> --- gcc/gcc.c.jj2014-11-26 11:08:51.0 +0100
> +++ gcc/gcc.c 2014-11-26 15:59:12.034729813 +0100
> @@ -794,7 +794,7 @@ proper position among the other output f
> %{!nostdlib:%{!nodefaultlibs:%{%:sanitize(address):" LIBASAN_SPEC "\
> %{static:%ecannot specify -static with -fsanitize=address}}\
> %{%:sanitize(thread):" LIBTSAN_SPEC "\
> -%{!pie:%{!shared:%e-fsanitize=thread linking must be done with -pie or
> -shared}}}\
> +%{static:%ecannot specify -static with -fsanitize=thread}}\
> %{%:sanitize(undefined):" LIBUBSAN_SPEC "}\
> %{%:sanitize(leak):" LIBLSAN_SPEC "}}}"
> #endif
> --- gcc/testsuite/lib/tsan-dg.exp.jj2014-10-10 19:42:16.0 +0200
> +++ gcc/testsuite/lib/tsan-dg.exp 2014-11-26 16:02:21.663347886 +0100
> @@ -18,9 +18,9 @@
> # code, 0 otherwise.
>
> proc check_effective_target_fsanitize_thread {} {
> -return [check_no_compiler_messages fanitize_thread executable {
> +return [check_no_compiler_messages fsanitize_thread executable {
> int main (void) { return 0; }
> -} "-fPIE -pie -fsanitize=thread"]
> +} "-fsanitize=thread"]
> }
>
> #
> @@ -93,12 +93,12 @@ proc tsan_init { args } {
> if [info exists ALWAYS_CXXFLAGS] {
> set tsan_saved_ALWAYS_CXXFLAGS $ALWAYS_CXXFLAGS
> set ALWAYS_CXXFLAGS [concat "{ldflags=$link_flags}" $ALWAYS_CXXFLAGS]
> - set ALWAYS_CXXFLAGS [concat "{additional_flags=-fPIE -pie
> -fsanitize=thread -g}" $ALWAYS_CXXFLAGS]
> + set ALWAYS_CXXFLAGS [concat "{additional_flags=-fsanitize=thread -g}"
> $ALWAYS_CXXFLAGS]
> } else {
> if [info exists TEST_ALWAYS_FLAGS] {
> - set TEST_ALWAYS_FLAGS "$link_flags -fPIE -pie -fsanitize=thread
> -g $TEST_ALWAYS_FLAGS"
> + set TEST_ALWAYS_FLAGS "$link_flags -fsanitize=thread -g
> $TEST_ALWAYS_FLAGS"
> } else {
> - set TEST_ALWAYS_FLAGS "$link_flags -fPIE -pie -fsanitize=thread
> -g"
> + set TEST_ALWAYS_FLAGS "$link_flags -fsanitize=thread -g"
> }
> }
>
> @@ -110,7 +110,7 @@ proc tsan_init { args } {
> set individual_timeout 20
> if [check_runtime_nocache tsan_works {
> int main () { return 0; }
> - } "-fPIE -pie -fsanitize=thread -g"] {
> + } "-fsanitize=thread -g"] {
> set dg-do-what-default run
> } else {
> set dg-do-what-default compile
>
>
> Jakub
[PATCH] Allow -fsanitize=thread without -pie
On Fri, Nov 21, 2014 at 04:20:44PM +0400, Dmitry Vyukov wrote:
> Yes, I think it's the way to go.
> I've just committed the following revision to clang that removes -pie
> when compiling with tsan:
> http://llvm.org/viewvc/llvm-project?view=revision&revision=222526
> The tests in llvm tree pass with this change.
Ok, here it is. I think -static still doesn't work, for static linking
symbols might not be interposed as libtsan wants, so I've changed it
to match what we do for -fsanitize=address.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2014-11-26 Jakub Jelinek
* gcc.c (SANITIZER_SPEC): Don't error on -fsanitize=thread
without -pie or -shared, error on -fsanitize=thread -static instead.
* lib/tsan-dg.exp (check_effective_target_fsanitize_thread,
tsan_init): Don't use -fPIE or -pie.
--- gcc/gcc.c.jj2014-11-26 11:08:51.0 +0100
+++ gcc/gcc.c 2014-11-26 15:59:12.034729813 +0100
@@ -794,7 +794,7 @@ proper position among the other output f
%{!nostdlib:%{!nodefaultlibs:%{%:sanitize(address):" LIBASAN_SPEC "\
%{static:%ecannot specify -static with -fsanitize=address}}\
%{%:sanitize(thread):" LIBTSAN_SPEC "\
-%{!pie:%{!shared:%e-fsanitize=thread linking must be done with -pie or
-shared}}}\
+%{static:%ecannot specify -static with -fsanitize=thread}}\
%{%:sanitize(undefined):" LIBUBSAN_SPEC "}\
%{%:sanitize(leak):" LIBLSAN_SPEC "}}}"
#endif
--- gcc/testsuite/lib/tsan-dg.exp.jj2014-10-10 19:42:16.0 +0200
+++ gcc/testsuite/lib/tsan-dg.exp 2014-11-26 16:02:21.663347886 +0100
@@ -18,9 +18,9 @@
# code, 0 otherwise.
proc check_effective_target_fsanitize_thread {} {
-return [check_no_compiler_messages fanitize_thread executable {
+return [check_no_compiler_messages fsanitize_thread executable {
int main (void) { return 0; }
-} "-fPIE -pie -fsanitize=thread"]
+} "-fsanitize=thread"]
}
#
@@ -93,12 +93,12 @@ proc tsan_init { args } {
if [info exists ALWAYS_CXXFLAGS] {
set tsan_saved_ALWAYS_CXXFLAGS $ALWAYS_CXXFLAGS
set ALWAYS_CXXFLAGS [concat "{ldflags=$link_flags}" $ALWAYS_CXXFLAGS]
- set ALWAYS_CXXFLAGS [concat "{additional_flags=-fPIE -pie
-fsanitize=thread -g}" $ALWAYS_CXXFLAGS]
+ set ALWAYS_CXXFLAGS [concat "{additional_flags=-fsanitize=thread -g}"
$ALWAYS_CXXFLAGS]
} else {
if [info exists TEST_ALWAYS_FLAGS] {
- set TEST_ALWAYS_FLAGS "$link_flags -fPIE -pie -fsanitize=thread -g
$TEST_ALWAYS_FLAGS"
+ set TEST_ALWAYS_FLAGS "$link_flags -fsanitize=thread -g
$TEST_ALWAYS_FLAGS"
} else {
- set TEST_ALWAYS_FLAGS "$link_flags -fPIE -pie -fsanitize=thread -g"
+ set TEST_ALWAYS_FLAGS "$link_flags -fsanitize=thread -g"
}
}
@@ -110,7 +110,7 @@ proc tsan_init { args } {
set individual_timeout 20
if [check_runtime_nocache tsan_works {
int main () { return 0; }
- } "-fPIE -pie -fsanitize=thread -g"] {
+ } "-fsanitize=thread -g"] {
set dg-do-what-default run
} else {
set dg-do-what-default compile
Jakub
Re: [Build, Graphite] PR64017 - support ISL-0.14 (gcc/configure.ac and gcc/graphite*.c)
On 11/26/14 09:27, Tobias Burnus wrote: On Wed, Nov 26, 2014 at 04:55:40PM +0100, Tobias Burnus wrote: >This patch adds a configure check for isl_schedule_constraints_compute_schedule, >which is new in ISL 0.13 - and uses it for conditional compilation. Repeating the test on a massively multicore system, it fails at stage2 with an in-tree 0.14 because -lisl cannot be found at gcc/configure time. Hence, HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE is false and one gets later an error because a 0.12.2 function is not available with ISL 0.14. However, running the configure test case manually - or building with fewer cores - works. As does removing "gcc" after the failure such that isl/ is available. Thus, it looks like a dependency problem. It seems to be fixed by the attached patch. OK for the trunk? Tobias g3.diff 2014-11-26 Tobias Burnus PR middle-end/64017 * Makefile.def: Make more dependent on mpfr, mpc and isl. * Makefile.in: Regenerate. This is OK for the trunk. jeff
[committed] Make 3 libgomp examples-4 tests cheaper
Hi!
I've looked at which tests are still too expensive
on heavily loaded many thread configurations (where other threads
during normal testing run many other tests), and found these:
The patch attempts to limit number of target or parallel regions
to something smaller than hundreds or thousands.
In the Fortran test, I haven't understand why the Fortran version
uses so much larger arrays than corresponding C test.
Regtested on x86_64-linux and i686-linux, committed to trunk.
2014-11-26 Jakub Jelinek
* testsuite/libgomp.c/examples-4/e.53.4.c: Add -DITESTITERS=20
to dg-options unless expensive testing is on.
(TESTITERS): Define to N if not defined.
(main): Use TESTITERS instead of N.
* testsuite/libgomp.c/examples-4/e.55.1.c: Define CHUNKSZ from
dg-additional-options depending on whether expensive testing is on.
* testsuite/libgomp.fortran/examples-4/e.55.1.f90 (e_55_1_mod):
Decrease N to 10 and CHUNKSZ to 1.
--- libgomp/testsuite/libgomp.c/examples-4/e.53.4.c.jj 2014-11-13
15:13:18.0 +0100
+++ libgomp/testsuite/libgomp.c/examples-4/e.53.4.c 2014-11-26
11:50:00.092550436 +0100
@@ -1,9 +1,13 @@
/* { dg-do run } */
+/* { dg-additional-options "-DTESTITERS=20" { target { ! run_expensive_tests }
} } */
#include
#define EPS 0.1
#define N 1000
+#ifndef TESTITERS
+#define TESTITERS N
+#endif
#pragma omp declare target
float Q[N][N];
@@ -60,7 +64,7 @@ int main ()
#pragma omp target update to(Q)
- for (i = 0; i < N; i++)
+ for (i = 0; i < TESTITERS; i++)
check (accum (i), accum_ref (i));
return 0;
--- libgomp/testsuite/libgomp.c/examples-4/e.55.1.c.jj 2014-11-13
15:13:18.0 +0100
+++ libgomp/testsuite/libgomp.c/examples-4/e.55.1.c 2014-11-26
11:52:16.206153938 +0100
@@ -1,10 +1,11 @@
/* { dg-do run } */
+/* { dg-additional-options "-DCHUNKSZ=5000" { target { ! run_expensive_tests }
} } */
+/* { dg-additional-options "-DCHUNKSZ=1000" { target run_expensive_tests } } */
#include
#define EPS 0.1
#define N 10
-#define CHUNKSZ 1000
float Y[N];
float Z[N];
--- libgomp/testsuite/libgomp.fortran/examples-4/e.55.1.f90.jj 2014-11-13
15:13:17.0 +0100
+++ libgomp/testsuite/libgomp.fortran/examples-4/e.55.1.f90 2014-11-26
12:00:03.531907670 +0100
@@ -1,7 +1,7 @@
! { dg-do run }
module e_55_1_mod
- integer, parameter :: N = 1000, CHUNKSZ = 10
+ integer, parameter :: N = 10, CHUNKSZ = 1
real :: Y(N), Z(N)
end module
Jakub
Re: [C/C++ PATCH] Don't convert RHS of a shift-expression to int (PR c/63862)
On Wed, Nov 26, 2014 at 02:25:54PM -0500, Jason Merrill wrote:
> How about converting the rhs to unsigned int if it is already unsigned?
That is fine. I'm just worried about the casts to wider types.
So perhaps just promote and cast to int if the promoted type is
signed or unsigned if it is unsigned?
E.g. if you have
long long foo (long long x, long long y, long long z)
{
return x << (y * z);
}
and long long multiplication requires say a libcall, while
int * int can be expanded inline, using long long for shift
count penalizes the code unnecessarily. Ditto for
long long foo (void) __attribute__((const));
int
bar (int x)
{
return x << (foo () & ~0xLL);
}
and similar. I know, we are running here back into the sadly :(
still missing type demotion and promotion passes, but changing this
now when those passes are missing can only hurt code.
Jakub
Re: [C++ Patch/RFC] PR 63757
OK. Jason
Re: [Patch, Fortran] convert almost all {warning,error}_now to common diagnostic
Hi Manuel,
Manuel López-Ibáñez wrote:
I think the changes to the common diagnostics part could be as simple as:
Good – that's simpler than I had feared.
However, I'm less familiar with the Fortran buffering mechanism.
I am not sure you find someone who really is. At least I also have
troubles – but most of the time it works as expected.
An implementation criterion would be that it doesn't regress, except
that changes which do make sense (are better or equivalent) are still fine.
It doesn't have to be completely identical.
So far, I understood that:
* gfc uses two gfc_error_buf (error_buffer, warning_buffer) and
pointer *cur_error_buffer; to switch between them. These are
equivalent to pretty-printer output_buffer, thus my idea is to create
a gfc_pretty_printer that contains three output_buffer (two for
buffering and another for the _now variants).
* There seems to be a global buffer_flag that controls when to buffer.
However, each gfc_error_buf has a variable "flag", that is also
checked sometimes. Unfortunately, nothing in gfc_error_buf has
comments, so I have no idea how these flags are supposed to interact.
It would be great if a Fortran person could explain it in detail.
Well gfc_error_buf's "flag" seems to be 1 if there is a buffered warning
– and it is set to 0 if either the warning has been printed or dropped
(e.g. via gfc_clear_error()). Thus, when
gfc_error_check()/gfc_warning_check() is called, the buffer can be
printed ("fputs (warning_buffer.message, stderr);").
The only reason that "warning_buffer.message" isn't directly used seems
to be that that way the memory allocation for the buffer can be reused.
The buffering itself seems to be only controlled by "buffer_flag".
* I am not sure how to handle the error/warnings counts. The Fortran
code seems to not care about how many errors or warnings are given
when buffering, only whether there was any error at all. Is that
right? It also seems to not care about -fmax-errors (similarly
-Wfatal-errors will only take care after flushing, thus potentially
many errors can be given before stopping). Is this correct? Not having
to handle those would simplify a first version a lot.
Looking at the code, newer errors even seem to always override previous
ones:
The gfc_{error,warning}{,_now_1} functions all have code like:
error_buffer.index = 0;
which effectively clears the error buffer by overriding it with the new
string.
Thus, one has only to take care of incrementing the count when actually
printing the buffer.
* I assume that ICEs and other such fatal_errors are never buffered.
For normal errors converted to fatal by Wfatal-errors, I just need to
be careful in the buffered variants, that is disable it before going
through the common machinery and re-enable it after returning from it.
(This is a note to myself).
Tobias
Re: [C/C++ PATCH] Don't convert RHS of a shift-expression to int (PR c/63862)
How about converting the rhs to unsigned int if it is already unsigned? Jason
Re: [C++ Patch/RFC] PR 63757
Hi,
On 11/26/2014 05:24 PM, Jason Merrill wrote:
On 11/24/2014 01:55 PM, Paolo Carlini wrote:
in this rejects-valid, as part of build_user_type_conversion_1,
standard_conversion is called by implicit_conversion with a *null* expr,
thus the condition in standard_conversion
/* [conv.ptr]
A null pointer constant can be converted to a pointer type; ... A
null pointer constant of integral type can be converted to an
rvalue of type std::nullptr_t. */
if ((tcode == POINTER_TYPE || TYPE_PTRMEM_P (to)
|| NULLPTR_TYPE_P (to))
&& expr && null_ptr_cst_p (expr))
conv = build_conv (ck_std, to, conv);
is false and the snippet is rejected. Should we pass a nullptr_node as
expr in such cases, ie, when handling conversions functions returning
std::nullptr_t?!?
I'd prefer to change the test quoted above to not require expr to be
non-null in the case of NULLPTR_TYPE_P.
Oh good, I was unsure about that. The below also passes testing.
Thanks,
Paolo.
///
Index: cp/call.c
===
--- cp/call.c (revision 218089)
+++ cp/call.c (working copy)
@@ -1194,7 +1194,8 @@ standard_conversion (tree to, tree from, tree expr
rvalue of type std::nullptr_t. */
if ((tcode == POINTER_TYPE || TYPE_PTRMEM_P (to)
|| NULLPTR_TYPE_P (to))
- && expr && null_ptr_cst_p (expr))
+ && ((expr && null_ptr_cst_p (expr))
+ || NULLPTR_TYPE_P (from)))
conv = build_conv (ck_std, to, conv);
else if ((tcode == INTEGER_TYPE && fcode == POINTER_TYPE)
|| (tcode == POINTER_TYPE && fcode == INTEGER_TYPE))
Index: testsuite/g++.dg/cpp0x/nullptr33.C
===
--- testsuite/g++.dg/cpp0x/nullptr33.C (revision 0)
+++ testsuite/g++.dg/cpp0x/nullptr33.C (working copy)
@@ -0,0 +1,19 @@
+// PR c++/63757
+// { dg-do compile { target c++11 } }
+
+typedef decltype(nullptr) nullptr_t;
+
+void bar(void*) {}
+
+struct foo
+{
+ operator nullptr_t()
+ {
+return nullptr;
+ }
+};
+
+int main()
+{
+ bar(foo());
+}
[PATCH] Fix expansion with COMPOUND_LITERAL_EXPR (PR middle-end/64067)
Hi!
The testcase shows that when expanding ARRAY_REF from a const static var
with initializer that contains COMPOUND_LITERAL_EXPR, we can try to expand
COMPOUND_LITERAL_EXPR even when modifier is not EXPAND_INITIALIZER (it is
EXPAND_SUM in that testcase, but could be many others).
While gimplification should get rid of all the compound literal exprs
for automatic compound literals, COMPOUND_LITERAL_EXPR for static compound
literals can survive in the initializers, thus this patch handles them
always.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2014-11-26 Jakub Jelinek
PR middle-end/64067
* expr.c (expand_expr_addr_expr_1) :
Handle it by returning address of COMPOUND_LITERAL_EXPR_DECL
not only if modifier is EXPAND_INITIALIZER, but whenever
COMPOUND_LITERAL_EXPR_DECL is non-NULL and TREE_STATIC.
* gcc.c-torture/compile/pr64067.c: New test.
--- gcc/expr.c.jj 2014-11-18 08:26:45.0 +0100
+++ gcc/expr.c 2014-11-26 17:38:52.877675088 +0100
@@ -7677,11 +7677,13 @@ expand_expr_addr_expr_1 (tree exp, rtx t
break;
case COMPOUND_LITERAL_EXPR:
- /* Allow COMPOUND_LITERAL_EXPR in initializers, if e.g.
-rtl_for_decl_init is called on DECL_INITIAL with
-COMPOUNT_LITERAL_EXPRs in it, they aren't gimplified. */
- if (modifier == EXPAND_INITIALIZER
- && COMPOUND_LITERAL_EXPR_DECL (exp))
+ /* Allow COMPOUND_LITERAL_EXPR in initializers or coming from
+initializers, if e.g. rtl_for_decl_init is called on DECL_INITIAL
+with COMPOUND_LITERAL_EXPRs in it, or ARRAY_REF on a const static
+array with address of COMPOUND_LITERAL_EXPR in DECL_INITIAL;
+the initializers aren't gimplified. */
+ if (COMPOUND_LITERAL_EXPR_DECL (exp)
+ && TREE_STATIC (COMPOUND_LITERAL_EXPR_DECL (exp)))
return expand_expr_addr_expr_1 (COMPOUND_LITERAL_EXPR_DECL (exp),
target, tmode, modifier, as);
/* FALLTHRU */
--- gcc/testsuite/gcc.c-torture/compile/pr64067.c.jj2014-11-26
17:37:37.835865797 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr64067.c 2014-11-26
17:37:02.0 +0100
@@ -0,0 +1,10 @@
+/* PR middle-end/64067 */
+
+struct S { int s; };
+int *const v[1] = { &((struct S) { .s = 42 }).s };
+
+int *
+foo (void)
+{
+ return v[0];
+}
Jakub
Re: [C/C++ PATCH] Don't convert RHS of a shift-expression to int (PR c/63862)
On November 26, 2014 7:49:55 PM CET, Jakub Jelinek wrote:
>On Wed, Nov 26, 2014 at 07:20:04PM +0100, Marek Polacek wrote:
>> On Wed, Nov 26, 2014 at 06:50:29PM +0100, Jakub Jelinek wrote:
>> > On Wed, Nov 26, 2014 at 06:39:44PM +0100, Marek Polacek wrote:
>> > > Both C11 and C++14 standards specify that integral promotions are
>> > > performed on both operands of a shift-expression. This we do
>just
>> > > fine. But then we convert the right operand to
>integer_type_node.
>> > > Not only is this unnecessary, it can also be harfmul, because for
>> > > e.g.
>> > > void
>> > > foo (unsigned int x)
>> > > {
>> > > if (-1 >> x != -1)
>
>I'd say fold-const not simplifying this is a bug even if x is signed
>int.
>I think negative shift counts are undefined even in the middle-end,
>Richard?
Well, in the past it has, at least for constant folding, pollibly only in
into_const_binop.
>Treating 5 >> -5 as 5 << 5 looks just wrong to me, does any FE rely on
>that?
Not sure.
>I don't think the expander will expand it that way though.
Not sure :)
>Anyway, if the shift count is unnecessary wide type, then the
>middle-end
>won't be able to optimize it as much as it could if it has been a
>narrower
>type.
>
>So, for ubsan purposes (but, shifts are instrumented in the FEs) it is
>of
>course desirable to see the original type, but perhaps it might be a
>good
>idea to cast the shift count to integer_type_node say during
>gimplification
>(perhaps in c_gimplify_expr?).
These are all separate issues but I'd rather get rid of conversions than adding
some.
So I think the original patch is OK.
Richard.
> Jakub
[PATCH] Fix simd clone vectorization (PR tree-optimization/64024)
Hi!
As discussed in the PR and on IRC, the problem here is that peeling
for alignment can for some linear argument that during vect analysis
passed simple_iv no longer pass it during vect transform phase.
So, to fix this, this patch remembers the base and step values from
simple_iv during vect analysis and uses them during transform phase
(biased by what the peeling for alignment advanced of course).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2014-11-26 Jakub Jelinek
PR tree-optimization/64024
* tree-vectorizer.h (struct _stmt_vec_info): Remove simd_clone_fndecl
field. Add simd_clone_info field.
(STMT_VINFO_SIMD_CLONE_FNDECL): Remove.
(STMT_VINFO_SIMD_CLONE_INFO): Define.
* tree-vect-stmts.c (vectorizable_simd_clone_call): Adjust for
STMT_VINFO_SIMD_CLONE_FNDECL becoming first element of
STMT_VINFO_SIMD_CLONE_INFO vector. For linear arguments, remember
base and linear_step from analysis phase and use it during transform
phase, biased by the difference between LOOP_VINFO_NITERS{_UNCHANGED,}
multiplied by linear_step.
(free_stmt_vec_info): Release STMT_VINFO_SIMD_CLONE_INFO.
* gcc.dg/vect/vect-simd-clone-13.c: New test.
* gcc.dg/vect/vect-simd-clone-14.c: New test.
--- gcc/tree-vectorizer.h.jj2014-11-19 18:48:07.0 +0100
+++ gcc/tree-vectorizer.h 2014-11-26 12:56:00.899824766 +0100
@@ -602,8 +602,10 @@ typedef struct _stmt_vec_info {
of this stmt. */
vec same_align_refs;
- /* Selected SIMD clone's function decl. */
- tree simd_clone_fndecl;
+ /* Selected SIMD clone's function info. First vector element
+ is SIMD clone's function decl, followed by a pair of trees (base + step)
+ for linear arguments (pair of NULLs for other arguments). */
+ vec simd_clone_info;
/* Classify the def of this stmt. */
enum vect_def_type def_type;
@@ -677,7 +679,7 @@ typedef struct _stmt_vec_info {
#define STMT_VINFO_RELATED_STMT(S) (S)->related_stmt
#define STMT_VINFO_PATTERN_DEF_SEQ(S) (S)->pattern_def_seq
#define STMT_VINFO_SAME_ALIGN_REFS(S) (S)->same_align_refs
-#define STMT_VINFO_SIMD_CLONE_FNDECL(S) (S)->simd_clone_fndecl
+#define STMT_VINFO_SIMD_CLONE_INFO(S) (S)->simd_clone_info
#define STMT_VINFO_DEF_TYPE(S) (S)->def_type
#define STMT_VINFO_GROUP_FIRST_ELEMENT(S) (S)->first_element
#define STMT_VINFO_GROUP_NEXT_ELEMENT(S) (S)->next_element
--- gcc/tree-vect-stmts.c.jj2014-11-19 18:47:59.0 +0100
+++ gcc/tree-vect-stmts.c 2014-11-26 15:38:59.883409014 +0100
@@ -2715,12 +2715,40 @@ vectorizable_simd_clone_call (gimple stm
else
gcc_assert (thisarginfo.vectype != NULL_TREE);
- if (thisarginfo.dt != vect_constant_def
- && thisarginfo.dt != vect_external_def
- && loop_vinfo
- && TREE_CODE (op) == SSA_NAME
- && simple_iv (loop, loop_containing_stmt (stmt), op, &iv, false)
- && tree_fits_shwi_p (iv.step))
+ /* For linear arguments, the analyze phase should have saved
+the base and step in STMT_VINFO_SIMD_CLONE_INFO. */
+ if (i * 2 + 3 <= STMT_VINFO_SIMD_CLONE_INFO (stmt_info).length ()
+ && STMT_VINFO_SIMD_CLONE_INFO (stmt_info)[i * 2 + 2])
+ {
+ gcc_assert (vec_stmt);
+ thisarginfo.linear_step
+ = tree_to_shwi (STMT_VINFO_SIMD_CLONE_INFO (stmt_info)[i * 2 + 2]);
+ thisarginfo.op
+ = STMT_VINFO_SIMD_CLONE_INFO (stmt_info)[i * 2 + 1];
+ /* If loop has been peeled for alignment, we need to adjust it. */
+ tree n1 = LOOP_VINFO_NITERS_UNCHANGED (loop_vinfo);
+ tree n2 = LOOP_VINFO_NITERS (loop_vinfo);
+ if (n1 != n2)
+ {
+ tree bias = fold_build2 (MINUS_EXPR, TREE_TYPE (n1), n1, n2);
+ tree step = STMT_VINFO_SIMD_CLONE_INFO (stmt_info)[i * 2 + 2];
+ tree opt = TREE_TYPE (thisarginfo.op);
+ bias = fold_convert (TREE_TYPE (step), bias);
+ bias = fold_build2 (MULT_EXPR, TREE_TYPE (step), bias, step);
+ thisarginfo.op
+ = fold_build2 (POINTER_TYPE_P (opt)
+ ? POINTER_PLUS_EXPR : PLUS_EXPR, opt,
+ thisarginfo.op, bias);
+ }
+ }
+ else if (!vec_stmt
+ && thisarginfo.dt != vect_constant_def
+ && thisarginfo.dt != vect_external_def
+ && loop_vinfo
+ && TREE_CODE (op) == SSA_NAME
+ && simple_iv (loop, loop_containing_stmt (stmt), op,
+&iv, false)
+ && tree_fits_shwi_p (iv.step))
{
thisarginfo.linear_step = tree_to_shwi (iv.step);
thisarginfo.op = iv.base;
@@ -2735,8 +2763,8 @@ vectorizable_simd_clone_call (gimple stm
unsigned int badness = 0;
struct cgraph_node *bestn = NULL;
- if (STMT_VINFO_
[PATCH] Fix find_base_term in 32-bit -fpic code (PR lto/64025, take 2)
On Tue, Nov 25, 2014 at 09:20:13AM +0100, Jakub Jelinek wrote:
> Actually, thinking about it more, at least according to
> commutative_operand_precedence the canonical order is
> what we used to return (i.e. (something - _G_O_T_) + (symbol_ref)
> or
> (something - _G_O_T_) + (const (symbol_ref +- const))
> So perhaps better fix is to follow find_base_value, which does something
> like:
> /* Guess which operand is the base address:
>If either operand is a symbol, then it is the base. If
>either operand is a CONST_INT, then the other is the base. */
> if (CONST_INT_P (src_1) || CONSTANT_P (src_0))
> return find_base_value (src_0);
> else if (CONST_INT_P (src_0) || CONSTANT_P (src_1))
> return find_base_value (src_1);
> and do something similar in find_base_term too. I.e. perhaps even with
> higher precedence over REG_P with REG_POINTER (or lower, in these cases
> it doesn't really matter, neither argument is REG_P), choose first
> operand that is CONSTANT_P and not CONST_INT_P.
Here it is. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2014-11-26 Jakub Jelinek
PR lto/64025
* alias.c (find_base_term): Use std::swap. Prefer tmp2
if it is CONSTANT_P other than CONST_INT.
--- gcc/alias.c.jj 2014-11-21 10:17:17.0 +0100
+++ gcc/alias.c 2014-11-26 12:31:24.719485590 +0100
@@ -1756,11 +1756,11 @@ find_base_term (rtx x)
if (REG_P (tmp1) && REG_POINTER (tmp1))
;
else if (REG_P (tmp2) && REG_POINTER (tmp2))
- {
- rtx tem = tmp1;
- tmp1 = tmp2;
- tmp2 = tem;
- }
+ std::swap (tmp1, tmp2);
+ /* If second argument is constant which has base term, prefer it
+ over variable tmp1. See PR64025. */
+ else if (CONSTANT_P (tmp2) && !CONST_INT_P (tmp2))
+ std::swap (tmp1, tmp2);
/* Go ahead and find the base term for both operands. If either base
term is from a pointer or is a named object or a special address
Jakub
Re: [PATCH] Enhance ASAN_CHECK optimization
On Wed, Nov 26, 2014 at 11:42:57AM -0700, ygribov wrote:
> > Testing SANITIZE_ADDRESS bit in flag_sanitize_recover doesn't make sense,
> > testing it in flag_sanitize of course does, but for recover you care
> > whether
> > the SANITIZE_{KERNEL,USER}_ADDRESS bit in flag_sanitize_recover is set
> > depending on if SANITIZE_{KERNEL,USER}_ADDRESS is set in
> > flag_sanitize_recover.
>
> Ok, got it. BTW shouldn't we disable local optimization of ASan checks (in
> asan.c) as well? That would be a massive perf hit ...
Ah, you're right, we are already doing that. So let's just optimize always
even when recovering and we'll see if users don't complain too much.
Jakub
Re: [C/C++ PATCH] Don't convert RHS of a shift-expression to int (PR c/63862)
On Wed, Nov 26, 2014 at 07:20:04PM +0100, Marek Polacek wrote:
> On Wed, Nov 26, 2014 at 06:50:29PM +0100, Jakub Jelinek wrote:
> > On Wed, Nov 26, 2014 at 06:39:44PM +0100, Marek Polacek wrote:
> > > Both C11 and C++14 standards specify that integral promotions are
> > > performed on both operands of a shift-expression. This we do just
> > > fine. But then we convert the right operand to integer_type_node.
> > > Not only is this unnecessary, it can also be harfmul, because for
> > > e.g.
> > > void
> > > foo (unsigned int x)
> > > {
> > > if (-1 >> x != -1)
I'd say fold-const not simplifying this is a bug even if x is signed int.
I think negative shift counts are undefined even in the middle-end, Richard?
Treating 5 >> -5 as 5 << 5 looks just wrong to me, does any FE rely on that?
I don't think the expander will expand it that way though.
Anyway, if the shift count is unnecessary wide type, then the middle-end
won't be able to optimize it as much as it could if it has been a narrower
type.
So, for ubsan purposes (but, shifts are instrumented in the FEs) it is of
course desirable to see the original type, but perhaps it might be a good
idea to cast the shift count to integer_type_node say during gimplification
(perhaps in c_gimplify_expr?).
Jakub
Re: [PATCH, i386] Add new arg values for __builtin_cpu_supports
Hello!
> I've added avx512f support to __builtin_cpu_supports.
> I'm not sure about bw+vl, i think for compound values like
> avx512bd+dq+vl, arch is better. Also for such cases prority is unclear,
> what should we choose bw+vl or e. g. avx512f+er?
> I've left MPX bits in cpuid.h, in case we will need them later (e. g.
> for runtime mpx tests enabling).
>
> Ok for trunk?
>
> gcc/
>
> * config/i386/cpuid.h (bit_MPX, bit_BNDREGS, bit_BNDCSR):
> Define.
> * config/i386/i386.c (get_builtin_code_for_version): Add avx512f.
> (fold_builtin_cpu): Ditto.
> * doc/extend.texi: Documment it.
>
>
> gcc/testsuite/
>
> * g++.dg/ext/mv2.C: Add test for target ("avx512f").
> * gcc.target/i386/builtin_target.c: Ditto.
>
>
> libgcc/
>
> * config/i386/cpuinfo.c (processor_features): Add FEATURE_AVX512F.
> * config/i386/cpuinfo.c (get_available_features): Detect it.
OK.
Thanks,
Uros.
Re: [PATCH] Enhance ASAN_CHECK optimization
> Testing SANITIZE_ADDRESS bit in flag_sanitize_recover doesn't make sense,
> testing it in flag_sanitize of course does, but for recover you care
> whether
> the SANITIZE_{KERNEL,USER}_ADDRESS bit in flag_sanitize_recover is set
> depending on if SANITIZE_{KERNEL,USER}_ADDRESS is set in
> flag_sanitize_recover.
Ok, got it. BTW shouldn't we disable local optimization of ASan checks (in
asan.c) as well? That would be a massive perf hit ...
-Y
--
View this message in context:
http://gcc.1065356.n5.nabble.com/PATCH-Optimize-UBSAN-NULL-checks-add-sanopt-c-tp1085786p1095536.html
Sent from the gcc - patches mailing list archive at Nabble.com.
Re: [patch c++]: Fix PR/53904
OK, thanks. Jason
Re: [PATCH] Enhance ASAN_CHECK optimization
On Wed, Nov 26, 2014 at 11:21:06AM -0700, ygribov wrote:
> > Formatting. The {} should be indented like static
> > and return 2 columns to the right of that.
>
> Right.
>
> > For base_addr computation, you don't really need g or ptr_checks,
> > do you? So why not move the:
> > auto_vec *ptr_checks = &ctx->asan_check_map.get_or_insert (ptr);
> > gimple g = maybe_get_dominating_check (*ptr_checks);
> > lines below the if?
>
> I can do this. But then base_checks would be invalidated when I call
> get_or_insert for ptr_checks so I'll still have to hash_map::get.
You're right.
> > If asan (kernel-address) is
> > recovering, I don't see a difference from not reporting two different
> > invalid accesses to the same function and not reporting two integer
> > overflows in the same function, at least if they have different
> > location_t.
>
> Ok, agreed. BTW how about replacing '& SANITIZE_KERNEL_ADDRESS' with '&
> SANITIZE_ADDRESS'? I know we do not support recovery for userspace but
> having a general enum sounds more logical.
Testing SANITIZE_ADDRESS bit in flag_sanitize_recover doesn't make sense,
testing it in flag_sanitize of course does, but for recover you care whether
the SANITIZE_{KERNEL,USER}_ADDRESS bit in flag_sanitize_recover is set
depending on if SANITIZE_{KERNEL,USER}_ADDRESS is set in
flag_sanitize_recover.
So supposedly
((flag_sanitize & flag_sanitize_recover)
& (SANITIZE_USER_ADDRESS | SANITIZE_KERNEL_ADDRESS)) != 0
is the right check for whether the current address sanitization wants to
recover.
Jakub
Re: [PATCH] Enhance ASAN_CHECK optimization
> Formatting. The {} should be indented like static
> and return 2 columns to the right of that.
Right.
> For base_addr computation, you don't really need g or ptr_checks,
> do you? So why not move the:
> auto_vec *ptr_checks = &ctx->asan_check_map.get_or_insert (ptr);
> gimple g = maybe_get_dominating_check (*ptr_checks);
> lines below the if?
I can do this. But then base_checks would be invalidated when I call
get_or_insert for ptr_checks so I'll still have to hash_map::get.
> If asan (kernel-address) is
> recovering, I don't see a difference from not reporting two different
> invalid accesses to the same function and not reporting two integer
> overflows in the same function, at least if they have different
> location_t.
Ok, agreed. BTW how about replacing '& SANITIZE_KERNEL_ADDRESS' with '&
SANITIZE_ADDRESS'? I know we do not support recovery for userspace but
having a general enum sounds more logical.
-Y
--
View this message in context:
http://gcc.1065356.n5.nabble.com/PATCH-Optimize-UBSAN-NULL-checks-add-sanopt-c-tp1085786p1095527.html
Sent from the gcc - patches mailing list archive at Nabble.com.
Re: [C/C++ PATCH] Don't convert RHS of a shift-expression to int (PR c/63862)
On 11/26/2014 01:18 PM, Joseph Myers wrote: On Wed, 26 Nov 2014, Marek Polacek wrote: Joseph, is that C FE part ok? The C changes are OK once Jakub's middle-end/expander issue is resolved (possibly by adding execution tests of shifts by in-range 64-bit and 128-bit integers, both constant and non-constant, and compilation tests of shifts by out-of-range 64-bit and 128-bit integers). Likewise for C++. Jason
Re: [C/C++ PATCH] Don't convert RHS of a shift-expression to int (PR c/63862)
On Wed, Nov 26, 2014 at 06:50:29PM +0100, Jakub Jelinek wrote:
> On Wed, Nov 26, 2014 at 06:39:44PM +0100, Marek Polacek wrote:
> > Both C11 and C++14 standards specify that integral promotions are
> > performed on both operands of a shift-expression. This we do just
> > fine. But then we convert the right operand to integer_type_node.
> > Not only is this unnecessary, it can also be harfmul, because for
> > e.g.
> > void
> > foo (unsigned int x)
> > {
> > if (-1 >> x != -1)
> > bar ();
> > }
> > with (int) x we lose info that x is nonnegative, which means that
> > tree_expr_nonnegative_p cannot fold this expr. Another problem
> > with the conversion is that we weren't able to detect e.g. shift
> > by 0x1ULL, since after the conversion this is 0.
>
> Have you checked what the expander does with it? Say trying to
> shift something by __int128 count or similar might upset it.
I tried
int
main ()
{
__int128 x = 1;
__int128 y = 200;
__int128 z;
asm ("" : "+g" (x), "+g" (y));
z = x << y;
asm ("" : "+g" (z));
return z;
}
and that works (even with ubsan) as expected. I haven't checked .expand
(I don't think I'd be able to judge the difference anyway), but on the
testcase above the assembly is the same with -O, with -O0 I see:
movq%rbx, -24(%rbp)
movq%rax, -48(%rbp)
movq%rdx, -40(%rbp)
- movq-48(%rbp), %rax
- movl%eax, %ecx
movq-32(%rbp), %rax
movq-24(%rbp), %rdx
+ movzbl -48(%rbp), %ecx
shldq %rax, %rdx
salq%cl, %rax
testb $64, %cl
> Wonder about if we don't make similar assumptions in the middle-end.
> It might make a difference (sometimes positive, sometimes negative)
> for vectorization too.
I don't really know; I assume the testsuite would have catched it if
something went awry.
> > 2014-11-26 Marek Polacek
> >
> > PR c/63862
> > c-family/
> > * c-ubsan.c (ubsan_instrument_shift): Change the type of a MINUS_EXPR
> > to op1_utype.
>
> This part is ok regardless of the rest.
Thanks,
Marek
Two small constexpr PATCHes
1) It occurred to me that now that there are side-effects in
constant-expressions, we need to guard against multiple evaluation of
SAVE_EXPR.
2) We don't want to complain about flowing off the end of a function
because something in the function was non-constant.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 87438df56d20107f2f9699ab0306991a03aa7abb
Author: Jason Merrill
Date: Wed Nov 26 11:13:03 2014 -0500
* constexpr.c (cxx_eval_constant_expression) [SAVE_EXPR]: Avoid
multiple evaluation.
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 111ea5b..ef9ef70 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -2974,11 +2974,22 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
*jump_target = t;
break;
+case SAVE_EXPR:
+ /* Avoid evaluating a SAVE_EXPR more than once. */
+ if (tree *p = ctx->values->get (t))
+ r = *p;
+ else
+ {
+ r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0), addr,
+ non_constant_p, overflow_p);
+ ctx->values->put (t, r);
+ }
+ break;
+
case NON_LVALUE_EXPR:
case TRY_CATCH_EXPR:
case CLEANUP_POINT_EXPR:
case MUST_NOT_THROW_EXPR:
-case SAVE_EXPR:
case EXPR_STMT:
case EH_SPEC_BLOCK:
r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0),
commit e1851b1d2be83cf9ee13a8a21faac3dcac52d746
Author: Jason Merrill
Date: Wed Nov 26 12:00:20 2014 -0500
* constexpr.c (cxx_eval_call_expression): Don't talk about
flowing off the end if we're already non-constant.
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 2678223..111ea5b 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1344,7 +1344,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
else
{
result = *ctx->values->get (slot ? slot : res);
- if (result == NULL_TREE)
+ if (result == NULL_TREE && !*non_constant_p)
{
if (!ctx->quiet)
error ("constexpr call flows off the end "
Re: [C/C++ PATCH] Don't convert RHS of a shift-expression to int (PR c/63862)
On Wed, 26 Nov 2014, Marek Polacek wrote: > Joseph, is that C FE part ok? The C changes are OK once Jakub's middle-end/expander issue is resolved (possibly by adding execution tests of shifts by in-range 64-bit and 128-bit integers, both constant and non-constant, and compilation tests of shifts by out-of-range 64-bit and 128-bit integers). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 3/5] combine: add regno field to LOG_LINKS
On 11/25/14 14:47, Segher Boessenkool wrote:
On Tue, Nov 25, 2014 at 11:46:52AM -0700, Jeff Law wrote:
On 11/14/14 12:19, Segher Boessenkool wrote:
With this new field in place, we can have LOG_LINKS for insns that set
more than one register and distribute them properly in distribute_links.
This then allows many more PARALLELs to be combined.
Also split off new functions can_combine_{def,use}_p from the
create_log_links function.
2014-11-14 Segher Boessenkool
gcc/
* combine.c (struct insn_link): New field `regno'.
(alloc_insn_link): New parameter `regno'. Use it.
(find_single_use): Check the new field.
(can_combine_def_p, can_combine_use_p): New functions. Split
off from ...
(create_log_links): ... here. Correct data type of `regno'.
Adjust call to alloc_insn_link.
(adjust_for_new_dest): Find regno, use it in call to
alloc_insn_link.
(try_combine): Adjust call to alloc_insn_link.
(distribute_links): Check the new field.
Didn't you lose the check that avoids duplicated LOG_LINKs?
I don't think so; if I did, that's a bug.
Or is the
claim that the check is no longer needed because there are no duplicates
now that we include the register associated with the link?
Are you talking about create_log_links? There can be no duplicates there
(anymore), that would be multiple defs of the same reg in the same insn,
indeed.
Yes, I was referring to the code in create_log_links. You dropped the
check for duplicate links. It caught my eye when reading the changes,
but then I realized the check may no longer be necessary.
Hmm, what about an insn that has two destinations, which happen to be
upper and lower SUBREGs of a pseudo. Would that create duplicate links
after your change?
Jeff
Re: [Ping]Re: [PR63762][4.9] Backport the patch which fixes "GCC generates UNPREDICTABLE STR with Rn = Rt for arm"
On Wed, Nov 26, 2014 at 10:09 AM, Renlin Li wrote: > On 26/11/14 12:16, H.J. Lu wrote: >> >> On Wed, Nov 26, 2014 at 4:07 AM, Renlin Li wrote: >>> >>> On 20/11/14 16:17, Renlin Li wrote: Hi all, This is a backport for gcc-4_9-branch of the patch "[PR63762]GCC generates UNPREDICTABLE STR with Rn = Rt for arm" posted in: https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02253.html arm-none-eabi has been test on the model, no new issues. bootstrapping and regression tested on x86, no new issues. Is it Okay for gcc-4_9-branch? gcc/ChangeLog: 2014-11-20 Renlin Li PR middle-end/63762 * ira.c (ira): Update preferred class. gcc/testsuite/ChangeLog: 2014-11-20 Renlin Li PR middle-end/63762 * gcc.dg/pr63762.c: New. >>> >>> Ping for it. >>> >> Please verify if it is the real fix for >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63661 >> >> If yes, please add a testcase for PR 63661 and mention it in >> your ChangeLog entry. >> >> Thanks. >> >> > Hi H.J. > > Yes, I have verified that, this patch additionally fixes PR 63661. > > I observed the same behaviour as I saw on arm backend. It will be great if > you can double check they are caused by exactly the same reason. I will ask our people to take a look. > > > A new testcase has been added, ChangeLog has been updated to reflect the > change. Updated patch has bee attached. > Okay for gcc-4_9-branch? > > Regards, > Renlin Li > > > gcc/ChangeLog: > > 2014-11-26 Renlin Li > > PR middle-end/63762 > PR middle-end/63661 > * ira.c (ira): Update preferred class. > > gcc/testsuite/ChangeLog: > > 2014-11-26 Renlin Li > > PR middle-end/63661 > PR middle-end/63762 > * testsuite/gcc.dg/pr63661.c: New. > * testsuite/gcc.dg/pr63762.c: New. > > pr63661.c should be moved to gcc.target/i386 and run it on PIC target. Thanks. -- H.J.
Re: [Ping]Re: [PR63762][4.9] Backport the patch which fixes "GCC generates UNPREDICTABLE STR with Rn = Rt for arm"
On 26/11/14 12:16, H.J. Lu wrote:
On Wed, Nov 26, 2014 at 4:07 AM, Renlin Li wrote:
On 20/11/14 16:17, Renlin Li wrote:
Hi all,
This is a backport for gcc-4_9-branch of the patch "[PR63762]GCC generates
UNPREDICTABLE STR with Rn = Rt for arm" posted in:
https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02253.html
arm-none-eabi has been test on the model, no new issues. bootstrapping and
regression tested on x86, no new issues.
Is it Okay for gcc-4_9-branch?
gcc/ChangeLog:
2014-11-20 Renlin Li
PR middle-end/63762
* ira.c (ira): Update preferred class.
gcc/testsuite/ChangeLog:
2014-11-20 Renlin Li
PR middle-end/63762
* gcc.dg/pr63762.c: New.
Ping for it.
Please verify if it is the real fix for
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63661
If yes, please add a testcase for PR 63661 and mention it in
your ChangeLog entry.
Thanks.
Hi H.J.
Yes, I have verified that, this patch additionally fixes PR 63661.
I observed the same behaviour as I saw on arm backend. It will be great
if you can double check they are caused by exactly the same reason.
A new testcase has been added, ChangeLog has been updated to reflect the
change. Updated patch has bee attached.
Okay for gcc-4_9-branch?
Regards,
Renlin Li
gcc/ChangeLog:
2014-11-26 Renlin Li
PR middle-end/63762
PR middle-end/63661
* ira.c (ira): Update preferred class.
gcc/testsuite/ChangeLog:
2014-11-26 Renlin Li
PR middle-end/63661
PR middle-end/63762
* testsuite/gcc.dg/pr63661.c: New.
* testsuite/gcc.dg/pr63762.c: New.
diff --git a/gcc/ira.c b/gcc/ira.c
index 4d91d21..0c703c5 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -5347,7 +5347,18 @@ ira (FILE *f)
ira_allocno_iterator ai;
FOR_EACH_ALLOCNO (a, ai)
- ALLOCNO_REGNO (a) = REGNO (ALLOCNO_EMIT_DATA (a)->reg);
+ {
+ int old_regno = ALLOCNO_REGNO (a);
+ int new_regno = REGNO (ALLOCNO_EMIT_DATA (a)->reg);
+
+ ALLOCNO_REGNO (a) = new_regno;
+
+ if (old_regno != new_regno)
+ setup_reg_classes (new_regno, reg_preferred_class (old_regno),
+ reg_alternate_class (old_regno),
+ reg_allocno_class (old_regno));
+ }
+
}
else
{
diff --git a/gcc/testsuite/gcc.dg/pr63661.c b/gcc/testsuite/gcc.dg/pr63661.c
new file mode 100644
index 000..261f616
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr63661.c
@@ -0,0 +1,76 @@
+/* PR middle-end/63661 */
+/* { dg-do run { target x86_64-*-* } } */
+/* { dg-options "-mtune=nehalem -fPIC -O2" } */
+
+static void __attribute__((noinline,noclone,hot))
+foo (double a, double q, double *ff, double *gx, int e, int ni)
+{
+ union
+{
+ double n;
+ unsigned long long o;
+} punner;
+
+ punner.n = q;
+ __builtin_printf("B: 0x%016llx %g\n", punner.o, q);
+
+ if(q != 5)
+__builtin_abort();
+}
+
+static int __attribute__((noinline,noclone,hot))
+bar (int order, double q, double c[])
+{
+ int ni, nn, i, e;
+ double g2, x2, de, s, ratio, ff;
+
+ nn = 0;
+ e = order & 1;
+ s = 0;
+ ratio = 0;
+ x2 = 0;
+ g2 = 0;
+
+ if(q == 0.0)
+return 0;
+
+ if (order < 5)
+{
+ ratio = 1.0 / q;
+ nn = order;
+}
+
+ ni = -nn;
+
+ while(1)
+{
+ de = ratio - g2 - x2;
+
+ foo (0, q, &ff, &g2, e, ni);
+
+ if((int)de == 0)
+break;
+}
+
+ s += 2 * nn * c[nn];
+
+ for (i = 0; i < 1; i++)
+{
+ c[0] = nn;
+ for (; i < 10; i++)
+c[i] = 0.0;
+ c[0] /= s;
+}
+
+ return 0;
+}
+
+int
+main ()
+{
+ double c[1000];
+
+ __builtin_printf("A: 0x%016llx\n", (unsigned long long)c);
+ bar (1, 5.0, c);
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/pr63762.c b/gcc/testsuite/gcc.dg/pr63762.c
new file mode 100644
index 000..df11067
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr63762.c
@@ -0,0 +1,77 @@
+/* PR middle-end/63762 */
+/* { dg-do assemble } */
+/* { dg-options "-O2" } */
+
+#include
+
+void *astFree ();
+void *astMalloc ();
+void astNegate (void *);
+int astGetNegated (void *);
+void astGetRegionBounds (void *, double *, double *);
+int astResampleF (void *, ...);
+
+extern int astOK;
+
+int
+MaskF (int inside, int ndim, const int lbnd[], const int ubnd[],
+ float in[], float val)
+{
+
+ void *used_region;
+ float *c, *d, *out, *tmp_out;
+ double *lbndgd, *ubndgd;
+ int *lbndg, *ubndg, idim, ipix, nax, nin, nout, npix, npixg, result = 0;
+ if (!astOK) return result;
+ lbndg = astMalloc (sizeof (int)*(size_t) ndim);
+ ubndg = astMalloc (sizeof (int)*(size_t) ndim);
+ lbndgd = astMalloc (sizeof (double)*(size_t) ndim);
+ ubndgd = astMalloc (sizeof (double)*(size_t) ndim);
+ if (astOK)
+{
+ astGetRegionBounds (used_region, lbndgd, ubndgd);
+ npix = 1;
+ npixg = 1;
+ for (idim = 0; idim < ndim; idim++)
+{
+ lbndg[ idim ] = lbnd[ idim ];
+ ubndg[ idim ] = ubnd[ idim ];
+ npix *= (ubnd[ idim ] - lbnd[ idim ] + 1);
+ if (npixg >= 0)
Re: [Patch, Fortran] convert almost all {warning,error}_now to common diagnostic
On 25 November 2014 at 23:42, Tobias Burnus wrote:
> FX:
>>>
>>> (a) those majority which might need buffering (gfc_error, gfc_warning);
>>
>> Is there a plan for those in the longer term?
>
>
> Well, the long-term solution is of course to support them. That requires
> adding buffering+discarding support to the common machinery and to use it
> then in gfortran. It's on Manuel's to-do list, but he suffers from the
> generic problem of finding the time to actually do it. Maybe he manages to
> finish that part by early next year - which might or might not be suitable
> for GCC 5 inclusion.
I think the changes to the common diagnostics part could be as simple as:
Index: gcc/pretty-print.c
===
--- gcc/pretty-print.c (revision 218090)
+++ gcc/pretty-print.c (working copy)
@@ -677,19 +677,33 @@ pp_format_verbatim (pretty_printer *pp,
/* Restore previous settings. */
pp_wrapping_mode (pp) = oldmode;
}
-/* Flush the content of BUFFER onto the attached stream. */
+/* Flush the content of BUFFER onto the attached stream. This
+ function does nothing unless pp->output_buffer->flush_p. */
void
pp_flush (pretty_printer *pp)
{
+ if (!pp->output_buffer->flush_p)
+return;
pp_write_text_to_stream (pp);
pp_clear_state (pp);
fflush (pp_buffer (pp)->stream);
}
+/* Flush the content of BUFFER onto the attached stream independently
+ of the value of pp->output_buffer->flush_p. */
+void
+pp_really_flush (pretty_printer *pp)
+{
+ pp_write_text_to_stream (pp);
+ pp_clear_state (pp);
+ fflush (pp_buffer (pp)->stream);
+}
+
+
/* Sets the number of maximum characters per line PRETTY-PRINTER can
output in line-wrapping mode. A LENGTH value 0 suppresses
line-wrapping. */
void
pp_set_line_maximum_length (pretty_printer *pp, int length)
@@ -772,11 +786,12 @@ pretty_printer::pretty_printer (const ch
wrapping (),
format_decoder (),
emitted_prefix (),
need_newline (),
translate_identifiers (true),
-show_color ()
+show_color (),
+flush_p (true)
{
pp_line_cutoff (this) = l;
/* By default, we emit prefixes once per message. */
pp_prefixing_rule (this) = DIAGNOSTICS_SHOW_PREFIX_ONCE;
pp_set_prefix (this, p);
Index: gcc/pretty-print.h
===
--- gcc/pretty-print.h (revision 218090)
+++ gcc/pretty-print.h (working copy)
@@ -98,10 +98,15 @@ struct output_buffer
int line_length;
/* This must be large enough to hold any printed integer or
floating-point value. */
char digit_buffer[128];
+
+ /* Nonzero means that text should be flushed when
+ appropriate. Otherwise, text is buffered until either
+ pp_pp_really_flush or pp_clear_output_area are called. */
+ bool flush_p;
};
/* The type of pretty-printer flags passed to clients. */
typedef unsigned int pp_flags;
However, I'm less familiar with the Fortran buffering mechanism. So
far, I understood that:
* gfc uses two gfc_error_buf (error_buffer, warning_buffer) and
pointer *cur_error_buffer; to switch between them. These are
equivalent to pretty-printer output_buffer, thus my idea is to create
a gfc_pretty_printer that contains three output_buffer (two for
buffering and another for the _now variants).
* There seems to be a global buffer_flag that controls when to buffer.
However, each gfc_error_buf has a variable "flag", that is also
checked sometimes. Unfortunately, nothing in gfc_error_buf has
comments, so I have no idea how these flags are supposed to interact.
It would be great if a Fortran person could explain it in detail.
* I am not sure how to handle the error/warnings counts. The Fortran
code seems to not care about how many errors or warnings are given
when buffering, only whether there was any error at all. Is that
right? It also seems to not care about -fmax-errors (similarly
-Wfatal-errors will only take care after flushing, thus potentially
many errors can be given before stopping). Is this correct? Not having
to handle those would simplify a first version a lot.
* I assume that ICEs and other such fatal_errors are never buffered.
For normal errors converted to fatal by Wfatal-errors, I just need to
be careful in the buffered variants, that is disable it before going
through the common machinery and re-enable it after returning from it.
(This is a note to myself).
Cheers,
Manuel.
Re: [patch c++]: Fix PR/53904
Ok. Adjusted patch attached. Nevertheless we should use here
unsigned HWI instead of possible truncation to signed int. I admit
that it is unlikely to have more then 2^31 elements, but well
Ok for apply with adjusted ChangeLog?
Regards,
Kai
Index: constexpr.c
===
--- constexpr.c (Revision 218076)
+++ constexpr.c (Arbeitskopie)
@@ -2013,12 +2013,12 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tre
bool *non_constant_p, bool *overflow_p)
{
tree elttype = TREE_TYPE (atype);
- int max = tree_to_shwi (array_type_nelts (atype));
+ unsigned HOST_WIDE_INT max = tree_to_uhwi (array_type_nelts_top (atype));
verify_ctor_sanity (ctx, atype);
vec **p = &CONSTRUCTOR_ELTS (ctx->ctor);
vec_alloc (*p, max + 1);
bool pre_init = false;
- int i;
+ unsigned HOST_WIDE_INT i;
/* For the default constructor, build up a call to the default
constructor of the element type. We only need to handle class types
@@ -2043,7 +2043,7 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tre
pre_init = true;
}
- for (i = 0; i <= max; ++i)
+ for (i = 0; i < max; ++i)
{
tree idx = build_int_cst (size_type_node, i);
tree eltinit;
Re: [C/C++ PATCH] Don't convert RHS of a shift-expression to int (PR c/63862)
On Wed, Nov 26, 2014 at 06:39:44PM +0100, Marek Polacek wrote:
> Both C11 and C++14 standards specify that integral promotions are
> performed on both operands of a shift-expression. This we do just
> fine. But then we convert the right operand to integer_type_node.
> Not only is this unnecessary, it can also be harfmul, because for
> e.g.
> void
> foo (unsigned int x)
> {
> if (-1 >> x != -1)
> bar ();
> }
> with (int) x we lose info that x is nonnegative, which means that
> tree_expr_nonnegative_p cannot fold this expr. Another problem
> with the conversion is that we weren't able to detect e.g. shift
> by 0x1ULL, since after the conversion this is 0.
Have you checked what the expander does with it? Say trying to
shift something by __int128 count or similar might upset it.
Wonder about if we don't make similar assumptions in the middle-end.
It might make a difference (sometimes positive, sometimes negative)
for vectorization too.
> 2014-11-26 Marek Polacek
>
> PR c/63862
> c-family/
> * c-ubsan.c (ubsan_instrument_shift): Change the type of a MINUS_EXPR
> to op1_utype.
This part is ok regardless of the rest.
Jakub
Re: [patch] Flatten streamer header files Pt. 1
On 11/26/14 06:11, Andrew MacLeod wrote: So the question to the maintainers is whether its permissible to do a bit of flattening into the early parts of stage 3, or whether you'd rather it stay on a branch until next stage 1. Andrew PIng.. anyone want to chime in? :-) Apparently not :-) I'd hazard a guess it's not on anyone's radar for stage3. But I'll take a look when I can. jeff
Re: [PATCH] Enhance array types debug info. for Ada
On Wed, Oct 08, 2014 at 09:05:30PM +0200, Pierre-Marie de Rodat wrote: > gcc/ > * dwarf2out.h (struct array_descr_info): Remove the base_decl field. > * dwarf2out.c (enum dw_scalar_form): New. > (struct loc_descr_context): New. > (add_scalar_info): New. > (add_bound_info): Add a context parameter. Use add_scalar_info. > (loc_list_from_tree): Add a context parameter. Handle PLACEHOLDER_EXPR > nodes for type-related expressions. Likewise for base declarations. > (loc_descriptor_from_tree): Add a context parameter. > (subrange_type_die): Update calls to add_bound_info. > (tls_mem_loc_descriptor): Likewise. > (loc_list_for_address_of_addr_expr_of_indirect_ref): Add a context > parameter. Update calls to loc_list_from_tree. > (add_subscript_info): Update calls to add_bound_info. > (gen_array_type_die): Update calls to loc_list_from_tree and to > add_bound_info. > (descr_info_loc): Remove. > (add_descr_info_field): Remove. > (gen_descr_array_type_die): Switch add_descr_info_field calls into > add_scalar_info/add_bound_info ones. > (gen_subprogram_die): Update calls to loc_list_from_tree. > (gen_variable_die): Likewise. Replace implicitely with implicitly in the whole patch. > + to refer to register values). > + > + CONTEXT provides information to customize the location descriptions > + generation. Its context_type field specifies what type is implicitely > + referenced by DW_OP_push_object_address. If it is NULL_TREE, this > operation > + will not be generated. > + > + If CONTEXT is NULL, the behavior is the same as if the context_type field > + was NULL_TREE. */ as if both context_type and base_decl were NULL_TREE? > @@ -14311,6 +14351,12 @@ loc_list_from_tree (tree loc, int want_address) > extending the values properly. Hopefully this won't be a real > problem... */ > > + if (context != NULL > + && context->base_decl == loc > + && want_address == 0) > +return new_loc_list (new_loc_descr (DW_OP_push_object_address, 0, 0), > + NULL, NULL, NULL); > + This isn't guarded with dwarf_version >= 3 || !dwarf_strict. Shouldn't it be too and return NULL otherwise? > + expansion_failed (loc, NULL_RTX, > + "PLACEHOLDER_EXPR for a unexpected type"); for an unexpected type? > @@ -14533,7 +14594,8 @@ loc_list_from_tree (tree loc, int want_address) > > list_ret = loc_list_from_tree (obj, > want_address == 2 > -&& !bitpos && !offset ? 2 : 1); > +&& !bitpos && !offset ? 2 : 1, > + context); Formatting. Should use tabs, not spaces. > + if (prec <= HOST_BITS_PER_WIDE_INT > +|| tree_fits_uhwi_p (value)) Formatting. || should be below p in prec. Would be nice if you tried more than one fortran testcase, say build all gfortran.dg/ tests with -O0 -g -dA (and perhaps -O2 -g -dA afterwards) with both unpatched and patched compilers and diff *.s files? Otherwise, LGTM. Jakub
Re: [PATCH 08/08] PR/64003 workaround (uninit memory in i386.md)
On 11/25/14 18:39, David Malcolm wrote: I suspect this is papering over a real problem, but I've been applying this workaround locally to keep my valgrind output clean. gcc/ChangeLog: PR/64003 * final.c (shorten_branches): Allocate insn_lengths with XCNEWVEC rather than XNEWVEC; remove subsequent per-element initialization. I'd like more information on this one. My first thought is that something must be creating a new insn after shorten_branches is complete or an existing insn that was not on the chain when we called shorten-branches, but got threaded onto the chain later. Either would be considered bad in various ways, so we'd like to fix it. Presumably you're not doing an out-of-range access into insn_lengths? Right? Do you have a reasonable testcase for this? jeff
[C/C++ PATCH] Don't convert RHS of a shift-expression to int (PR c/63862)
Both C11 and C++14 standards specify that integral promotions are
performed on both operands of a shift-expression. This we do just
fine. But then we convert the right operand to integer_type_node.
Not only is this unnecessary, it can also be harfmul, because for
e.g.
void
foo (unsigned int x)
{
if (-1 >> x != -1)
bar ();
}
with (int) x we lose info that x is nonnegative, which means that
tree_expr_nonnegative_p cannot fold this expr. Another problem
with the conversion is that we weren't able to detect e.g. shift
by 0x1ULL, since after the conversion this is 0.
This patch does away with the conversion to integer type for both
C and C++ FEs. It exposed an ubsan bug where I was using incorrect
type for a MINUS_EXPR. With this patch, the XFAIL in shiftopt-1.c
is not needed anymore.
Joseph, is that C FE part ok?
Jason, is that C++ FE part ok?
Jakub, is that ubsan part ok?
Bootstrapped/regtested on ppc64-linux; ubsan.exp tested even on x86_64.
2014-11-26 Marek Polacek
PR c/63862
c-family/
* c-ubsan.c (ubsan_instrument_shift): Change the type of a MINUS_EXPR
to op1_utype.
c/
* c-typeck.c (build_binary_op) : Don't
convert the right operand to integer type.
cp/
* typeck.c (cp_build_binary_op) : Don't
convert the right operand to integer type.
testsuite/
* gcc.c-torture/execute/shiftopt-1.c: Don't XFAIL anymore.
* c-c++-common/ubsan/shift-7.c: New test.
diff --git gcc/c-family/c-ubsan.c gcc/c-family/c-ubsan.c
index 90b03f2..96afc67 100644
--- gcc/c-family/c-ubsan.c
+++ gcc/c-family/c-ubsan.c
@@ -151,7 +151,7 @@ ubsan_instrument_shift (location_t loc, enum tree_code code,
&& !TYPE_UNSIGNED (type0)
&& flag_isoc99)
{
- tree x = fold_build2 (MINUS_EXPR, unsigned_type_node, uprecm1,
+ tree x = fold_build2 (MINUS_EXPR, op1_utype, uprecm1,
fold_convert (op1_utype, op1));
tt = fold_convert_loc (loc, unsigned_type_for (type0), op0);
tt = fold_build2 (RSHIFT_EXPR, TREE_TYPE (tt), tt, x);
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 67efb46..bf0f306 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -10513,11 +10513,6 @@ build_binary_op (location_t location, enum tree_code
code,
/* Use the type of the value to be shifted. */
result_type = type0;
- /* Convert the non vector shift-count to an integer, regardless
-of size of value being shifted. */
- if (TREE_CODE (TREE_TYPE (op1)) != VECTOR_TYPE
- && TYPE_MAIN_VARIANT (TREE_TYPE (op1)) != integer_type_node)
- op1 = convert (integer_type_node, op1);
/* Avoid converting op1 to result_type later. */
converted = 1;
}
@@ -10563,11 +10558,6 @@ build_binary_op (location_t location, enum tree_code
code,
/* Use the type of the value to be shifted. */
result_type = type0;
- /* Convert the non vector shift-count to an integer, regardless
-of size of value being shifted. */
- if (TREE_CODE (TREE_TYPE (op1)) != VECTOR_TYPE
- && TYPE_MAIN_VARIANT (TREE_TYPE (op1)) != integer_type_node)
- op1 = convert (integer_type_node, op1);
/* Avoid converting op1 to result_type later. */
converted = 1;
}
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 8b66acc..6ca346b 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -4295,10 +4295,6 @@ cp_build_binary_op (location_t location,
"right shift count >= width of type");
}
}
- /* Convert the shift-count to an integer, regardless of
-size of value being shifted. */
- if (TYPE_MAIN_VARIANT (TREE_TYPE (op1)) != integer_type_node)
- op1 = cp_convert (integer_type_node, op1, complain);
/* Avoid converting op1 to result_type later. */
converted = 1;
}
@@ -4344,10 +4340,6 @@ cp_build_binary_op (location_t location,
"left shift count >= width of type");
}
}
- /* Convert the shift-count to an integer, regardless of
-size of value being shifted. */
- if (TYPE_MAIN_VARIANT (TREE_TYPE (op1)) != integer_type_node)
- op1 = cp_convert (integer_type_node, op1, complain);
/* Avoid converting op1 to result_type later. */
converted = 1;
}
diff --git gcc/testsuite/c-c++-common/ubsan/shift-7.c
gcc/testsuite/c-c++-common/ubsan/shift-7.c
index e69de29..1e33273 100644
--- gcc/testsuite/c-c++-common/ubsan/shift-7.c
+++ gcc/testsuite/c-c++-common/ubsan/shift-7.c
@@ -0,0 +1,27 @@
+/* PR c/63862 */
+/* { dg-do run } */
+/* { dg-options "-fsanitize=undefined" } */
+
+unsigned long long int __attribute__ ((noinline, noclone))
+foo (unsigned long long int i, unsigned long long int j)
+{
+ asm ("");
+ return i >> j;
+}
+
+unsigned long long int __attribute__ ((n
Re: [Patch, ARM, ping1] Fix PR target/56846
On 26/11/14 17:23 -, Thomas Preud'homme wrote: Ping? I'm OK with backporting it if a release manager approves it. -Original Message- From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- ow...@gcc.gnu.org] On Behalf Of Thomas Preud'homme Sent: Wednesday, November 19, 2014 6:00 PM To: Tony Wang; gcc-patches@gcc.gnu.org; d...@debian.org; aph- g...@littlepinkcloud.com; Richard Earnshaw; Ramana Radhakrishnan; libstd...@gcc.gnu.org Subject: RE: [Patch ARM] Fix PR target/56846 > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@gcc.gnu.org] On Behalf Of Tony Wang > > > Hi all, > > The bug is reported at > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56846, and it’s about the > problem that > when exception handler is involved in the function, then > _Unwind_Backtrace function will run into deadloop on > arm target. The patch (in r215101) can be backported without any change on 4.8 and 4.9 branches. I checked in QEMU with and without the patch on both branches and it indeed solves the problem. Testsuite run without regression when compiled with arm-none-eabi cross compiler and executed on QEMU emulating Cortex-M3. I also bootstrapped gcc on x86_64-linux-gnu and run the testsuite without regressions. Is it ok for backport? Best regards, Thomas
RE: [Patch, ARM, ping1] Fix PR target/56846
Ping? > -Original Message- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@gcc.gnu.org] On Behalf Of Thomas Preud'homme > Sent: Wednesday, November 19, 2014 6:00 PM > To: Tony Wang; gcc-patches@gcc.gnu.org; d...@debian.org; aph- > g...@littlepinkcloud.com; Richard Earnshaw; Ramana Radhakrishnan; > libstd...@gcc.gnu.org > Subject: RE: [Patch ARM] Fix PR target/56846 > > > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > > ow...@gcc.gnu.org] On Behalf Of Tony Wang > > > > > > > Hi all, > > > > The bug is reported at > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56846, and it’s about the > > problem that > > when exception handler is involved in the function, then > > _Unwind_Backtrace function will run into deadloop on > > arm target. > > The patch (in r215101) can be backported without any change on 4.8 and > 4.9 branches. I checked in QEMU with and without the patch on both > branches and it indeed solves the problem. > > Testsuite run without regression when compiled with arm-none-eabi > cross compiler and executed on QEMU emulating Cortex-M3. > > I also bootstrapped gcc on x86_64-linux-gnu and run the testsuite > without > regressions. > > Is it ok for backport? > > Best regards, > > Thomas > > > >
Re: [PATCH 00/03] JIT refactoring
On 11/25/14 18:45, David Malcolm wrote: Various cleanups of the jit code Successfully bootstrapped & regrtested on x86_64-unknown-linux-gnu (Fedora 20). OK for trunk? (am not yet officially blessed as the JIT maintainer, so I require review for the non-obvious patches) You're now the official maintainer for the JIT bits. jeff
Re: Move dwarf2 frame tables to read-only section for AIX
On 09/22/2014 09:06 PM, Andrew Dixie wrote: - || ((fde_encoding & 0x70) != DW_EH_PE_absptr - && (fde_encoding & 0x70) != DW_EH_PE_aligned - && (per_encoding & 0x70) != DW_EH_PE_absptr - && (per_encoding & 0x70) != DW_EH_PE_aligned - && (lsda_encoding & 0x70) != DW_EH_PE_absptr - && (lsda_encoding & 0x70) != DW_EH_PE_aligned)) - ? 0 : SECTION_WRITE); + || ((fde_encoding & 0x70) != DW_EH_PE_absptr + && (fde_encoding & 0x70) != DW_EH_PE_aligned + && (per_encoding & 0x70) != DW_EH_PE_absptr + && (per_encoding & 0x70) != DW_EH_PE_aligned + && (lsda_encoding & 0x70) != DW_EH_PE_absptr + && (lsda_encoding & 0x70) != DW_EH_PE_aligned)) + ? 0 : SECTION_WRITE); Please don't change whitespace on lines unrelated to your change. OK with that removed. Jason
Re: [PATCH] Fix ubsan and C++14 constexpr ICEs (PR sanitizer/63956)
On 11/20/2014 02:04 PM, Marek Polacek wrote:
+ if (fun == NULL_TREE)
+switch (CALL_EXPR_IFN (t))
+ {
+ case IFN_UBSAN_NULL:
+ case IFN_UBSAN_BOUNDS:
+ return void_node;
+ default:
+ break;
+ }
Other IFNs should make the call non-constant.
-/* { dg-error "is not a constant expression" "" { target c++ } 12 } */
+/* { dg-warning "right shift count is negative" "" { target c++ } 12 } */
This should be an xfail (pending the delayed folding work) instead of a
different test.
+constexpr int n3 = fn7 ((const int *) 0, 8); // { dg-error "is not a constant
expression|constexpr call flows off" }
The "flows off the end" error is a bug and should not be tested for.
I'm going to check in a fix.
Jason
Re: [C++ PATCH] Detect UB in shifts in constexpr functions
Please give diagnostics explaining what's wrong with the shift rather than the generic "is not a constant expression". + tree t = build_int_cst (unsigned_type_node, uprec - 1); + t = fold_build2 (MINUS_EXPR, unsigned_type_node, t, rhs); + tree ulhs = fold_convert (unsigned_type_for (lhstype), lhs); + t = fold_build2 (RSHIFT_EXPR, TREE_TYPE (ulhs), ulhs, t); + if (tree_int_cst_lt (integer_one_node, t)) This could also use a comment explaining the logic. Jason
Re: Ping with testcase: [PATCH][AArch64] Fix __builtin_aarch64_absdi, must not fold to ABS_EXPR
On Wed, Nov 26, 2014 at 03:56:03PM +, Alan Lawrence wrote:
> So in case there's any confusion about the behaviour expected of *the vabs
> intrinsic*, here's a testcase (failing without patch, passing with it)...
>
> --Alan
>
> Alan Lawrence wrote:
> > ...as the former is defined as returning MIN_VALUE for argument MIN_VALUE,
> > whereas the latter is 'undefined', and gcc can optimize "abs(x)>=0" to
> > "true",
> > which is wrong for __builtin_aarch64_abs.
> >
> > There has been much debate here, although not recently - I think the last
> > was
> > https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00387.html . However without
> > a
> > definite solution, we should at least stop the folding, as otherwise this
> > is a bug.
Well, I don't see myself getting round to looking at the solutions
proposed in that thread any time soon, and as you say this is a longstanding
bug in our intrinsics implementation. So I'm in favour of that part of
your patch going in, though I have some comments:
> @@ -1317,9 +1322,6 @@ aarch64_fold_builtin (tree fndecl, int n_args
> ATTRIBUTE_UNUSED, tree *args,
>
>switch (fcode)
> {
> - BUILTIN_VALLDI (UNOP, abs, 2)
> - return fold_build1 (ABS_EXPR, type, args[0]);
> - break;
>BUILTIN_VALLDI (BINOP, cmge, 0)
> return fold_build2 (GE_EXPR, type, args[0], args[1]);
> break;
Why do we want to turn off folding for the V4SF/V2SF/V2DF modes of these
intrinsics? There should be no difference between the mid-end definition
and the intrinsic definition of their behaviour.
I also note that the integer forms of these now end up as an "abs" RTL
expression - can we guarantee that preserves the intrinsics behaviour and
no RTL-folder will come along and mis-optimize? Documentation is vague
on this point.
I'm also not convinced by the SIMD_ARG_SCRATCH foo you add. Looking at
the aarch64.md:absdi2 pattern I can't see why we need that scratch at
all. It seems we could get away with marking operand 0 early-clobber and
using it as our scratch register. Then we could drop all the extra
infrastructure from this patch.
Thanks,
James
> >
> > The complication here is that the folding meant we never expanded the call
> > to
> > __builtin_aarch64_absdi, and thus never realized that expanding would cause
> > an
> > ICE: gen_absdi2() takes only two parameters (source and dest operand), and
> > has
> > no parameter corresponding to the scratch operand in the insn_data; but the
> > code
> > in aarch64_simd_expand_args, reads the number of arguments from the
> > insn_data,
> > and so tries to get another operand from the call to
> > __builtin_aarch64_absdi,
> > causing the ICE. Hence, we have to introduce a SIMD_ARG_SCRATCH, used in
> > aarch64_simd_expand_args to skip over the argument.
> >
> > (There is an alternative way of doing this, i.e. to update the for-loop in
> > aarch64_simd_expand_builtin by adding yet another version of 'k'. However,
> > I
> > thought there were enough versions already that I really didn't want to add
> > one
> > more...)
> >
> > To go with this, I've renamed qualifier_internal to qualifier_scratch, as
> > it's
> > not clear that any non-scratch internal operands (whatever such might be)
> > would
> > want the same treatment!
> >
> > Cross-tested check-gcc on aarch64-none-elf.
> >
> > Ok for trunk?
> >
> > gcc/ChangeLog:
> >
> > * config/aarch64/aarch64-builtins.c (enum aarch64_type_qualifiers):
> > Rename qualifier_internal to qualifier_scratch.
> > (aarch64_types_unop_qualifiers, aarch64_init_simd_builtins): Follow
> > renaming.
> > (builtin_simd_arg): New SIMD_ARG_SCRATCH enum value.
> > (aarch64_simd_expand_args): Skip over SIMD_ARG_SCRATCHes.
> > (aarch64_simd_expand_builtin): Handle qualifier_scratch.
> > (aarch64_fold_builtin): Remove folding of abs.
> diff --git a/gcc/testsuite/gcc.target/aarch64/vabs_intrinsic_2.c
> b/gcc/testsuite/gcc.target/aarch64/vabs_intrinsic_2.c
> new file mode 100644
> index
> ..12fdd813bc3f58a183b4986c5c9c532c2b608699
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/vabs_intrinsic_2.c
> @@ -0,0 +1,17 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#include
> +
> +extern void abort (void);
> +
> +int
> +main (int argc, char **argv)
> +{
> + int8x8_t a = vabs_s8 (vdup_n_s8 (-128)); /* Should all be -128. */
> + uint8x8_t b = vcltz_s8 (a); /* Should all be true i.e. -1. */
> + if (vget_lane_u8 (b, 1))
> +return 0;
> + abort ();
> +}
> +
Re: [Build, Graphite] PR64017 - support ISL-0.14 (gcc/configure.ac and gcc/graphite*.c)
On 26.11.2014 17:27, Tobias Burnus wrote: On Wed, Nov 26, 2014 at 04:55:40PM +0100, Tobias Burnus wrote: This patch adds a configure check for isl_schedule_constraints_compute_schedule, which is new in ISL 0.13 - and uses it for conditional compilation. Repeating the test on a massively multicore system, it fails at stage2 with an in-tree 0.14 because -lisl cannot be found at gcc/configure time. Hence, HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE is false and one gets later an error because a 0.12.2 function is not available with ISL 0.14. However, running the configure test case manually - or building with fewer cores - works. As does removing "gcc" after the failure such that isl/ is available. Thus, it looks like a dependency problem. It seems to be fixed by the attached patch. OK for the trunk? OK from the graphite site. This needs a buildsystem OK though. Tobias
Re: [Build, Graphite] PR64017 - support ISL-0.14 (gcc/configure.ac and gcc/graphite*.c)
On Wed, Nov 26, 2014 at 04:55:40PM +0100, Tobias Burnus wrote:
> This patch adds a configure check for
> isl_schedule_constraints_compute_schedule,
> which is new in ISL 0.13 - and uses it for conditional compilation.
Repeating the test on a massively multicore system, it fails at stage2 with an
in-tree 0.14 because -lisl cannot be found at gcc/configure time. Hence,
HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE is false and one gets later an
error because a 0.12.2 function is not available with ISL 0.14.
However, running the configure test case manually - or building with fewer
cores - works. As does removing "gcc" after the failure such that isl/ is
available. Thus, it looks like a dependency problem.
It seems to be fixed by the attached patch.
OK for the trunk?
Tobias
2014-11-26 Tobias Burnus
PR middle-end/64017
* Makefile.def: Make more dependent on mpfr, mpc and isl.
* Makefile.in: Regenerate.
diff --git a/Makefile.def b/Makefile.def
index ddcbf5b..7c8761a 100644
--- a/Makefile.def
+++ b/Makefile.def
@@ -308,6 +308,9 @@ dependencies = { module=all-build-libcpp; on=all-build-libiberty; };
// Host modules specific to gcc.
dependencies = { module=configure-gcc; on=configure-intl; };
dependencies = { module=configure-gcc; on=all-gmp; };
+dependencies = { module=configure-gcc; on=all-mpfr; };
+dependencies = { module=configure-gcc; on=all-mpc; };
+dependencies = { module=configure-gcc; on=all-isl; };
dependencies = { module=configure-gcc; on=all-lto-plugin; };
dependencies = { module=configure-gcc; on=all-binutils; };
dependencies = { module=configure-gcc; on=all-gas; };
diff --git a/Makefile.in b/Makefile.in
index 02d914c..182e1fd 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -48094,6 +48094,30 @@ configure-stage3-gcc: maybe-all-stage3-gmp
configure-stage4-gcc: maybe-all-stage4-gmp
configure-stageprofile-gcc: maybe-all-stageprofile-gmp
configure-stagefeedback-gcc: maybe-all-stagefeedback-gmp
+configure-gcc: maybe-all-mpfr
+
+configure-stage1-gcc: maybe-all-stage1-mpfr
+configure-stage2-gcc: maybe-all-stage2-mpfr
+configure-stage3-gcc: maybe-all-stage3-mpfr
+configure-stage4-gcc: maybe-all-stage4-mpfr
+configure-stageprofile-gcc: maybe-all-stageprofile-mpfr
+configure-stagefeedback-gcc: maybe-all-stagefeedback-mpfr
+configure-gcc: maybe-all-mpc
+
+configure-stage1-gcc: maybe-all-stage1-mpc
+configure-stage2-gcc: maybe-all-stage2-mpc
+configure-stage3-gcc: maybe-all-stage3-mpc
+configure-stage4-gcc: maybe-all-stage4-mpc
+configure-stageprofile-gcc: maybe-all-stageprofile-mpc
+configure-stagefeedback-gcc: maybe-all-stagefeedback-mpc
+configure-gcc: maybe-all-isl
+
+configure-stage1-gcc: maybe-all-stage1-isl
+configure-stage2-gcc: maybe-all-stage2-isl
+configure-stage3-gcc: maybe-all-stage3-isl
+configure-stage4-gcc: maybe-all-stage4-isl
+configure-stageprofile-gcc: maybe-all-stageprofile-isl
+configure-stagefeedback-gcc: maybe-all-stagefeedback-isl
configure-gcc: maybe-all-lto-plugin
configure-stage1-gcc: maybe-all-stage1-lto-plugin
Re: [C++ Patch/RFC] PR 63757
On 11/24/2014 01:55 PM, Paolo Carlini wrote: in this rejects-valid, as part of build_user_type_conversion_1, standard_conversion is called by implicit_conversion with a *null* expr, thus the condition in standard_conversion /* [conv.ptr] A null pointer constant can be converted to a pointer type; ... A null pointer constant of integral type can be converted to an rvalue of type std::nullptr_t. */ if ((tcode == POINTER_TYPE || TYPE_PTRMEM_P (to) || NULLPTR_TYPE_P (to)) && expr && null_ptr_cst_p (expr)) conv = build_conv (ck_std, to, conv); is false and the snippet is rejected. Should we pass a nullptr_node as expr in such cases, ie, when handling conversions functions returning std::nullptr_t?!? I'd prefer to change the test quoted above to not require expr to be non-null in the case of NULLPTR_TYPE_P. Jason
Re: [PATCH] -fsanitize=vptr instrumentation (take 2)
On 10/28/2014 08:44 AM, Jakub Jelinek wrote:
+cp_ubsan_check_member_access_r (tree *stmt_p, int *walk_subtrees, void *data)
This function needs a longer comment about exactly what forms it's
trying to instrument.
+ /* T t; t.foo (); doesn't need instrumentation, if the type is known. */
+ if (is_addr
+ && TREE_CODE (op) == ADDR_EXPR
+ && DECL_P (TREE_OPERAND (op, 0))
+ && same_type_p (type,
+ TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (op, 0)
+return NULL_TREE;
How do we know the decl's vptr hasn't been clobbered? This seems like
one of the optimizations we decided to drop.
+ if (TREE_CODE (base) == COMPONENT_REF
+ && DECL_ARTIFICIAL (TREE_OPERAND (base, 1)))
+{
+ tree base2 = TREE_OPERAND (base, 0);
+ while (TREE_CODE (base2) == COMPONENT_REF
+|| TREE_CODE (base2) == ARRAY_REF
+|| TREE_CODE (base2) == ARRAY_RANGE_REF)
+ base2 = TREE_OPERAND (base2, 0);
+ if (TREE_CODE (base2) != INDIRECT_REF
+ && TREE_CODE (base2) != MEM_REF)
+ return;
+}
+ else if (TREE_CODE (base) != INDIRECT_REF
+ && TREE_CODE (base) != MEM_REF)
+return;
Why do you look through ARRAY_REF here? An element of an array is its
own complete object.
Jason
Re: [PATCH, PR63995, CHKP] Use single static bounds var for varpool nodes sharing asm name
2014-11-26 19:07 GMT+03:00 Jan Hubicka :
>> On 25 Nov 15:03, Ilya Enkovich wrote:
>> > 2014-11-25 14:11 GMT+03:00 Richard Biener :
>> > > On Tue, Nov 25, 2014 at 11:19 AM, Ilya Enkovich
>> > > wrote:
>> > >
>> > > Ok, then it's get_for_asmname (). That said - the above loops look
>> > > bogus to me. Honza - any better ideas?
>> >
>> > get_for_asmname () returns the first element in a chain of nodes with
>> > the same asm name. May I rely on the order of nodes in this chain?
>> > Probably use ASSEMBLER_NAME as a key in chkp_static_var_bounds hash?
>> >
>> > Thanks,
>> > Ilya
>> >
>> > >
>> > > Richard.
>> > >
>>
>> A variant with var's ASSEMBLER_NAME as a key works fine. Instrumented
>> bootstrap passes. OK for trunk?
>
> It is possible to have two different assembler name for same resulting symbol.
What do you mean? DECL_ASSEMBLER_NAME is changed during compilation?
Or transparent alias chain?
> Why you simply don't use symtab nodes as keys?
I cannot use symtab node because I have two different nodes for
symbols with the same assembler name. I suppose with LTO these
symbols would be merged. But having two var decls and two symtab
nodes I need to create and emit only one corresponding bounds symbol.
Ilya
>
> Honza
>>
>> Thanks,
>> Ilya
>> --
>> gcc/
>>
>> 2014-11-26 Ilya Enkovich
>>
>> PR bootstrap/63995
>> * tree-chkp.c (chkp_make_static_bounds): Share bounds var
>> between nodes sharing assembler name.
>>
>> gcc/testsuite
>>
>> 2014-11-26 Ilya Enkovich
>>
>> PR bootstrap/63995
>> * g++.dg/dg.exp: Add mpx-dg.exp.
>> * g++.dg/pr63995-1.C: New.
>>
>>
>> diff --git a/gcc/testsuite/g++.dg/dg.exp b/gcc/testsuite/g++.dg/dg.exp
>> index 14beae1..44eab0c 100644
>> --- a/gcc/testsuite/g++.dg/dg.exp
>> +++ b/gcc/testsuite/g++.dg/dg.exp
>> @@ -18,6 +18,7 @@
>>
>> # Load support procs.
>> load_lib g++-dg.exp
>> +load_lib mpx-dg.exp
>>
>> # If a testcase doesn't have special options, use these.
>> global DEFAULT_CXXFLAGS
>> diff --git a/gcc/testsuite/g++.dg/pr63995-1.C
>> b/gcc/testsuite/g++.dg/pr63995-1.C
>> new file mode 100644
>> index 000..82e7606
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/pr63995-1.C
>> @@ -0,0 +1,16 @@
>> +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
>> +/* { dg-require-effective-target mpx } */
>> +/* { dg-options "-O2 -g -fcheck-pointer-bounds -mmpx" } */
>> +
>> +int test1 (int i)
>> +{
>> + extern const int arr[10];
>> + return arr[i];
>> +}
>> +
>> +extern const int arr[10];
>> +
>> +int test2 (int i)
>> +{
>> + return arr[i];
>> +}
>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>> index 3e38691..924cb71 100644
>> --- a/gcc/tree-chkp.c
>> +++ b/gcc/tree-chkp.c
>> @@ -2727,9 +2727,23 @@ chkp_make_static_bounds (tree obj)
>>/* First check if we already have required var. */
>>if (chkp_static_var_bounds)
>> {
>> - slot = chkp_static_var_bounds->get (obj);
>> - if (slot)
>> - return *slot;
>> + /* For vars we use assembler name as a key in
>> + chkp_static_var_bounds map. It allows to
>> + avoid duplicating bound vars for decls
>> + sharing assembler name. */
>> + if (TREE_CODE (obj) == VAR_DECL)
>> + {
>> + tree name = DECL_ASSEMBLER_NAME (obj);
>> + slot = chkp_static_var_bounds->get (name);
>> + if (slot)
>> + return *slot;
>> + }
>> + else
>> + {
>> + slot = chkp_static_var_bounds->get (obj);
>> + if (slot)
>> + return *slot;
>> + }
>> }
>>
>>/* Build decl for bounds var. */
>> @@ -2793,7 +2807,13 @@ chkp_make_static_bounds (tree obj)
>>if (!chkp_static_var_bounds)
>> chkp_static_var_bounds = new hash_map;
>>
>> - chkp_static_var_bounds->put (obj, bnd_var);
>> + if (TREE_CODE (obj) == VAR_DECL)
>> +{
>> + tree name = DECL_ASSEMBLER_NAME (obj);
>> + chkp_static_var_bounds->put (name, bnd_var);
>> +}
>> + else
>> +chkp_static_var_bounds->put (obj, bnd_var);
>>
>>return bnd_var;
>> }
[PING 3] Enhance array types debug info. for Ada
Hello, Ping for https://gcc.gnu.org/ml/gcc-patches/2014-10/msg00694.html Thanks in advance! Regards, -- Pierre-Marie de Rodat
Re: [PATCH, PR63995, CHKP] Use single static bounds var for varpool nodes sharing asm name
> On 25 Nov 15:03, Ilya Enkovich wrote:
> > 2014-11-25 14:11 GMT+03:00 Richard Biener :
> > > On Tue, Nov 25, 2014 at 11:19 AM, Ilya Enkovich
> > > wrote:
> > >
> > > Ok, then it's get_for_asmname (). That said - the above loops look
> > > bogus to me. Honza - any better ideas?
> >
> > get_for_asmname () returns the first element in a chain of nodes with
> > the same asm name. May I rely on the order of nodes in this chain?
> > Probably use ASSEMBLER_NAME as a key in chkp_static_var_bounds hash?
> >
> > Thanks,
> > Ilya
> >
> > >
> > > Richard.
> > >
>
> A variant with var's ASSEMBLER_NAME as a key works fine. Instrumented
> bootstrap passes. OK for trunk?
It is possible to have two different assembler name for same resulting symbol.
Why you simply don't use symtab nodes as keys?
Honza
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-11-26 Ilya Enkovich
>
> PR bootstrap/63995
> * tree-chkp.c (chkp_make_static_bounds): Share bounds var
> between nodes sharing assembler name.
>
> gcc/testsuite
>
> 2014-11-26 Ilya Enkovich
>
> PR bootstrap/63995
> * g++.dg/dg.exp: Add mpx-dg.exp.
> * g++.dg/pr63995-1.C: New.
>
>
> diff --git a/gcc/testsuite/g++.dg/dg.exp b/gcc/testsuite/g++.dg/dg.exp
> index 14beae1..44eab0c 100644
> --- a/gcc/testsuite/g++.dg/dg.exp
> +++ b/gcc/testsuite/g++.dg/dg.exp
> @@ -18,6 +18,7 @@
>
> # Load support procs.
> load_lib g++-dg.exp
> +load_lib mpx-dg.exp
>
> # If a testcase doesn't have special options, use these.
> global DEFAULT_CXXFLAGS
> diff --git a/gcc/testsuite/g++.dg/pr63995-1.C
> b/gcc/testsuite/g++.dg/pr63995-1.C
> new file mode 100644
> index 000..82e7606
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr63995-1.C
> @@ -0,0 +1,16 @@
> +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
> +/* { dg-require-effective-target mpx } */
> +/* { dg-options "-O2 -g -fcheck-pointer-bounds -mmpx" } */
> +
> +int test1 (int i)
> +{
> + extern const int arr[10];
> + return arr[i];
> +}
> +
> +extern const int arr[10];
> +
> +int test2 (int i)
> +{
> + return arr[i];
> +}
> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
> index 3e38691..924cb71 100644
> --- a/gcc/tree-chkp.c
> +++ b/gcc/tree-chkp.c
> @@ -2727,9 +2727,23 @@ chkp_make_static_bounds (tree obj)
>/* First check if we already have required var. */
>if (chkp_static_var_bounds)
> {
> - slot = chkp_static_var_bounds->get (obj);
> - if (slot)
> - return *slot;
> + /* For vars we use assembler name as a key in
> + chkp_static_var_bounds map. It allows to
> + avoid duplicating bound vars for decls
> + sharing assembler name. */
> + if (TREE_CODE (obj) == VAR_DECL)
> + {
> + tree name = DECL_ASSEMBLER_NAME (obj);
> + slot = chkp_static_var_bounds->get (name);
> + if (slot)
> + return *slot;
> + }
> + else
> + {
> + slot = chkp_static_var_bounds->get (obj);
> + if (slot)
> + return *slot;
> + }
> }
>
>/* Build decl for bounds var. */
> @@ -2793,7 +2807,13 @@ chkp_make_static_bounds (tree obj)
>if (!chkp_static_var_bounds)
> chkp_static_var_bounds = new hash_map;
>
> - chkp_static_var_bounds->put (obj, bnd_var);
> + if (TREE_CODE (obj) == VAR_DECL)
> +{
> + tree name = DECL_ASSEMBLER_NAME (obj);
> + chkp_static_var_bounds->put (name, bnd_var);
> +}
> + else
> +chkp_static_var_bounds->put (obj, bnd_var);
>
>return bnd_var;
> }
Re: [PATCH 2/5] combine: handle I2 a parallel of two SETs
On Sat, Nov 15, 2014 at 06:59:23AM -0600, Segher Boessenkool wrote: > On Fri, Nov 14, 2014 at 08:35:48PM +0100, Bernd Schmidt wrote: > > On 11/14/2014 08:19 PM, Segher Boessenkool wrote: > > >+ /* If I2 is a PARALLEL of two SETs of REGs (and perhaps some CLOBBERs), > > >+ make those two SETs separate I1 and I2 insns, and make an I0 that is > > >+ the original I1. */ > > >+ if (i0 == 0 > > >+ && GET_CODE (PATTERN (i2)) == PARALLEL > > >+ && XVECLEN (PATTERN (i2), 0) >= 2 > > >+ && GET_CODE (XVECEXP (PATTERN (i2), 0, 0)) == SET > > >+ && GET_CODE (XVECEXP (PATTERN (i2), 0, 1)) == SET > > >+ && REG_P (SET_DEST (XVECEXP (PATTERN (i2), 0, 0))) > > >+ && REG_P (SET_DEST (XVECEXP (PATTERN (i2), 0, 1))) > > >+ && !reg_used_between_p (SET_DEST (XVECEXP (PATTERN (i2), 0, 0)), > > >i2, i3) > > >+ && !reg_used_between_p (SET_DEST (XVECEXP (PATTERN (i2), 0, 1)), > > >i2, i3) > > > > Don't we have other code in combine checking the reg_used_between case? Actually, no, no we don't. Under the old regime (before adding the regno field to log_links), a link links a first insn with its earliest successor that uses any of the regs that first insn sets. This guarantees that in try_combine none of the insns between I2 and I3 will use any of the registers set by I2. Under the new regime (patches 3 and 4) a link links a first insn with its earliest successor that uses a particular reg the first insn sets. For single sets, this is the same as before; but not so for multiple sets. There are only two cases where this matters: this patch, and the code right before it (that never seems to trigger, not on any target; trying to figure out if that is true, and if so, why). New patchset is bootstrapping/testing. Cheers, Segher
Re: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop
FWIW, I agree this is the right fix, but: Andrew Bennett writes: > + /* When using branch likely (-mfix-r1), the delay slot instruction > + will be annulled on false. The normal delay slot instructions > + calculate the overall result of the atomic operation and must not > + be annulled. To ensure this behaviour unconditionally use a NOP > + in the delay slot for the branch likely case. */ remember to use US spelling: "behavior" rather than "behaviour". Thanks, Richard
Re: [Build, Graphite] PR64017 - support ISL-0.14 (gcc/configure.ac and gcc/graphite*.c)
On 26.11.2014 16:55, Tobias Burnus wrote: This patch adds a configure check for isl_schedule_constraints_compute_schedule, which is new in ISL 0.13 - and uses it for conditional compilation. The graphite*c patch is based on the one of Jack Howarth, https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00906.html - without checking whether the ISL replacements make sense. With the patch, I have successfully bootstrapped GCC with ISL 0.12.2 and ISL 0.14 (both using an in-tree build). [I still have to regtest the two variants and I also want to do a system-ISL built with ISL 0.12.2] Is the patch OK for the trunk? Tobias PS: I'd be happy if some others could run additional tests. PPS: If the patch is in, can someone put ISL 0.14 to infrastructure? Then one can update contrib/download_prerequisites - such that the graphite failure is at least fixed for in-tree builds (cf. PR64017, PR62289). That looks good from the graphite site. Cheers, Tobias
Re: [C++ PATCH] Diagnose conversions from string constants to char* as forbidden, not deprecated, in c++11 and above
On 11/26/2014 10:48 AM, Manuel López-Ibáñez wrote:
If I understand correctly, the cxx11 standard requires to diagnose
this. Thus, it should be always a pedwarn, independently of Wpedantic.
I agree. This is what I checked in:
commit 2aeccf739475ee181f4ca6422776b46bc9526352
Author: jason
Date: Wed Nov 26 15:16:07 2014 +
Diagnose string constant conversion to char* in c++11 and above
as forbidden, not deprecated.
* typeck.c (string_conv_p): Do a pedwarn in c++11 and above,
change the diagnostic for the Wwrite-strings case for c++11 and above.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@218087 138bc75d-0d04-0410-961f-82ee72b054a4
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index e100d70..8b66acc 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -2139,12 +2139,18 @@ string_conv_p (const_tree totype, const_tree exp, int warn)
|| TREE_CODE (TREE_OPERAND (exp, 0)) != STRING_CST)
return 0;
}
-
- /* This warning is not very useful, as it complains about printf. */
if (warn)
-warning (OPT_Wwrite_strings,
- "deprecated conversion from string constant to %qT",
- totype);
+{
+ if (cxx_dialect >= cxx11)
+ pedwarn (input_location,
+ pedantic ? OPT_Wpedantic : OPT_Wwrite_strings,
+ "ISO C++ forbids converting a string constant to %qT",
+ totype);
+ else
+ warning (OPT_Wwrite_strings,
+ "deprecated conversion from string constant to %qT",
+ totype);
+}
return 1;
}
diff --git a/gcc/testsuite/g++.dg/warn/write-strings-default.C b/gcc/testsuite/g++.dg/warn/write-strings-default.C
index ee6b217..063b303 100644
--- a/gcc/testsuite/g++.dg/warn/write-strings-default.C
+++ b/gcc/testsuite/g++.dg/warn/write-strings-default.C
@@ -3,5 +3,5 @@
int main()
{
- char* p = "Asgaard"; // { dg-warning "deprecated" }
+ char* p = "Asgaard"; // { dg-warning "deprecated|forbids converting a string constant" }
}
diff --git a/gcc/testsuite/g++.dg/warn/write-strings.C b/gcc/testsuite/g++.dg/warn/write-strings.C
index 73c8149..1293e85 100644
--- a/gcc/testsuite/g++.dg/warn/write-strings.C
+++ b/gcc/testsuite/g++.dg/warn/write-strings.C
@@ -3,5 +3,5 @@
int main()
{
- char* p = "Asgaard"; // { dg-warning "deprecated" }
+ char* p = "Asgaard"; // { dg-warning "deprecated|forbids converting a string constant" }
}
diff --git a/gcc/testsuite/g++.old-deja/g++.bob/inherit1.C b/gcc/testsuite/g++.old-deja/g++.bob/inherit1.C
index e75190b..c037a1c 100644
--- a/gcc/testsuite/g++.old-deja/g++.bob/inherit1.C
+++ b/gcc/testsuite/g++.old-deja/g++.bob/inherit1.C
@@ -12,7 +12,7 @@ public:
class B : public A {
public:
char* m1 () { C::m1(); return ""; } // { dg-error "cannot call" }
-// { dg-warning "deprecated" "depr" { target *-*-* } 14 }
+// { dg-warning "deprecated|forbids converting a string constant" "depr" { target *-*-* } 14 }
};
int main () {
diff --git a/gcc/testsuite/g++.old-deja/g++.brendan/template17.C b/gcc/testsuite/g++.old-deja/g++.brendan/template17.C
index 94eaf3d..b8e2a0b 100644
--- a/gcc/testsuite/g++.old-deja/g++.brendan/template17.C
+++ b/gcc/testsuite/g++.old-deja/g++.brendan/template17.C
@@ -10,6 +10,6 @@ public:
const Regex NDAMName<'L'>::pattern("^[Ll](.*)$", 1);// { dg-error "type/value mismatch" "mismatch" }
// { dg-message "expected a type" "expected" { target *-*-* } 11 }
-// { dg-warning "deprecated" "depr" { target *-*-* } 11 }
+// { dg-warning "deprecated|forbids converting a string constant" "depr" { target *-*-* } 11 }
unsigned NDAMName<'L'>::sequence_number = 0;// { dg-error "type/value mismatch" "mismatch" }
// { dg-message "expected a type" "exp" { target *-*-* } 14 }
diff --git a/gcc/testsuite/g++.old-deja/g++.law/temps1.C b/gcc/testsuite/g++.old-deja/g++.law/temps1.C
index bd344b4..5734210 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/temps1.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/temps1.C
@@ -16,5 +16,5 @@ struct cookie
cookie ( foo * x) { v=x; }
};
-cookie cat(&foo("apabepa"));// { dg-warning "deprecated conversion" "dep" }
+cookie cat(&foo("apabepa"));// { dg-warning "deprecated conversion|forbids converting a string constant" "dep" }
// { dg-warning "taking address of temporary" "add" { target *-*-* } 19 }
diff --git a/gcc/testsuite/g++.old-deja/g++.martin/typedef2.C b/gcc/testsuite/g++.old-deja/g++.martin/typedef2.C
index fa31867..99603cf 100644
--- a/gcc/testsuite/g++.old-deja/g++.martin/typedef2.C
+++ b/gcc/testsuite/g++.old-deja/g++.martin/typedef2.C
@@ -3,5 +3,5 @@
// Check implicit conversion from string constants into typedefs
typedef char CHAR;
-void f2(CHAR *s=""); // { dg-warning "deprecated" }
+void f2(CHAR *s=""); // { dg-warning "deprecated|forbids converting a string constant" }
Ping with testcase: [PATCH][AArch64] Fix __builtin_aarch64_absdi, must not fold to ABS_EXPR
So in case there's any confusion about the behaviour expected of *the vabs
intrinsic*, here's a testcase (failing without patch, passing with it)...
--Alan
Alan Lawrence wrote:
...as the former is defined as returning MIN_VALUE for argument MIN_VALUE,
whereas the latter is 'undefined', and gcc can optimize "abs(x)>=0" to "true",
which is wrong for __builtin_aarch64_abs.
There has been much debate here, although not recently - I think the last was
https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00387.html . However without a
definite solution, we should at least stop the folding, as otherwise this is a bug.
The complication here is that the folding meant we never expanded the call to
__builtin_aarch64_absdi, and thus never realized that expanding would cause an
ICE: gen_absdi2() takes only two parameters (source and dest operand), and has
no parameter corresponding to the scratch operand in the insn_data; but the code
in aarch64_simd_expand_args, reads the number of arguments from the insn_data,
and so tries to get another operand from the call to __builtin_aarch64_absdi,
causing the ICE. Hence, we have to introduce a SIMD_ARG_SCRATCH, used in
aarch64_simd_expand_args to skip over the argument.
(There is an alternative way of doing this, i.e. to update the for-loop in
aarch64_simd_expand_builtin by adding yet another version of 'k'. However, I
thought there were enough versions already that I really didn't want to add one
more...)
To go with this, I've renamed qualifier_internal to qualifier_scratch, as it's
not clear that any non-scratch internal operands (whatever such might be) would
want the same treatment!
Cross-tested check-gcc on aarch64-none-elf.
Ok for trunk?
gcc/ChangeLog:
* config/aarch64/aarch64-builtins.c (enum aarch64_type_qualifiers):
Rename qualifier_internal to qualifier_scratch.
(aarch64_types_unop_qualifiers, aarch64_init_simd_builtins): Follow
renaming.
(builtin_simd_arg): New SIMD_ARG_SCRATCH enum value.
(aarch64_simd_expand_args): Skip over SIMD_ARG_SCRATCHes.
(aarch64_simd_expand_builtin): Handle qualifier_scratch.
(aarch64_fold_builtin): Remove folding of abs.
diff --git a/gcc/testsuite/gcc.target/aarch64/vabs_intrinsic_2.c b/gcc/testsuite/gcc.target/aarch64/vabs_intrinsic_2.c
new file mode 100644
index ..12fdd813bc3f58a183b4986c5c9c532c2b608699
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/vabs_intrinsic_2.c
@@ -0,0 +1,17 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include
+
+extern void abort (void);
+
+int
+main (int argc, char **argv)
+{
+ int8x8_t a = vabs_s8 (vdup_n_s8 (-128)); /* Should all be -128. */
+ uint8x8_t b = vcltz_s8 (a); /* Should all be true i.e. -1. */
+ if (vget_lane_u8 (b, 1))
+return 0;
+ abort ();
+}
+
[Build, Graphite] PR64017 - support ISL-0.14 (gcc/configure.ac and gcc/graphite*.c)
This patch adds a configure check for isl_schedule_constraints_compute_schedule,
which is new in ISL 0.13 - and uses it for conditional compilation.
The graphite*c patch is based on the one of Jack Howarth,
https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00906.html - without checking
whether the ISL replacements make sense.
With the patch, I have successfully bootstrapped GCC with ISL 0.12.2 and
ISL 0.14 (both using an in-tree build). [I still have to regtest the two
variants and I also want to do a system-ISL built with ISL 0.12.2]
Is the patch OK for the trunk?
Tobias
PS: I'd be happy if some others could run additional tests.
PPS: If the patch is in, can someone put ISL 0.14 to infrastructure? Then
one can update contrib/download_prerequisites - such that the graphite
failure is at least fixed for in-tree builds (cf. PR64017, PR62289).
2014-11-26 Tobias Burnus
Jack Howarth
PR middle-end/64017
* configure.ac (ac_has_isl_schedule_constraints_compute_schedule):
New check.
* doc/install.texi (ISL): Permit ISL 0.14.
* graphite-optimize-isl.c (getScheduleForBandList, optimize_isl):
Conditionally use ISL 0.13+ functions.
* graphite-interchange.c: Make 'extern "C"' conditional.
* graphite-isl-ast-to-gimple.c: Ditto.
* graphite-poly.c: Ditto.
* graphite-sese-to-poly.c: Ditto.
* config.in: Regenerate.
* gcc/configure: Regenerate.
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 2293fb8..48c8000 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -5601,6 +5601,30 @@ if test "x${ISLLIBS}" != "x" ; then
AC_DEFINE(HAVE_isl, 1, [Define if isl is in use.])
fi
+# Check whether isl_schedule_constraints_compute_schedule is available;
+# it's new in ISL-0.13.
+if test "x${ISLLIBS}" != "x" ; then
+ saved_CFLAGS="$CFLAGS"
+ CFLAGS="$CFLAGS $ISLINC"
+ saved_LIBS="$LIBS"
+ LIBS="$LIBS $ISLLIBS $GMPLIBS"
+
+ AC_MSG_CHECKING([Checking for isl_schedule_constraints_compute_schedule])
+ AC_TRY_LINK([#include ],
+ [isl_schedule_constraints_compute_schedule (NULL);],
+ [ac_has_isl_schedule_constraints_compute_schedule=yes],
+ [ac_has_isl_schedule_constraints_compute_schedule=no])
+ AC_MSG_RESULT($ac_has_isl_schedule_constraints_compute_schedule)
+
+ LIBS="$saved_LIBS"
+ CFLAGS="$saved_CFLAGS"
+
+ if test x"$ac_has_isl_schedule_constraints_compute_schedule" = x"yes"; then
+ AC_DEFINE(HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE, 1,
+ [Define if isl_schedule_constraints_compute_schedule exists.])
+ fi
+fi
+
GCC_ENABLE_PLUGINS
AC_SUBST(pluginlibs)
AC_SUBST(enable_plugin)
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index e9ea4a2..1fbef63 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -383,7 +383,7 @@ installed but it is not in your default library search path, the
@option{--with-mpc} configure option should be used. See also
@option{--with-mpc-lib} and @option{--with-mpc-include}.
-@item ISL Library version 0.12.2
+@item ISL Library version 0.14 (or 0.12.2)
Necessary to build GCC with the Graphite loop optimizations.
It can be downloaded from @uref{ftp://gcc.gnu.org/pub/gcc/infrastructure/}
diff --git a/gcc/graphite-interchange.c b/gcc/graphite-interchange.c
index 81ba391..9f30d24 100644
--- a/gcc/graphite-interchange.c
+++ b/gcc/graphite-interchange.c
@@ -30,11 +30,13 @@ along with GCC; see the file COPYING3. If not see
#include
#include
#include
-#if defined(__cplusplus)
+
+/* Since ISL-0.13, the extern is in val_gmp.h. */
+#if !defined(HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE) && defined(__cplusplus)
extern "C" {
#endif
#include
-#if defined(__cplusplus)
+#if !defined(HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE) && defined(__cplusplus)
}
#endif
#endif
diff --git a/gcc/graphite-isl-ast-to-gimple.c b/gcc/graphite-isl-ast-to-gimple.c
index bbf3055..456b24e 100644
--- a/gcc/graphite-isl-ast-to-gimple.c
+++ b/gcc/graphite-isl-ast-to-gimple.c
@@ -25,11 +25,13 @@ along with GCC; see the file COPYING3. If not see
#include
#include
#include
-#if defined(__cplusplus)
+
+/* Since ISL-0.13, the extern is in val_gmp.h. */
+#if !defined(HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE) && defined(__cplusplus)
extern "C" {
#endif
#include
-#if defined(__cplusplus)
+#if !defined(HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE) && defined(__cplusplus)
}
#endif
#endif
diff --git a/gcc/graphite-optimize-isl.c b/gcc/graphite-optimize-isl.c
index 195101a..4cce700 100644
--- a/gcc/graphite-optimize-isl.c
+++ b/gcc/graphite-optimize-isl.c
@@ -460,7 +460,11 @@ getScheduleForBandList (isl_band_list *BandList, isl_union_map **map_sepcl)
if (flag_loop_unroll_jam && (i != (ScheduleDimensions - depth)))
continue;
+#ifdef HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE
+ if (isl_band_member_is_coincident (Band, i))
+#else
if (isl_band_member_is_zero_distance (Band, i))
+#endif
{
isl_map *TileMap;
isl_union_map *TileUMap;
@@ -564,6 +568,9 @@ optimize_isl
Re: [C++ PATCH] Diagnose conversions from string constants to char* as forbidden, not deprecated, in c++11 and above
Sorry to be pedantic, but I think this is not exactly right:
+if (cxx_dialect >= cxx11)
+ {
+ if (pedantic)
+ pedwarn (input_location, OPT_Wpedantic,
+ "ISO C++ forbids converting a string constant to %qT",
+ totype);
+ else
+ warning (OPT_Wwrite_strings,
+ "ISO C++ forbids converting a string constant to %qT",
+ totype);
+ }
Quoting from here: https://gcc.gnu.org/wiki/DiagnosticsGuidelines
pedwarn is for code that is accepted by GCC but it should be rejected
or diagnosed according to the current standard, or it conflicts with
the standard (either the default or the one selected by -std=). Most
pedwarns are controlled by -Wpedantic, a few are controlled by options
that are enabled by -Wpedantic and others are enabled by default.
That is, although Wpedantic is only used for pedwarns, the distinction
between pedwarn and warning is independent of Wpedantic and only
depends on the current -std= value.
If I understand correctly, the cxx11 standard requires to diagnose
this. Thus, it should be always a pedwarn, independently of Wpedantic.
Now, if we also want to warn for Wwrite_strings, then you can do:
+if (cxx_dialect >= cxx11)
+ {
+ pedwarn (input_location, pedantic ? OPT_Wpedantic : OPT_Wwrite_strings,
+ "ISO C++ forbids converting a string constant to %qT",
+ totype);
+ }
The effect is the same, because right now there is no difference
between pedwarn and warning except when -pedantic-errors is given, and
that will enable Wpedantic anyway. However, since the latter code is
simpler and pedantically more correct, I'd suggest to use that.
Cheers,
Manuel.
Re: [PATCH, i386] Add new arg values for __builtin_cpu_supports
> >> I think using cpuid for that is just fine. __builtin_cpu_supports
> >> is for ISA additions users might actually want to version code for,
> >> MPX stuff, as the instructions are nops without hw support, are not
> >> something one would multi-version a function for.
> >> If anything, AVX512F and AVX512BW+VL might be good candidates for that,
> >> not
> >> MPX.
> >
> > SOrry, I didn't know the __builtin_cpu_supports was really only ment for
> > user multi-versioning. In that case, it won't make any sense to put the MPX
> > stuff in there.
> >
> > Sorry for sending you down a wrong path Ilya.
>
> It's OK, AVX guys will just transform this MPX patch into AVX512 one :)
>
Hi,
I've added avx512f support to __builtin_cpu_supports.
I'm not sure about bw+vl, i think for compound values like
avx512bd+dq+vl, arch is better. Also for such cases prority is unclear,
what should we choose bw+vl or e. g. avx512f+er?
I've left MPX bits in cpuid.h, in case we will need them later (e. g.
for runtime mpx tests enabling).
Ok for trunk?
gcc/
* config/i386/cpuid.h (bit_MPX, bit_BNDREGS, bit_BNDCSR):
Define.
* config/i386/i386.c (get_builtin_code_for_version): Add avx512f.
(fold_builtin_cpu): Ditto.
* doc/extend.texi: Documment it.
gcc/testsuite/
* g++.dg/ext/mv2.C: Add test for target ("avx512f").
* gcc.target/i386/builtin_target.c: Ditto.
libgcc/
* config/i386/cpuinfo.c (processor_features): Add FEATURE_AVX512F.
* config/i386/cpuinfo.c (get_available_features): Detect it.
---
gcc/config/i386/cpuid.h| 5 +++
gcc/config/i386/i386.c | 10 +++--
gcc/doc/extend.texi| 2 +
gcc/testsuite/g++.dg/ext/mv2.C | 51 +++---
gcc/testsuite/gcc.target/i386/builtin_target.c | 4 ++
libgcc/config/i386/cpuinfo.c | 5 ++-
6 files changed, 52 insertions(+), 25 deletions(-)
diff --git a/gcc/config/i386/cpuid.h b/gcc/config/i386/cpuid.h
index 6c6e7f3..e3b7646 100644
--- a/gcc/config/i386/cpuid.h
+++ b/gcc/config/i386/cpuid.h
@@ -72,6 +72,7 @@
#define bit_AVX2 (1 << 5)
#define bit_BMI2 (1 << 8)
#define bit_RTM(1 << 11)
+#define bit_MPX(1 << 14)
#define bit_AVX512F(1 << 16)
#define bit_AVX512DQ (1 << 17)
#define bit_RDSEED (1 << 18)
@@ -91,6 +92,10 @@
#define bit_PREFETCHWT1 (1 << 0)
#define bit_AVX512VBMI (1 << 1)
+/* XFEATURE_ENABLED_MASK register bits (%eax == 13, %ecx == 0) */
+#define bit_BNDREGS (1 << 3)
+#define bit_BNDCSR (1 << 4)
+
/* Extended State Enumeration Sub-leaf (%eax == 13, %ecx == 1) */
#define bit_XSAVEOPT (1 << 0)
#define bit_XSAVEC (1 << 1)
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index eafc15a..2493130 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -34235,7 +34235,8 @@ get_builtin_code_for_version (tree decl, tree
*predicate_list)
P_FMA,
P_PROC_FMA,
P_AVX2,
-P_PROC_AVX2
+P_PROC_AVX2,
+P_AVX512F
};
enum feature_priority priority = P_ZERO;
@@ -34263,7 +34264,8 @@ get_builtin_code_for_version (tree decl, tree
*predicate_list)
{"fma4", P_FMA4},
{"xop", P_XOP},
{"fma", P_FMA},
- {"avx2", P_AVX2}
+ {"avx2", P_AVX2},
+ {"avx512f", P_AVX512F}
};
@@ -35238,6 +35240,7 @@ fold_builtin_cpu (tree fndecl, tree *args)
F_FMA4,
F_XOP,
F_FMA,
+F_AVX512F,
F_MAX
};
@@ -35326,7 +35329,8 @@ fold_builtin_cpu (tree fndecl, tree *args)
{"fma4", F_FMA4},
{"xop",F_XOP},
{"fma",F_FMA},
- {"avx2", F_AVX2}
+ {"avx2", F_AVX2},
+ {"avx512f",F_AVX512F}
};
tree __processor_model_type = build_processor_model_struct ();
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 7178c9a..773e14c 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -11642,6 +11642,8 @@ SSE4.2 instructions.
AVX instructions.
@item avx2
AVX2 instructions.
+@item avx512f
+AVX512F instructions.
@end table
Here is an example:
diff --git a/gcc/testsuite/g++.dg/ext/mv2.C b/gcc/testsuite/g++.dg/ext/mv2.C
index 869e99b..d4f1f92 100644
--- a/gcc/testsuite/g++.dg/ext/mv2.C
+++ b/gcc/testsuite/g++.dg/ext/mv2.C
@@ -20,31 +20,34 @@ int foo () __attribute__ ((target ("sse4.2")));
int foo () __attribute__ ((target ("popcnt")));
int foo () __attribute__ ((target ("avx")));
int foo () __attribute__ ((target ("avx2")));
+int foo () __attribute__ ((target ("avx512f")));
int main ()
{
int val = foo ();
- if (__builtin_cpu_supports ("avx2"))
-assert (val == 1);
+ if (__builtin_cpu_supports ("avx512f"))
+assert (val == 11);
+ else if (__builtin_cpu_supports ("avx2"))
+assert (val == 10);
else if (__builtin_cpu_supports ("avx"))
-assert (val == 2);
+assert (val == 9);
else if (__builtin_cpu_supports ("popcnt"))
-assert (val == 3);
+a
Re: [patch] Fix handling of inlining and nested functions in dwarf2out.c
I ran a quick test to see if the output after this patch matches the examples in D.7, and it does. So the patch is OK. Jason
Re: [PATCH 04/08] PR jit/63854: Remove xstrdup from ipa/cgraph fprintf calls
On Wed, 2014-11-26 at 09:13 +0100, Uros Bizjak wrote: > Hello! > > > cgraph*.c and ipa-*.c use xstrdup on strings when dumping them via > > fprintf, leaking all of the duplicated buffers. > > > > Is/was there a reason for doing this? > > Yes, please see [1] and PR 53136 [2]. As said in [1]: > > "There is a problem with multiple calls of cgraph_node_name in fprintf > dumps. Please note that C++ uses caching in > cxx_printable_name_internal (aka LANG_HOOKS_DECL_PRINTABLE_NAME), so > when cxx_printable_name_internal is called multiple times from printf > (i.e. fprintf "%s/%i -> %s/%i"), it can happen that the first string > gets evicted by the second call, before fprintf is fully evaluated." > > > Taking them out fixes these leaks (seen when dumping is enabled): > > But you will get "Invalid read of size X" instead. > > The patch at [1] fixed these, but introduced memory leaks, which were > tolerable at the time: > > "I think that small memory leak is tolerable here (the changes are > exclusively in the dump code), and follows the same approach as in > java frontend." > > It seems that these assumptions are not valid anymore. > > [1] https://gcc.gnu.org/ml/gcc-patches/2012-04/msg01904.html > [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53136 Aha! Thanks. I was only running under valgrind when testing the jit code, and the jit code looks like a frontend to the rest of GCC, it doesn't provide an implementation of LANG_HOOKS_DECL_PRINTABLE_NAME. I ran the rest of the gcc testsuite outside of gcc. So perhaps the duplications of the transient decl strings is still needed, and my patch would undo that fix. Oops. I'm about to disappear for a US holiday, but maybe a good solution here is to invent something like a "xstrdup_for_dump" function like this: /* When using fprintf (or similar), problems can arise with transient generated strings. Many string-generation APIs only support one result being alive at once (e.g. by returning a pointer to a statically-allocated buffer). If there is more than one generated string within one fprintf call: the first string gets evicted or overwritten by the second, before fprintf is fully evaluated. See e.g. PR/53136. This function provides a workaround for this, by providing a simple way to create copies of these transient strings, without the need to have explicit cleanup: fprintf (dumpfile, "string 1: %s string 2:%s\n", xstrdup_for_dump (EXPR_1), xstrdup_for_dump (EXPR_2)); The copied string is eventually freed, from code within toplev::finalize. */ extern const char * xstrdup_for_dump (const char *transient_str); We could then use this instead of xstrdup at these callsites, which would document what's going on. (I'm not in love with the name, but hopefully the idea is clear). This could be implemented either: (A) in terms of the "long-term allocator" idea here: https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02420.html which uses a obstack on the gcc::context, eventually freeing the allocations from toplev::finalize (although that's currently only called from libgccjit.so, but we could call it from cc1 etc when running under valgrind). (B) or in terms of ggc_xstrdup, with the assumption that no GC can occur in anything within a fprintf call. That would document what's going on, ensure that we don't have use-after-free issues, and stop memory leak warnings from valgrind concerning these duplicates. Thoughts? Dave
Re: [PATCH] Fix sanitizer/63788
On Wed, Nov 26, 2014 at 02:57:20PM +0100, Marek Polacek wrote: > Ping. Ok, thanks. > > On Wed, Nov 19, 2014 at 08:09:21PM +0100, Marek Polacek wrote: > > As discussed in the PR, the problem here is that when running gfortran, > > the __builtin_object_size decl isn't available, because c-family's > > c_define_builtins wasn't called. One way how to fix this is to build > > the __builtin_object_size decl in initialize_sanitizer_builtins, if > > needed. Alternatively we could just bail in instrument_object_size > > if builtin_decl_explicit (BUILT_IN_OBJECT_SIZE) returns NULL_TREE... > > > > No test attached since we don't have any Fortran ubsan infrastructure, > > I've just tried > > ./xgcc -B ./ -B ../x86_64-unknown-linux-gnu/libsanitizer/ubsan/.libs/ > > -Wl,-rpath=../x86_64-unknown-linux-gnu/libsanitizer/ubsan/.libs/ -B > > ../x86_64-unknown-linux-gnu/libgfortran/.libs/ -O -fsanitize=undefined > > testcase.f -lgfortran; ./a.out > > on the testcase attached in PR - and it doesn't ICE. > > > > Bootstrapped/regtested on ppc64-linux, ok for trunk? > > > > 2014-11-19 Marek Polacek > > > > PR sanitizer/63788 > > * asan.c (initialize_sanitizer_builtins): Add BT_FN_SIZE_CONST_PTR_INT > > var. Conditionally build BUILT_IN_OBJECT_SIZE decl. > > (ATTR_PURE_NOTHROW_LEAF_LIST): Define. Jakub
Re: [PATCH] Fix sanitizer/63788
Ping.
On Wed, Nov 19, 2014 at 08:09:21PM +0100, Marek Polacek wrote:
> As discussed in the PR, the problem here is that when running gfortran,
> the __builtin_object_size decl isn't available, because c-family's
> c_define_builtins wasn't called. One way how to fix this is to build
> the __builtin_object_size decl in initialize_sanitizer_builtins, if
> needed. Alternatively we could just bail in instrument_object_size
> if builtin_decl_explicit (BUILT_IN_OBJECT_SIZE) returns NULL_TREE...
>
> No test attached since we don't have any Fortran ubsan infrastructure,
> I've just tried
> ./xgcc -B ./ -B ../x86_64-unknown-linux-gnu/libsanitizer/ubsan/.libs/
> -Wl,-rpath=../x86_64-unknown-linux-gnu/libsanitizer/ubsan/.libs/ -B
> ../x86_64-unknown-linux-gnu/libgfortran/.libs/ -O -fsanitize=undefined
> testcase.f -lgfortran; ./a.out
> on the testcase attached in PR - and it doesn't ICE.
>
> Bootstrapped/regtested on ppc64-linux, ok for trunk?
>
> 2014-11-19 Marek Polacek
>
> PR sanitizer/63788
> * asan.c (initialize_sanitizer_builtins): Add BT_FN_SIZE_CONST_PTR_INT
> var. Conditionally build BUILT_IN_OBJECT_SIZE decl.
> (ATTR_PURE_NOTHROW_LEAF_LIST): Define.
>
> diff --git gcc/asan.c gcc/asan.c
> index ff08f1b..c2ed3d5 100644
> --- gcc/asan.c
> +++ gcc/asan.c
> @@ -2294,6 +2294,9 @@ initialize_sanitizer_builtins (void)
> pointer_sized_int_node, NULL_TREE);
>tree BT_FN_VOID_INT
> = build_function_type_list (void_type_node, integer_type_node,
> NULL_TREE);
> + tree BT_FN_SIZE_CONST_PTR_INT
> += build_function_type_list (size_type_node, const_ptr_type_node,
> + integer_type_node, NULL_TREE);
>tree BT_FN_BOOL_VPTR_PTR_IX_INT_INT[5];
>tree BT_FN_IX_CONST_VPTR_INT[5];
>tree BT_FN_IX_VPTR_IX_INT[5];
> @@ -2365,6 +2368,8 @@ initialize_sanitizer_builtins (void)
> #undef ATTR_COLD_CONST_NORETURN_NOTHROW_LEAF_LIST
> #define ATTR_COLD_CONST_NORETURN_NOTHROW_LEAF_LIST \
>/* ECF_COLD missing */ ATTR_CONST_NORETURN_NOTHROW_LEAF_LIST
> +#undef ATTR_PURE_NOTHROW_LEAF_LIST
> +#define ATTR_PURE_NOTHROW_LEAF_LIST ECF_PURE | ATTR_NOTHROW_LEAF_LIST
> #undef DEF_SANITIZER_BUILTIN
> #define DEF_SANITIZER_BUILTIN(ENUM, NAME, TYPE, ATTRS) \
>decl = add_builtin_function ("__builtin_" NAME, TYPE, ENUM,
> \
> @@ -2374,6 +2379,15 @@ initialize_sanitizer_builtins (void)
>
> #include "sanitizer.def"
>
> + /* -fsanitize=object-size uses __builtin_object_size, but that might
> + not be available for e.g. Fortran at this point. We use
> + DEF_SANITIZER_BUILTIN here only as a convenience macro. */
> + if ((flag_sanitize & SANITIZE_OBJECT_SIZE)
> + && !builtin_decl_implicit_p (BUILT_IN_OBJECT_SIZE))
> +DEF_SANITIZER_BUILTIN (BUILT_IN_OBJECT_SIZE, "object_size",
> +BT_FN_SIZE_CONST_PTR_INT,
> +ATTR_PURE_NOTHROW_LEAF_LIST)
> +
> #undef DEF_SANITIZER_BUILTIN
> }
>
>
> Marek
Marek
[PATCH] Fix PR63704
The following fixes the assert in mems_in_disjoint_alias_sets_p trigger.
What the comment says can easily happen by using
attribute((optimize("O3"))) inside a -fno-strict-aliasing TU. Dependent
on luck a global variable may get a alias-set non-zero MEM if expanded
from the -O3 context and thus the assert triggers for uses in a
-fno-strict-aliasing function.
Fixed by ignoring recorded alias-sets for -fno-strict-aliasing like
we do everywhere else.
Another possibility is to not allow changing -fstrict-aliasing
via the optimize attribute (but then the same may happen via
LTO of a -O0 and a -O3 unit and pre-recorded type mems).
Bootstrap & regtest pending on x86_64-unknown-linux-gnu.
Richard.
2014-11-26 Richard Biener
PR middle-end/63704
* alias.c (mems_in_disjoint_alias_sets_p): Remove assert
and instead return false when !fstrict-aliasing.
Index: gcc/alias.c
===
--- gcc/alias.c (revision 218078)
+++ gcc/alias.c (working copy)
@@ -417,17 +417,9 @@ get_alias_set_entry (alias_set_type alia
static inline int
mems_in_disjoint_alias_sets_p (const_rtx mem1, const_rtx mem2)
{
-/* Perform a basic sanity check. Namely, that there are no alias sets
- if we're not using strict aliasing. This helps to catch bugs
- whereby someone uses PUT_CODE, but doesn't clear MEM_ALIAS_SET, or
- where a MEM is allocated in some way other than by the use of
- gen_rtx_MEM, and the MEM_ALIAS_SET is not cleared. If we begin to
- use alias sets to indicate that spilled registers cannot alias each
- other, we might need to remove this check. */
- gcc_assert (flag_strict_aliasing
- || (!MEM_ALIAS_SET (mem1) && !MEM_ALIAS_SET (mem2)));
-
- return ! alias_sets_conflict_p (MEM_ALIAS_SET (mem1), MEM_ALIAS_SET (mem2));
+ return (flag_strict_aliasing
+ && ! alias_sets_conflict_p (MEM_ALIAS_SET (mem1),
+ MEM_ALIAS_SET (mem2)));
}
/* Return true if the first alias set is a subset of the second. */
Re: [PATCH] Fix PR bootstrap/63995
On Wed, Nov 26, 2014 at 04:48:13PM +0300, Ilya Enkovich wrote: > 2014-11-26 Ilya Enkovich > > PR bootstrap/63995 > * tree-chkp-opt.c (chkp_reduce_bounds_lifetime): Ignore > debug statement when searching for a new position for > bounds load/creation statement. > > gcc/testsuite/ > > 2014-11-26 Ilya Enkovich > > PR bootstrap/63995 > * gcc.target/i386/pr63995-2.c: New. Ok, thanks. Jakub
Re: [PATCH] Fix PR bootstrap/63995
On 26 Nov 13:46, Jakub Jelinek wrote:
> On Wed, Nov 26, 2014 at 03:41:46PM +0300, Ilya Enkovich wrote:
> > Hi,
> >
> > This patch makes optimization for bounds lifetime reduction to ignore
> > debug stetments. This fixes stage2 and stage3 comparision for
> > instrumented boostrap. OK for trunk?
>
> Please add a small testcase (with -fcompare-debug -fcheck-pointer-bounds (or
> what
> other options you need to reproduce it) that fails without the patch and
> succeeds with it.
>
> > 2014-11-26 Ilya Enkovich
> >
> > PR bootstrap/63995
> > * tree-chkp-opt.c (chkp_reduce_bounds_lifetime): Ignore
> > debug statement when searching for a new position for
> > bounds load/creation statement.
> >
> >
> > diff --git a/gcc/tree-chkp-opt.c b/gcc/tree-chkp-opt.c
> > index ff390d7..92e0694 100644
> > --- a/gcc/tree-chkp-opt.c
> > +++ b/gcc/tree-chkp-opt.c
> > @@ -1175,6 +1175,9 @@ chkp_reduce_bounds_lifetime (void)
> >
> >FOR_EACH_IMM_USE_STMT (use_stmt, use_iter, op)
> > {
> > + if (is_gimple_debug (use_stmt))
> > + continue;
> > +
> > if (dom_bb &&
> > dominated_by_p (CDI_DOMINATORS,
> > dom_bb, gimple_bb (use_stmt)))
>
> Jakub
Here is a version with a test added.
Thanks,
Ilya
--
gcc/
2014-11-26 Ilya Enkovich
PR bootstrap/63995
* tree-chkp-opt.c (chkp_reduce_bounds_lifetime): Ignore
debug statement when searching for a new position for
bounds load/creation statement.
gcc/testsuite/
2014-11-26 Ilya Enkovich
PR bootstrap/63995
* gcc.target/i386/pr63995-2.c: New.
diff --git a/gcc/testsuite/gcc.target/i386/pr63995-2.c
b/gcc/testsuite/gcc.target/i386/pr63995-2.c
new file mode 100644
index 000..7c22e62
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr63995-2.c
@@ -0,0 +1,28 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target mpx } */
+/* { dg-options "-O2 -g -fcheck-pointer-bounds -mmpx -fcompare-debug" } */
+
+struct ts
+{
+ int field;
+};
+
+extern void test1 ();
+extern void test2 (struct ts *);
+
+static void
+init (struct ts *c)
+{
+ c->field = -1;
+}
+
+struct ts
+test3 (const struct ts *other)
+{
+ struct ts r;
+ if (other->field != 0)
+test1 ();
+ init (&r);
+ test2 (&r);
+ return r;
+}
diff --git a/gcc/tree-chkp-opt.c b/gcc/tree-chkp-opt.c
index ff390d7..92e0694 100644
--- a/gcc/tree-chkp-opt.c
+++ b/gcc/tree-chkp-opt.c
@@ -1175,6 +1175,9 @@ chkp_reduce_bounds_lifetime (void)
FOR_EACH_IMM_USE_STMT (use_stmt, use_iter, op)
{
+ if (is_gimple_debug (use_stmt))
+ continue;
+
if (dom_bb &&
dominated_by_p (CDI_DOMINATORS,
dom_bb, gimple_bb (use_stmt)))
[v3] XFAIL 27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc on Solaris (PR libstdc++/64054)
The 27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc currently
FAILs on Solaris
FAIL: 27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc execution test
As determined in the PR, this happens because Solaris only supports
hexfloats in C99 mode, but gcc doesn't correctly link with values-xpg6.o
in that case (PR target/40411). Until that is fixed, the test is
XFAILed.
Tested with the appropriate runtest invocations on i386-pc-solaris2.11
and x86_64-unknown-linux-gnu, installed on mainline as preapproved by
Jonathan in the PR.
Rainer
2014-11-26 Jonathan Wakely
Rainer Orth
PR libstdc++/64054
* testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc:
XFAIL execution on *-*-solaris*.
# HG changeset patch
# Parent f115d90509c3d6ec3a202b6d84c3f9f9e05de993
XFAIL 27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc on Solaris (PR target/64054)
diff --git a/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc b/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc
--- a/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc
+++ b/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc
@@ -1,5 +1,6 @@
// { dg-options "-std=gnu++11" }
// { dg-require-string-conversions "" }
+// { dg-xfail-run-if "PR libstdc++/64054" { *-*-solaris* } }
// 2014-03-27 RÃŒdiger Sonderfeld
// test the hexadecimal floating point inserters (facet num_put)
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
[testsuite, i386] XFAIL gcc.target/i386/mcount_pic.c etc. on Solaris
Two tests scanning for get_pc_thunk were FAILing on Solaris/x86:
FAIL: gcc.target/i386/mcount_pic.c (test for excess errors)
WARNING: gcc.target/i386/mcount_pic.c compilation failed to produce executable
FAIL: gcc.target/i386/mcount_pic.c scan-assembler get_pc_thunk
FAIL: gcc.target/i386/pr63620.c scan-assembler get_pc_thunk
The first one initially failed due to the use of -p: on Solaris, this
requires mcrt1.o, which is only distributed with the Studio compilers,
while gcrt1.o (for -pg) is included either with the OS or comes from
libgcc. On Linux/x86, both -p and -pg work, so I've changed the test to
use the latter.
After that, the scans for get_pc_thunk FAILed, explained by this comment
in i386/sol2.h:
/* Only recent versions of Solaris 11 ld properly support hidden .gnu.linkonce
sections, so don't use them. */
#ifndef USE_GLD
#define USE_HIDDEN_LINKONCE 0
#endif
The following patch fixes those issues by compiling with -pg and
XFAILing the scan-assembler tests on Solaris/x86 without gld.
Tested with the appropriate runtest invocations on i386-pc-solaris2.11
(with and without gld), and x86_64-unknown-linux-gnu (both 32 and 64-bit
multilibs). Installed on mainline.
Rainer
2014-11-07 Rainer Orth
* gcc.target/i386/pr63620.c: XFAIL get_pc_thunk scan on
*-*-solaris* && !gld.
* gcc.target/i386/mcount_pic.c: Likewise.
Use -pg.
# HG changeset patch
# Parent aeefb237cf7036cddb5dea755c6cfd987e7c8a0f
Skip gcc.target/i386/mcount_pic.c on Solaris
diff --git a/gcc/testsuite/gcc.target/i386/mcount_pic.c b/gcc/testsuite/gcc.target/i386/mcount_pic.c
--- a/gcc/testsuite/gcc.target/i386/mcount_pic.c
+++ b/gcc/testsuite/gcc.target/i386/mcount_pic.c
@@ -3,7 +3,7 @@
/* { dg-do run } */
/* { dg-require-effective-target fpic } */
/* { dg-require-effective-target ia32 } */
-/* { dg-options "-O2 -fpic -p -save-temps" } */
+/* { dg-options "-O2 -fpic -pg -save-temps" } */
int main ()
{
@@ -11,5 +11,5 @@ int main ()
}
/* { dg-final { scan-assembler "mcount" } } */
-/* { dg-final { scan-assembler "get_pc_thunk" } } */
+/* { dg-final { scan-assembler "get_pc_thunk" { xfail { *-*-solaris* && { ! gld } } } } } */
/* { dg-final { cleanup-saved-temps } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr63620.c b/gcc/testsuite/gcc.target/i386/pr63620.c
--- a/gcc/testsuite/gcc.target/i386/pr63620.c
+++ b/gcc/testsuite/gcc.target/i386/pr63620.c
@@ -17,4 +17,4 @@ test (__float128 x, int p, func f)
return x;
}
-/* { dg-final { scan-assembler "get_pc_thunk" } } */
+/* { dg-final { scan-assembler "get_pc_thunk" { xfail { *-*-solaris* && { ! gld } } } } } */
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH][AArch64] Remove crypto extension from default for cortex-a53, cortex-a57
On Tuesday 2014-11-25 16:08, Kyrill Tkachov wrote: The change is to the behaviour of -mcpu, not -march. -march is only mentioned as a way of getting the previous behaviour if the user so wishes. Ah, okay. How about this amendment? Where you have "add the +crypto extension to the value" I suggest to just say "add +crypto to the value". That avoids using extension twice in the same sentence, for two different things. Go ahead and commit then. Thanks! Gerald
[PING] Fix gcc_assert in expand_omp_for_static_chunk
On 12-11-14 11:00, Tom de Vries wrote:
Jakub,
this patch fixes a gcc_assert in expand_omp_for_static_chunk.
The assert follows a loop with composite loop condition:
...
vec *head = redirect_edge_var_map_vector (re);
ene = single_succ_edge (entry_bb);
psi = gsi_start_phis (fin_bb);
for (i = 0; !gsi_end_p (psi) && head->iterate (i, &vm);
gsi_next (&psi), ++i)
...
AFAIU, the intention of the assert is that it tries to check that both:
- all phis have been handled (gsi_end_p (psi)), and
- all elements of head have been used (head->length () == i).
In other words, that we have stopped iterating because both loop conditions are
false.
The current assert checks that *not* all phis have been handled:
...
gcc_assert (!gsi_end_p (psi) && i == head->length ());
...
Looking back in the history, it seems we started out with the 'all phis handled'
semantics, but I suspect that that got lost due to a typo:
...
79acaae1 2007-09-07
gcc_assert (!phi && !args);
75a70cf95 2008-07-28
gcc_assert (!gsi_end_p (psi) && i == VEC_length (edge_var_map, head));
f1f41a6c 2012-11-18
gcc_assert (!gsi_end_p (psi) && i == head->length ());
...
Now, if the current assert is incorrect, why didn't it trigger?
The assert is in ssa-handling code in expand_omp_for_static_chunk. Ssa-handling
code in omp-low.c is only triggered by pass_parallelize_loops, and that pass
doesn't specify a chunk size on the GIMPLE_OMP_FOR it constructs, so that will
only call expand_omp_for_static_nochunk.
I managed to trigger this assert in my oacc kernels directive patch set (on top
of the gomp-4_0-branch), which constructs an oacc for loop in
pass_parallelize_loops, and then this code in gomp-4_0-branch has the effect
that we trigger expand_omp_for_static_chunk:
...
//TODO
/* For OpenACC loops, force a chunk size of one, as this avoids the default
scheduling where several subsequent iterations are being executed by the
same thread. */
if (gimple_omp_for_kind (for_stmt) == GF_OMP_FOR_KIND_OACC_LOOP)
{
gcc_assert (fd->chunk_size == NULL_TREE);
fd->chunk_size = build_int_cst (TREE_TYPE (fd->loop.v), 1);
}
...
So, AFAIU, this assert (and associated ssa-handling code in
expand_omp_for_static_chunk) is dead on trunk, but I'm excercising the code
currently in my patch series, so I'd prefer to fix it rather than remove it.
Bootstrapped and reg-tested on x86_64, on top of trunk, gomp-4_0-branch and
internal oacc dev branch.
OK for trunk?
Ping.
Thanks,
- Tom
0001-Fix-gcc_assert-in-expand_omp_for_static_chunk.patch
2014-11-12 Tom de Vries
* omp-low.c (expand_omp_for_static_chunk): Fix assert.
---
gcc/omp-low.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index b59d069..5210de1 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -6775,7 +6775,7 @@ expand_omp_for_static_chunk (struct omp_region *region,
locus = redirect_edge_var_map_location (vm);
add_phi_arg (nphi, redirect_edge_var_map_def (vm), re, locus);
}
- gcc_assert (!gsi_end_p (psi) && i == head->length ());
+ gcc_assert (gsi_end_p (psi) && i == head->length ());
redirect_edge_var_map_clear (re);
while (1)
{
-- 1.9.1
Re: [patch] Flatten streamer header files Pt. 1
On 11/20/2014 03:14 PM, Andrew MacLeod wrote: On 11/20/2014 03:05 PM, Michael Collison wrote: This is a part one of two part patch that flattens gimple-streamer.h, lto-streamer.h and tree-streamer.h. This work is part of the GCC Re-Architecture effort being led by Andrew MacLeod. In gimple-streamer.h I moved all exports for gimple-streamer-in.c to a new file gimple-streamer-in.h. I also moved all exports for gimple-streamer-out.c to a new file gimple-streamer-out.h. Finally because gimple-streamer.h only contained exports for gimple-streamer-in.c and gimple-streamer-out.c I removed the file. In lto-streamer.h I moved all exports for lto-streamer-in.c to a new file lto-streamer-in.h. I also moved all exports for lto-streamer-out.c to a new file gimple-streamer-out.h. In tree-streamer.h I moved all exports for tree-streamer-in.c to a new file tree-streamer-in.h. I also moved all exports for tree-streamer-out.c to a new file tree-streamer-out.h. As a result of the flattening I had to add new include files to gengtype.c. I performed a full bootstrap with all languages on x86-linux. I also bootstrapped on all targets listed in contrib/config-list.mk with c and c++ enabled. Is this okay for trunk? Yes, so thats the question... Flattening ought not affect code generation nor anything else since its just a simple restructure of includes. We could maintain these in a branch, but due to the nature of the changes, they can bit rot really quickly so its nicer to get them into trunk (especially with other large changes still going in) So the question to the maintainers is whether its permissible to do a bit of flattening into the early parts of stage 3, or whether you'd rather it stay on a branch until next stage 1. Andrew PIng.. anyone want to chime in? :-) Andrew
Re: [PATCH] Fix PR bootstrap/63995
On Wed, Nov 26, 2014 at 03:41:46PM +0300, Ilya Enkovich wrote:
> Hi,
>
> This patch makes optimization for bounds lifetime reduction to ignore
> debug stetments. This fixes stage2 and stage3 comparision for
> instrumented boostrap. OK for trunk?
Please add a small testcase (with -fcompare-debug -fcheck-pointer-bounds (or
what
other options you need to reproduce it) that fails without the patch and
succeeds with it.
> 2014-11-26 Ilya Enkovich
>
> PR bootstrap/63995
> * tree-chkp-opt.c (chkp_reduce_bounds_lifetime): Ignore
> debug statement when searching for a new position for
> bounds load/creation statement.
>
>
> diff --git a/gcc/tree-chkp-opt.c b/gcc/tree-chkp-opt.c
> index ff390d7..92e0694 100644
> --- a/gcc/tree-chkp-opt.c
> +++ b/gcc/tree-chkp-opt.c
> @@ -1175,6 +1175,9 @@ chkp_reduce_bounds_lifetime (void)
>
>FOR_EACH_IMM_USE_STMT (use_stmt, use_iter, op)
> {
> + if (is_gimple_debug (use_stmt))
> + continue;
> +
> if (dom_bb &&
> dominated_by_p (CDI_DOMINATORS,
> dom_bb, gimple_bb (use_stmt)))
Jakub
[PATCH] Fix PR bootstrap/63995
Hi,
This patch makes optimization for bounds lifetime reduction to ignore debug
stetments. This fixes stage2 and stage3 comparision for instrumented boostrap.
OK for trunk?
Thanks,
Ilya
--
2014-11-26 Ilya Enkovich
PR bootstrap/63995
* tree-chkp-opt.c (chkp_reduce_bounds_lifetime): Ignore
debug statement when searching for a new position for
bounds load/creation statement.
diff --git a/gcc/tree-chkp-opt.c b/gcc/tree-chkp-opt.c
index ff390d7..92e0694 100644
--- a/gcc/tree-chkp-opt.c
+++ b/gcc/tree-chkp-opt.c
@@ -1175,6 +1175,9 @@ chkp_reduce_bounds_lifetime (void)
FOR_EACH_IMM_USE_STMT (use_stmt, use_iter, op)
{
+ if (is_gimple_debug (use_stmt))
+ continue;
+
if (dom_bb &&
dominated_by_p (CDI_DOMINATORS,
dom_bb, gimple_bb (use_stmt)))
Re: [PATCH, PR63995, CHKP] Use single static bounds var for varpool nodes sharing asm name
On 25 Nov 15:03, Ilya Enkovich wrote:
> 2014-11-25 14:11 GMT+03:00 Richard Biener :
> > On Tue, Nov 25, 2014 at 11:19 AM, Ilya Enkovich
> > wrote:
> >
> > Ok, then it's get_for_asmname (). That said - the above loops look
> > bogus to me. Honza - any better ideas?
>
> get_for_asmname () returns the first element in a chain of nodes with
> the same asm name. May I rely on the order of nodes in this chain?
> Probably use ASSEMBLER_NAME as a key in chkp_static_var_bounds hash?
>
> Thanks,
> Ilya
>
> >
> > Richard.
> >
A variant with var's ASSEMBLER_NAME as a key works fine. Instrumented
bootstrap passes. OK for trunk?
Thanks,
Ilya
--
gcc/
2014-11-26 Ilya Enkovich
PR bootstrap/63995
* tree-chkp.c (chkp_make_static_bounds): Share bounds var
between nodes sharing assembler name.
gcc/testsuite
2014-11-26 Ilya Enkovich
PR bootstrap/63995
* g++.dg/dg.exp: Add mpx-dg.exp.
* g++.dg/pr63995-1.C: New.
diff --git a/gcc/testsuite/g++.dg/dg.exp b/gcc/testsuite/g++.dg/dg.exp
index 14beae1..44eab0c 100644
--- a/gcc/testsuite/g++.dg/dg.exp
+++ b/gcc/testsuite/g++.dg/dg.exp
@@ -18,6 +18,7 @@
# Load support procs.
load_lib g++-dg.exp
+load_lib mpx-dg.exp
# If a testcase doesn't have special options, use these.
global DEFAULT_CXXFLAGS
diff --git a/gcc/testsuite/g++.dg/pr63995-1.C b/gcc/testsuite/g++.dg/pr63995-1.C
new file mode 100644
index 000..82e7606
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr63995-1.C
@@ -0,0 +1,16 @@
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-require-effective-target mpx } */
+/* { dg-options "-O2 -g -fcheck-pointer-bounds -mmpx" } */
+
+int test1 (int i)
+{
+ extern const int arr[10];
+ return arr[i];
+}
+
+extern const int arr[10];
+
+int test2 (int i)
+{
+ return arr[i];
+}
diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index 3e38691..924cb71 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -2727,9 +2727,23 @@ chkp_make_static_bounds (tree obj)
/* First check if we already have required var. */
if (chkp_static_var_bounds)
{
- slot = chkp_static_var_bounds->get (obj);
- if (slot)
- return *slot;
+ /* For vars we use assembler name as a key in
+chkp_static_var_bounds map. It allows to
+avoid duplicating bound vars for decls
+sharing assembler name. */
+ if (TREE_CODE (obj) == VAR_DECL)
+ {
+ tree name = DECL_ASSEMBLER_NAME (obj);
+ slot = chkp_static_var_bounds->get (name);
+ if (slot)
+ return *slot;
+ }
+ else
+ {
+ slot = chkp_static_var_bounds->get (obj);
+ if (slot)
+ return *slot;
+ }
}
/* Build decl for bounds var. */
@@ -2793,7 +2807,13 @@ chkp_make_static_bounds (tree obj)
if (!chkp_static_var_bounds)
chkp_static_var_bounds = new hash_map;
- chkp_static_var_bounds->put (obj, bnd_var);
+ if (TREE_CODE (obj) == VAR_DECL)
+{
+ tree name = DECL_ASSEMBLER_NAME (obj);
+ chkp_static_var_bounds->put (name, bnd_var);
+}
+ else
+chkp_static_var_bounds->put (obj, bnd_var);
return bnd_var;
}
Re: [arm][patch] fix arm_neon_ok check on !arm_arch7
On 14/11/14 11:12, Andrew Stubbs wrote: On 07/11/14 10:35, Andrew Stubbs wrote: if armv6 never co-exist with NEON, personally I think your original patch is better because TARGET_NEON generally will be used when all options are processed. any way, this needs gate keeper's approval. Ping, Richard. Ping. Ping. Andrew
Re: [Ping]Re: [PR63762][4.9] Backport the patch which fixes "GCC generates UNPREDICTABLE STR with Rn = Rt for arm"
On Wed, Nov 26, 2014 at 4:07 AM, Renlin Li wrote: > On 20/11/14 16:17, Renlin Li wrote: >> >> Hi all, >> >> This is a backport for gcc-4_9-branch of the patch "[PR63762]GCC generates >> UNPREDICTABLE STR with Rn = Rt for arm" posted in: >> https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02253.html >> >> arm-none-eabi has been test on the model, no new issues. bootstrapping and >> regression tested on x86, no new issues. >> >> Is it Okay for gcc-4_9-branch? >> >> gcc/ChangeLog: >> >> 2014-11-20 Renlin Li >> >> PR middle-end/63762 >> * ira.c (ira): Update preferred class. >> >> gcc/testsuite/ChangeLog: >> >> 2014-11-20 Renlin Li >> >> PR middle-end/63762 >> * gcc.dg/pr63762.c: New. > > Ping for it. > Please verify if it is the real fix for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63661 If yes, please add a testcase for PR 63661 and mention it in your ChangeLog entry. Thanks. -- H.J.
