Re: Request to merge Undefined Behavior Sanitizer in (take 2)
Ping.
On Wed, Aug 07, 2013 at 06:12:58PM +0200, Marek Polacek wrote:
> On Wed, Aug 07, 2013 at 10:24:59AM -0400, Jason Merrill wrote:
> > On 08/07/2013 06:06 AM, Marek Polacek wrote:
> > >I might've misunderstood what you mean. If we drop the hunk above,
> > >then we'll call
> > > error ("%q+E is not a constant expression", t);
> > >so, we'll print "is not a constant expression" no matter what
> >
> > Yes, that's fine; 1/0 is not a constant expression, because it has
> > undefined behavior.
>
> Indeed.
>
> > Print something more meaningful to the user.
>
> Something along these lines? (Not including CL/testcase yet on purpose.)
> In beforementioned situation it'd print:
>
> w.C:7:22: error: ‘’ is not a constant expression
>case 0 * (1 / 0):
> ^
> I'm not entirely happy about it, on the other hand, this situation
> should be very rare...
>
> diff --git a/gcc/cp/error.c b/gcc/cp/error.c
> index 440169a..db50b5f 100644
> --- a/gcc/cp/error.c
> +++ b/gcc/cp/error.c
> @@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. If not see
> #include "tree-pretty-print.h"
> #include "pointer-set.h"
> #include "c-family/c-objc.h"
> +#include "ubsan.h"
>
> #define pp_separate_with_comma(PP) pp_cxx_separate_with (PP, ',')
> #define pp_separate_with_semicolon(PP) pp_cxx_separate_with (PP, ';')
> @@ -1972,6 +1973,12 @@ dump_expr (tree t, int flags)
> }
> skipfirst = true;
> }
> + if (flag_sanitize & SANITIZE_UNDEFINED
> + && is_ubsan_builtin_p (fn))
> + {
> + pp_string (cxx_pp, M_(""));
> + break;
> + }
> dump_expr (fn, flags | TFF_EXPR_IN_PARENS);
> dump_call_expr_args (t, flags, skipfirst);
>}
> diff --git a/gcc/ubsan.c b/gcc/ubsan.c
> index 8135cc9..565758d 100644
> --- a/gcc/ubsan.c
> +++ b/gcc/ubsan.c
> @@ -456,3 +456,13 @@ ubsan_instrument_unreachable (location_t loc)
>tree t = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE);
>return build_call_expr_loc (loc, t, 1, build_fold_addr_expr_loc (loc,
> data));
> }
> +
> +/* Return true if T is a call to a libubsan routine. */
> +
> +bool
> +is_ubsan_builtin_p (tree t)
> +{
> + gcc_checking_assert (TREE_CODE (t) == FUNCTION_DECL);
> + return strncmp (IDENTIFIER_POINTER (DECL_NAME (t)),
> + "__builtin___ubsan_", 18) == 0;
> +}
> diff --git a/gcc/ubsan.h b/gcc/ubsan.h
> index abf4f5d..3553a6c 100644
> --- a/gcc/ubsan.h
> +++ b/gcc/ubsan.h
> @@ -25,6 +25,7 @@ extern tree ubsan_instrument_unreachable (location_t);
> extern tree ubsan_create_data (const char *, location_t, ...);
> extern tree ubsan_type_descriptor (tree);
> extern tree ubsan_encode_value (tree);
> +extern bool is_ubsan_builtin_p (tree);
>
> #endif /* GCC_UBSAN_H */
Marek
Re: [PATCH, gcc-4_8-branch] Fix the issue of incorrectly using #ifdef HAVE_ATTR_enabled
2013/8/15 Jakub Jelinek : > On Thu, Aug 15, 2013 at 04:09:18AM +0800, Chung-Ju Wu wrote: >> According to this discussion thread: >> http://gcc.gnu.org/ml/gcc/2013-08/msg00142.html >> >> The bug that David mentioned has been fixed on trunk but not on 4.8 branch. >> IMHO it would be a good idea to backport it and Vladmir agreed with that. > > Generally maintainers can ack backports to release branches, RM ack isn't > needed, unless the branch is frozen. This is ok. > > Jakub Thanks for clarifying it. I will follow this ack policy for other patches. Also, thank your and Vlad's approval. :) Committed into gcc-4_8-branch as Rev.201764 http://gcc.gnu.org/r201764 Best regards, jasonwucj
Re: [PATCH] Redesign pthread in LIB_SPEC for systems without libpthread
Could anybody please take a look? This is important for building gcc for android. ping^4 thanks, Alexander 2013/8/15 Alexander Ivchenko : > Could anybody please take a look? This is important for building gcc for > android. > > ping^4 > thanks, > Alexander > > > 2013/5/28 Pavel Chupin >> >> On Thu, May 16, 2013 at 10:49 AM, Pavel Chupin >> wrote: >> > On Mon, Apr 29, 2013 at 5:36 PM, Alexander Ivchenko >> > wrote: >> >> *ping* >> >> >> >> thank you, >> >> Alexander >> >> >> >> 2013/4/15 Pavel Chupin : >> >>> On Tue, Apr 2, 2013 at 1:59 PM, Pavel Chupin >> >>> wrote: >> On Mon, Apr 1, 2013 at 7:07 PM, Pavel Chupin >> wrote: >> > On Android pthread is integrated into libc. >> > Attached patch fixes configures for this case by trying to build >> > test >> > without -pthread -lpthread. >> > >> > 2013-04-01 Pavel Chupin >> > >> > Fix libatomic and libgomp configure for systems without >> > libpthread >> > * libatomic/configure.ac: Add test without -pthread >> > -lpthread. >> > * libgomp/configure.ac: Ditto. >> > * libatomic/configure: Regenerate. >> > * libgomp/configure: Regenerate. >> > >> > OK for trunk? >> > >> >> I think I made a better fix: >> >> 2013-04-02 Pavel Chupin >> >> Redesign pthread in LIB_SPEC for systems without libpthread >> * gcc/config/gnu-user.h: Remove pthread from >> GNU_USER_TARGET_LIB_SPEC >> but keep in default LIB_SPEC >> * gcc/config/linux-android.h: Add pthread to ANDROID_LIB_SPEC >> >> Is it OK for trunk? >> >>> >> >>> Ping >> >>> >> > >> > Ping. >> > >> >> Ping. >> >> -- >> Pavel Chupin >> Intel Corporation > >
Re: Request to merge Undefined Behavior Sanitizer in (take 2)
On Wed, Aug 07, 2013 at 04:58:03PM +0200, Marek Polacek wrote:
> I actually don't know what I prefer more, but if you like this version
> more, I'll go with it. Maybe this is better because we don't have to
> create SAVE_EXPR and also we avoid one fold_build call. Thanks,
Not creating the SAVE_EXPRs turned to be a no-go: firstly, the logic
when to create the SAVE_EXPRs and when not is hard-to-follow (depends
on C/C++, depends on various standards and it also depends on whether
the types are signed or not) - both for shift and division. Moreover,
when we have nested expressions like e.g.
(i << u) << x
if any of the operands is wrapped in SAVE_EXPR, we get an
-Wuninitialized warning. So, what I did is to evaluate
the op0 always in the condition, like "if (op0, )",
which is safe and all the uninitialized warnings are gone,
now the bootstrap with -fsanitize=undefined finally finishes.
(albeit with
Comparing stages 2 and 3
warning: gcc/cc1plus-checksum.o differs
warning: gcc/cc1-checksum.o differs
Bootstrap comparison failure!
gcc/combine.o differs
gcc/tree-ssa-loop-ivopts.o differs
gcc/rtlanal.o differs
gcc/calls.o differs
gcc/emit-rtl.o differs
gcc/dbxout.o differs
gcc/function.o differs
gcc/cfgexpand.o differs
gcc/stor-layout.o differs
gcc/tree.o differs
gcc/tree-vect-generic.o differs
gcc/expr.o differs
gcc/fixed-value.o differs
gcc/fold-const.o differs
gcc/i386.o differs
libdecnumber/decimal64.o differs
libdecnumber/decNumber.o differs
make[2]: *** [compare] Error 1
).
Tested x86_64-pc-linux-gnu, applying to the ubsan branch.
2013-08-15 Marek Polacek
* c-ubsan.c (ubsan_instrument_division): Evaluate SAVE_EXPRs
before the condition.
(ubsan_instrument_shift): Likewise.
* c-c++-common/ubsan/save-expr-1.c: New test.
* c-c++-common/ubsan/save-expr-2.c: New test.
* c-c++-common/ubsan/save-expr-3.c: New test.
* c-c++-common/ubsan/save-expr-4.c: New test.
--- gcc/c-family/c-ubsan.c.mp 2013-08-15 10:44:51.189966522 +0200
+++ gcc/c-family/c-ubsan.c 2013-08-15 12:43:30.454523049 +0200
@@ -73,6 +73,10 @@ ubsan_instrument_division (location_t lo
x = fold_build2 (TRUTH_AND_EXPR, boolean_type_node, x, tt);
t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, t, x);
}
+
+ /* In case we have a SAVE_EXPR in a conditional context, we need to
+ make sure it gets evaluated before the condition. */
+ t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), op0, t);
tree data = ubsan_create_data ("__ubsan_overflow_data",
loc, ubsan_type_descriptor (type),
NULL_TREE);
@@ -133,6 +137,10 @@ ubsan_instrument_shift (location_t loc,
build_int_cst (type0, 0));
tt = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, x, tt);
}
+
+ /* In case we have a SAVE_EXPR in a conditional context, we need to
+ make sure it gets evaluated before the condition. */
+ t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), op0, t);
tree data = ubsan_create_data ("__ubsan_shift_data",
loc, ubsan_type_descriptor (type0),
ubsan_type_descriptor (type1), NULL_TREE);
--- gcc/testsuite/c-c++-common/ubsan/save-expr-1.c.mp 2013-08-15
10:45:37.377057713 +0200
+++ gcc/testsuite/c-c++-common/ubsan/save-expr-1.c 2013-08-15
10:53:01.601695980 +0200
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=shift -Wall -Werror -O" } */
+
+static int x;
+int
+main (void)
+{
+ int o = 1;
+ int y = x << o;
+ return y;
+}
--- gcc/testsuite/c-c++-common/ubsan/save-expr-2.c.mp 2013-08-15
10:58:37.505938910 +0200
+++ gcc/testsuite/c-c++-common/ubsan/save-expr-2.c 2013-08-15
10:58:30.258913139 +0200
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=shift -Wall -Werror -O" } */
+
+int
+foo (int i, unsigned int u)
+{
+ return u / i;
+}
+
+int
+bar (int i, unsigned int u)
+{
+ return u % i;
+}
--- gcc/testsuite/c-c++-common/ubsan/save-expr-3.c.mp 2013-08-15
11:02:39.42875 +0200
+++ gcc/testsuite/c-c++-common/ubsan/save-expr-3.c 2013-08-15
11:02:34.874127652 +0200
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=shift -Wall -Werror -O" } */
+
+int x;
+
+int
+foo (int i, int u)
+{
+ return (i << u) << x;
+}
+
+int
+bar (int i, int u)
+{
+ return (i >> u) >> x;
+}
--- gcc/testsuite/c-c++-common/ubsan/save-expr-4.c.mp 2013-08-15
11:09:53.453746629 +0200
+++ gcc/testsuite/c-c++-common/ubsan/save-expr-4.c 2013-08-15
11:09:45.238716728 +0200
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=shift -Wall -Werror -O" } */
+
+int x;
+
+int
+foo (int i, unsigned int u)
+{
+ return (i % u) << (x / u);
+}
+
+int
+bar (int i, unsigned int u)
+{
+ return (((x % u) << (u / i)) >> x);
+}
Marek
[PATCH,ARM] fix testsuite failures for arm-none-linux-gnueabihf
Hi
The attached patch fixes some tests which fail when testing gcc for a
arm-none-linux-gnueabihf target because they do not expect to be built
with a hard float ABI.
The change in target-supports.exp fixes arm-fp16-ops-5.c and arm-fp16-ops-6.c.
Tested on arm-none-linux-gnueabihf using qemu-arm, and does not cause
any other tests to break.
Comments? This is my first patch, so please point out anything wrong.
2013-08-15 Charles Baylis
* gcc.dg/builtin-apply2.c: skip test on arm hardfloat ABI targets
* gcc.dg/tls/pr42894.c: Use -mfloat-abi=soft as Thumb1 does
not support hardfloat ABI
* arm/thumb-ltu.c: Use -mfloat-abi=soft as Thumb1 does not
support hardfloat ABI
* target-supports.exp: don't force -mfloat-abi=soft when
building for hardfloat target
Index: gcc/testsuite/gcc.dg/builtin-apply2.c
===
--- gcc/testsuite/gcc.dg/builtin-apply2.c (revision 201726)
+++ gcc/testsuite/gcc.dg/builtin-apply2.c (working copy)
@@ -1,6 +1,7 @@
/* { dg-do run } */
/* { dg-skip-if "Variadic funcs have all args on stack. Normal funcs have args
in registers." { "aarch64*-*-* avr-*-* " } { "*" } { "" } } */
/* { dg-skip-if "Variadic funcs use Base AAPCS. Normal funcs use VFP
variant." { "arm*-*-*" } { "-mfloat-abi=hard" } { "" } } */
+/* { dg-skip-if "Variadic funcs use Base AAPCS. Normal funcs use VFP
variant." { "arm*-*-gnueabihf" } { "*" } { "-mfloat-abi=soft*" } } */
/* PR target/12503 */
/* Origin: */
Index: gcc/testsuite/gcc.dg/tls/pr42894.c
===
--- gcc/testsuite/gcc.dg/tls/pr42894.c (revision 201726)
+++ gcc/testsuite/gcc.dg/tls/pr42894.c (working copy)
@@ -1,6 +1,7 @@
/* PR target/42894 */
/* { dg-do compile } */
/* { dg-options "-march=armv5te -mthumb" { target arm*-*-* } } */
+/* { dg-options "-march=armv5te -mthumb -mfloat-abi=soft" { target arm*-*-*hf
} } */
/* { dg-require-effective-target tls } */
extern __thread int t;
Index: gcc/testsuite/gcc.target/arm/thumb-ltu.c
===
--- gcc/testsuite/gcc.target/arm/thumb-ltu.c(revision 201726)
+++ gcc/testsuite/gcc.target/arm/thumb-ltu.c(working copy)
@@ -1,6 +1,6 @@
/* { dg-do compile } */
/* { dg-skip-if "incompatible options" { arm*-*-* } { "-march=*" } {
"-march=armv6" "-march=armv6j" "-march=armv6z" } } */
-/* { dg-options "-mcpu=arm1136jf-s -mthumb -O2" } */
+/* { dg-options "-mcpu=arm1136jf-s -mthumb -O2 -mfloat-abi=soft" } */
void f(unsigned a, unsigned b, unsigned c, unsigned d)
{
Index: gcc/testsuite/lib/target-supports.exp
===
--- gcc/testsuite/lib/target-supports.exp (revision 201726)
+++ gcc/testsuite/lib/target-supports.exp (working copy)
@@ -2445,6 +2445,11 @@
# Must generate floating-point instructions.
return 0
}
+if [check-flags [list "" { *-*-gnueabihf } { "*" } { "" } ]] {
+# Use existing float-abi and force an fpu which supports fp16
+ set et_arm_fp16_flags "-mfpu=vfpv4"
+ return 1;
+}
if [check-flags [list "" { *-*-* } { "-mfpu=*" } { "" } ]] {
# The existing -mfpu value is OK; use it, but add softfp.
set et_arm_fp16_flags "-mfloat-abi=softfp"
Request to merge Undefined Behavior Sanitizer in (take 3)
I've fixed a few unpleasant bugs; mainly the bug that introduced various uninitialized variable warnings was bothersome. Now it is possible to do bootstrap with -fsanitize=undefined, even though there are some comparison failures at the end. v3: - Fix Wuninitialized warnings because of SAVE_EXPRs http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00861.html - [ubsan] Add -lpthread to POSTSTAGE1_LDFLAGS http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00793.html - [ubsan] Fix assert in c-ubsan.c http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00802.html - [ubsan] Properly create const char type http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00213.html v2: - [ubsan] Add -static-libubsan http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01467.html - [ubsan] Don't try to sanitize shifts outside of functions http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01468.html - [ubsan] Use build_constructor_va where possible http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01469.html - [ubsan] Add bootstrap-ubsan.mk http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01470.html - [ubsan] Rename obsolete variable http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01473.html - [ubsan] Instrument expr only when doing shift or division http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01529.html - [ubsan] Improve documentation of -fsanitize=undefined http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01540.html - [ubsan] Add missing ubsan tests in g++.dg/dg.exp http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01563.html Regtested/bootstrapped on x86_64-linux and ppc64-linux. One patch is not in yet, but that isn't anything important: http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00376.html Ok to merge ubsan into trunk? Marek U.tar.bz2 Description: BZip2 compressed data
RE: [PATCH,ARM] fix testsuite failures for arm-none-linux-gnueabihf
Hi Charles,
CC'ing some of the ARM maintainers...
> Hi
>
> The attached patch fixes some tests which fail when testing gcc for a
> arm-none-linux-gnueabihf target because they do not expect to be built
> with a hard float ABI.
>
> The change in target-supports.exp fixes arm-fp16-ops-5.c and arm-fp16-
> ops-6.c.
>
> Tested on arm-none-linux-gnueabihf using qemu-arm, and does not cause
> any other tests to break.
>
> Comments? This is my first patch, so please point out anything wrong.
>
--- gcc/testsuite/gcc.dg/tls/pr42894.c (revision 201726)
+++ gcc/testsuite/gcc.dg/tls/pr42894.c (working copy)
@@ -1,6 +1,7 @@
/* PR target/42894 */
/* { dg-do compile } */
/* { dg-options "-march=armv5te -mthumb" { target arm*-*-* } } */
+/* { dg-options "-march=armv5te -mthumb -mfloat-abi=soft" { target arm*-*-*hf
} } */
/* { dg-require-effective-target tls } */
This test also fails for a bare-metal arm-none-eabi target configured with
hard float, so perhaps this could be:
-/* { dg-options "-march=armv5te -mthumb" { target arm*-*-* } } */
+/* { dg-options "-march=armv5te -mthumb -mfloat-abi=soft" { target arm*-*-* }
} */
>
>
>
> 2013-08-15 Charles Baylis
>
> * gcc.dg/builtin-apply2.c: skip test on arm hardfloat ABI targets
> * gcc.dg/tls/pr42894.c: Use -mfloat-abi=soft as Thumb1 does
> not support hardfloat ABI
> * arm/thumb-ltu.c: Use -mfloat-abi=soft as Thumb1 does not
> support hardfloat ABI
ChangeLog entries are specified relative to the ChangeLog location. This
should be:
* gcc.target/arm/thumb-ltu.c:...
> * target-supports.exp: don't force -mfloat-abi=soft when
> building for hardfloat target
Likewise, and also it's a good idea to specify which function/construct you're
changing. In this case I think you're modifying
check_effective_target_arm_fp16_ok_nocache. So this entry would be:
* lib/target-supports.exp
(check_effective_target_arm_fp16_ok_nocache):...
Thanks,
Kyrill
Re: Request to merge Undefined Behavior Sanitizer in (take 2)
OK. Jason
Re: Request to merge Undefined Behavior Sanitizer in (take 2)
On 08/15/2013 07:04 AM, Marek Polacek wrote: if any of the operands is wrapped in SAVE_EXPR, we get an -Wuninitialized warning. So, what I did is to evaluate the op0 always in the condition, like "if (op0, )", which is safe and all the uninitialized warnings are gone, That sounds fine. now the bootstrap with -fsanitize=undefined finally finishes. (albeit with Bootstrap comparison failure! That's not so fine. :) Jason
Re: [RFC] Bare bones of virtual call tracking
> On 08/14/2013 02:14 AM, Jan Hubicka wrote:
> >As a temporary hack I suppose I can rely on assembler name of virtual table
> >to be unique for each templated class?
>
> Actually, that seems like a fine solution for devirtualization; just
> compare the mangled name of the vtable to establish type identity.
OK,
does this seem to make sense? I checked that it gets rid of my
false ODR violation warnings (I disabled the code for types not
having virtual tables).
So if it looks OK, I would like to go ahead with this patch.
(I am testing it now in isolation at x86_64-linux).
Honza
Index: tree.c
===
--- tree.c (revision 201764)
+++ tree.c (working copy)
@@ -11833,15 +11833,15 @@ types_same_for_odr (tree type1, tree typ
if (type1 == type2)
return true;
- /* If types are not structuraly same, do not bother to contnue.
- Match in the remainder of code would mean ODR violation. */
- if (!types_compatible_p (type1, type2))
-return false;
-
+ /* Without LTO main variants of types are unique. */
#ifndef ENABLE_CHECKING
if (!in_lto_p)
return false;
#endif
+ /* If types are not structuraly same, do not bother to contnue.
+ Match in the remainder of code would mean ODR violation. */
+ if (!types_compatible_p (type1, type2))
+ return false;
/* Check for anonymous namespaces. Those have !TREE_PUBLIC
on the corresponding TYPE_STUB_DECL. */
@@ -11852,6 +11852,34 @@ types_same_for_odr (tree type1, tree typ
|| !TREE_PUBLIC (TYPE_STUB_DECL (type2
return false;
+ /* When assembler name of virtual table is available, it is
+ easy to compare types for equivalence.
+ FIXME: the code bellow consider all instantiations of the same
+ template to have same name. This is because we have no access
+ to template parameters.
+ To avoid false positives that may lead to wrong devirtualizations,
+ compoare also representative virtual tables.
+ We can still return false positive positives for types without
+ virtual tables, but for the moment we do not care. */
+ if (TYPE_BINFO (type1) && TYPE_BINFO (type2)
+ && BINFO_VTABLE (TYPE_BINFO (type1))
+ && BINFO_VTABLE (TYPE_BINFO (type2)))
+{
+ tree v1 = BINFO_VTABLE (TYPE_BINFO (type1));
+ tree v2 = BINFO_VTABLE (TYPE_BINFO (type2));
+
+ if (TREE_CODE (v1) == POINTER_PLUS_EXPR)
+ {
+ if (TREE_CODE (v2) != POINTER_PLUS_EXPR
+ || !operand_equal_p (TREE_OPERAND (v1, 1),
+ TREE_OPERAND (v2, 1), 0))
+ return false;
+ v1 = TREE_OPERAND (TREE_OPERAND (v1, 0), 0);
+ v2 = TREE_OPERAND (TREE_OPERAND (v2, 0), 0);
+ }
+ return DECL_ASSEMBLER_NAME (v1) == DECL_ASSEMBLER_NAME (v2);
+}
+
if (!TYPE_NAME (type1))
return false;
if (!decls_same_for_odr (TYPE_NAME (type1), TYPE_NAME (type2)))
Fix class type lookup from OBJ_TYPE_REF
Hi,
while working on the devirt code, I noticed that sometimes we get
as a type of object a non-classes (like void types). This is
because we look up object's class from the OBJ_TYPE_REF_OBJECT
and this is a generic pointer that gimple passes like to change
type of.
I tracked down few cases. For example when compiling passes.c one
gets copyrename changing
from
void execute_ipa_summary_passes(ipa_opt_pass_d*) (struct ipa_opt_pass_d *
ipa_pass)
{
...
struct opt_pass * pass
...
pass_12 = ipa_pass_1;
...
_20 = OBJ_TYPE_REF(_18;pass_12->3) (pass_12);
to
ipa_pass_12 = ipa_pass_1
...
_20 = OBJ_TYPE_REF(_18;ipa_pass_12->3) (ipa_pass_12);
Ths makes us to believe that OBJ_TYPE_REF reffers to call of
method of ipa_opt_pass_d instead of opt_pass. This is wrong and
I believe it may lead to wrong code, too - the tokens have
different meanings.
This patch changes code looking up OTR types to actually look
into OTR type (that is pointer to method) and look the class
type from type of the method's this pointer.
Bootstrapped/regtested x86_64-linux, OK?
* tree.c (obj_type_ref_class): New function.
* tree.h (obj_type_ref_class): Declare.
* ipa-prop.c (ipa_analyze_virtual_call_uses): Use it.
Index: tree.c
===
--- tree.c (revision 201764)
+++ tree.c (working copy)
@@ -11864,6 +11892,21 @@ types_same_for_odr (tree type1, tree typ
return true;
}
+/* REF is OBJ_TYPE_REF, return the class the ref corresponds to. */
+
+tree
+obj_type_ref_class (tree ref)
+{
+ gcc_checking_assert (TREE_CODE (ref) == OBJ_TYPE_REF);
+ ref = TREE_TYPE (ref);
+ gcc_checking_assert (TREE_CODE (ref) == POINTER_TYPE);
+ ref = TREE_TYPE (ref);
+ gcc_checking_assert (TREE_CODE (ref) == METHOD_TYPE);
+ ref = TREE_VALUE (TYPE_ARG_TYPES (ref));
+ gcc_checking_assert (TREE_CODE (ref) == POINTER_TYPE);
+ return TREE_TYPE (ref);
+}
+
/* Try to find a base info of BINFO that would have its field decl at offset
OFFSET within the BINFO type and which is of EXPECTED_TYPE. If it can be
found, return, otherwise return NULL_TREE. */
Index: tree.h
===
--- tree.h (revision 201764)
+++ tree.h (working copy)
@@ -5974,6 +5974,7 @@ extern location_t tree_nonartificial_loc
extern tree block_ultimate_origin (const_tree);
extern tree get_binfo_at_offset (tree, HOST_WIDE_INT, tree);
+extern tree obj_type_ref_class (tree ref);
extern bool types_same_for_odr (tree type1, tree type2);
extern tree get_ref_base_and_extent (tree, HOST_WIDE_INT *,
HOST_WIDE_INT *, HOST_WIDE_INT *);
Index: ipa-prop.c
===
--- ipa-prop.c (revision 201764)
+++ ipa-prop.c (working copy)
@@ -1903,7 +1903,7 @@ ipa_analyze_virtual_call_uses (struct cg
ii = cs->indirect_info;
ii->offset = anc_offset;
ii->otr_token = tree_low_cst (OBJ_TYPE_REF_TOKEN (target), 1);
- ii->otr_type = TREE_TYPE (TREE_TYPE (OBJ_TYPE_REF_OBJECT (target)));
+ ii->otr_type = obj_type_ref_class (target);
ii->polymorphic = 1;
}
Re: [RFC] Bare bones of virtual call tracking
Some suggestions for the future:
1) add summary info in the odr dump -- i.e. for each node, dump the
number of direct bases, direct subtypes, number of all bases, all
subtypes;
2) add statistics dump -- average size of a hierarchy subgraph
3) Dump the graph using top-order -- starting from roots of each sub-graph;
4) Add VCG dump per-hierarchy.
thanks,
David
On Thu, Aug 15, 2013 at 9:17 AM, Jan Hubicka wrote:
>> On 08/14/2013 02:14 AM, Jan Hubicka wrote:
>> >As a temporary hack I suppose I can rely on assembler name of virtual table
>> >to be unique for each templated class?
>>
>> Actually, that seems like a fine solution for devirtualization; just
>> compare the mangled name of the vtable to establish type identity.
> OK,
> does this seem to make sense? I checked that it gets rid of my
> false ODR violation warnings (I disabled the code for types not
> having virtual tables).
>
> So if it looks OK, I would like to go ahead with this patch.
> (I am testing it now in isolation at x86_64-linux).
>
> Honza
>
> Index: tree.c
> ===
> --- tree.c (revision 201764)
> +++ tree.c (working copy)
> @@ -11833,15 +11833,15 @@ types_same_for_odr (tree type1, tree typ
>if (type1 == type2)
> return true;
>
> - /* If types are not structuraly same, do not bother to contnue.
> - Match in the remainder of code would mean ODR violation. */
> - if (!types_compatible_p (type1, type2))
> -return false;
> -
> + /* Without LTO main variants of types are unique. */
> #ifndef ENABLE_CHECKING
>if (!in_lto_p)
> return false;
> #endif
> + /* If types are not structuraly same, do not bother to contnue.
> + Match in the remainder of code would mean ODR violation. */
> + if (!types_compatible_p (type1, type2))
> + return false;
>
>/* Check for anonymous namespaces. Those have !TREE_PUBLIC
> on the corresponding TYPE_STUB_DECL. */
> @@ -11852,6 +11852,34 @@ types_same_for_odr (tree type1, tree typ
> || !TREE_PUBLIC (TYPE_STUB_DECL (type2
> return false;
>
> + /* When assembler name of virtual table is available, it is
> + easy to compare types for equivalence.
> + FIXME: the code bellow consider all instantiations of the same
> + template to have same name. This is because we have no access
> + to template parameters.
> + To avoid false positives that may lead to wrong devirtualizations,
> + compoare also representative virtual tables.
> + We can still return false positive positives for types without
> + virtual tables, but for the moment we do not care. */
> + if (TYPE_BINFO (type1) && TYPE_BINFO (type2)
> + && BINFO_VTABLE (TYPE_BINFO (type1))
> + && BINFO_VTABLE (TYPE_BINFO (type2)))
> +{
> + tree v1 = BINFO_VTABLE (TYPE_BINFO (type1));
> + tree v2 = BINFO_VTABLE (TYPE_BINFO (type2));
> +
> + if (TREE_CODE (v1) == POINTER_PLUS_EXPR)
> + {
> + if (TREE_CODE (v2) != POINTER_PLUS_EXPR
> + || !operand_equal_p (TREE_OPERAND (v1, 1),
> + TREE_OPERAND (v2, 1), 0))
> + return false;
> + v1 = TREE_OPERAND (TREE_OPERAND (v1, 0), 0);
> + v2 = TREE_OPERAND (TREE_OPERAND (v2, 0), 0);
> + }
> + return DECL_ASSEMBLER_NAME (v1) == DECL_ASSEMBLER_NAME (v2);
> +}
> +
>if (!TYPE_NAME (type1))
> return false;
>if (!decls_same_for_odr (TYPE_NAME (type1), TYPE_NAME (type2)))
Re: [RFC] Bare bones of virtual call tracking
> Some suggestions for the future: > > 1) add summary info in the odr dump -- i.e. for each node, dump the > number of direct bases, direct subtypes, number of all bases, all > subtypes; OK, can add that. > 2) add statistics dump -- average size of a hierarchy subgraph > > 3) Dump the graph using top-order -- starting from roots of each sub-graph; This is already done - dumping recursively dumps subtypes and we dump only types without bases. Problem is that with multiple inheritance types appear twice. > 4) Add VCG dump per-hierarchy. Hmm, may be nice - firefox definitely has huge graph. Currently my type graph is very partial - I have no nodes for types without virtual methods at all. This is because types_same_for_odr is not able to give exact answer (it gives false positives for templates). Hope to solve it incrementaly. I found many bugs at random places, so will push those out first and send updated patch. Honza
[Google] X86_TUNE_USE_VECTOR_CONVERTS adjustment
Turning off X86_TUNE_USE_VECTOR_CONVERTS uses cvtss2sd instead of unpcklps+cvtps2pd, which is better for some recent intel micro arch such as westmere and sandybridge. So turn it off for m_GENERIC and m_CORE_ALL. regression and bootstrap ok. ok for 4.8 branch? Index: config/i386/i386.c === --- config/i386/i386.c (revision 201675) +++ config/i386/i386.c (working copy) @@ -1995,7 +1995,7 @@ static unsigned int initial_ix86_tune_fe /* X86_TUNE_USE_VECTOR_FP_CONVERTS: Prefer vector packed SSE conversion from FP to FP. */ - m_CORE_ALL | m_AMDFAM10 | m_GENERIC, + m_AMDFAM10, /* X86_TUNE_USE_VECTOR_CONVERTS: Prefer vector packed SSE conversion from integer to FP. */
Re: [RFC] Bare bones of virtual call tracking
On Thu, Aug 15, 2013 at 9:46 AM, Jan Hubicka wrote: >> Some suggestions for the future: >> >> 1) add summary info in the odr dump -- i.e. for each node, dump the >> number of direct bases, direct subtypes, number of all bases, all >> subtypes; > > OK, can add that. >> 2) add statistics dump -- average size of a hierarchy subgraph >> >> 3) Dump the graph using top-order -- starting from roots of each sub-graph; > > This is already done - dumping recursively dumps subtypes and we dump only > types without bases. Problem is that with multiple inheritance types appear > twice. > >> 4) Add VCG dump per-hierarchy. > > Hmm, may be nice - firefox definitely has huge graph. That is why the graph should be split up and dumped independently. If the design is such that all types are derived from one common root class we have a problem here :) > > Currently my type graph is very partial - I have no nodes for types without > virtual methods at all. Those are not probably interesting anyways. David > This is because types_same_for_odr is not able to give > exact answer (it gives false positives for templates). Hope to solve it > incrementaly. > > I found many bugs at random places, so will push those out first and send > updated patch. > > Honza
Re: [Google] X86_TUNE_USE_VECTOR_CONVERTS adjustment
yes. thanks, David On Thu, Aug 15, 2013 at 9:50 AM, Wei Mi wrote: > Turning off X86_TUNE_USE_VECTOR_CONVERTS uses cvtss2sd instead of > unpcklps+cvtps2pd, which is better for some recent intel micro arch > such as westmere and sandybridge. So turn it off for m_GENERIC and > m_CORE_ALL. > > regression and bootstrap ok. ok for 4.8 branch? > > Index: config/i386/i386.c > === > --- config/i386/i386.c (revision 201675) > +++ config/i386/i386.c (working copy) > @@ -1995,7 +1995,7 @@ static unsigned int initial_ix86_tune_fe > >/* X86_TUNE_USE_VECTOR_FP_CONVERTS: Prefer vector packed SSE conversion > from FP to FP. */ > - m_CORE_ALL | m_AMDFAM10 | m_GENERIC, > + m_AMDFAM10, > >/* X86_TUNE_USE_VECTOR_CONVERTS: Prefer vector packed SSE conversion > from integer to FP. */
Re: [RFC] Bare bones of virtual call tracking
> On Thu, Aug 15, 2013 at 9:46 AM, Jan Hubicka wrote: > >> Some suggestions for the future: > >> > >> 1) add summary info in the odr dump -- i.e. for each node, dump the > >> number of direct bases, direct subtypes, number of all bases, all > >> subtypes; > > > > OK, can add that. > >> 2) add statistics dump -- average size of a hierarchy subgraph > >> > >> 3) Dump the graph using top-order -- starting from roots of each sub-graph; > > > > This is already done - dumping recursively dumps subtypes and we dump only > > types without bases. Problem is that with multiple inheritance types > > appear twice. > > > >> 4) Add VCG dump per-hierarchy. > > > > Hmm, may be nice - firefox definitely has huge graph. > > That is why the graph should be split up and dumped independently. If > the design is such that all types are derived from one common root > class we have a problem here :) Hehe, for now the main problem is to locate the dump itself in hypergigantic .cgraph dump. I really ought to split that dump into more files... Honza
Re: [RFC] Bare bones of virtual call tracking
> > Currently my type graph is very partial - I have no nodes for types without > > virtual methods at all. > > Those are not probably interesting anyways. Forgot to mention, I think in longer term we can use it to maintain more of C++ type based aliasing rules. Currently we unify canonical types only based on memory layout, since that is safe. If we marked types that follows ODR rule by C++ FE, then we can do odr_type based matching for canonical type unless there is non-ODR type of same layout originating from other language - then we probably need to make it to alias with everything... Because canonical type construction is incremental, I am not really sure how to decide if there is ODR-non-ODR conflict. But I did not really look in detail anyway. Honza
Re: [Ping^4] [Patch, AArch64, ILP32] 3/5 Minor change in function.c:assign_parm_find_data_types()
Ping^4~ I am aware that it is currently holiday season, but it would be really nice if this tiny patch can get some further comments even if it is not an approval. The original RFA email is here: http://gcc.gnu.org/ml/gcc-patches/2013-06/msg01485.html Regards, Yufeng On 07/18/13 11:28, Yufeng Zhang wrote: Ping^3~ Thanks, Yufeng On 07/08/13 11:11, Yufeng Zhang wrote: Ping^2~ Thanks, Yufeng On 07/02/13 23:44, Yufeng Zhang wrote: Ping~ Can I get an OK please if there is no objection? Regards, Yufeng On 06/26/13 23:39, Yufeng Zhang wrote: This patch updates assign_parm_find_data_types to assign passed_mode and nominal_mode with the mode of the built pointer type instead of the hard-coded Pmode in the case of pass-by-reference. This is in line with the assignment to passed_mode and nominal_mode in other cases inside the function. assign_parm_find_data_types generally uses TYPE_MODE to calculate passed_mode and nominal_mode: /* Find mode of arg as it is passed, and mode of arg as it should be during execution of this function. */ passed_mode = TYPE_MODE (passed_type); nominal_mode = TYPE_MODE (nominal_type); this includes the case when the passed argument is a pointer by itself. However there is a discrepancy when it deals with argument passed by invisible reference; it builds the argument's corresponding pointer type, but sets passed_mode and nominal_mode with Pmode directly. This is OK for targets where Pmode == ptr_mode, but on AArch64 with ILP32 they are different with Pmode as DImode and ptr_mode as SImode. When such a reference is passed on stack, the reference is prepared by the caller in the lower 4 bytes of an 8-byte slot but is fetched by the callee as an 8-byte datum, of which the higher 4 bytes may contain junk. It is probably the combination of Pmode != ptr_mode and the particular ABI specification that make the AArch64 ILP32 the first target on which the issue manifests itself. Bootstrapped on x86_64-none-linux-gnu. OK for the trunk? Thanks, Yufeng gcc/ * function.c (assign_parm_find_data_types): Set passed_mode and nominal_mode to the TYPE_MODE of nominal_type for the built pointer type in case of the struct-pass-by-reference.
Re: Fix class type lookup from OBJ_TYPE_REF
Jan Hubicka wrote:
>Hi,
>while working on the devirt code, I noticed that sometimes we get
>as a type of object a non-classes (like void types). This is
>because we look up object's class from the OBJ_TYPE_REF_OBJECT
>and this is a generic pointer that gimple passes like to change
>type of.
>
>I tracked down few cases. For example when compiling passes.c one
>gets copyrename changing
>
>from
>void execute_ipa_summary_passes(ipa_opt_pass_d*) (struct ipa_opt_pass_d
>* ipa_pass)
>{
>...
> struct opt_pass * pass
>...
> pass_12 = ipa_pass_1;
>...
> _20 = OBJ_TYPE_REF(_18;pass_12->3) (pass_12);
>
>
>to
>
> ipa_pass_12 = ipa_pass_1
>...
> _20 = OBJ_TYPE_REF(_18;ipa_pass_12->3) (ipa_pass_12);
>
>Ths makes us to believe that OBJ_TYPE_REF reffers to call of
>method of ipa_opt_pass_d instead of opt_pass. This is wrong and
>I believe it may lead to wrong code, too - the tokens have
>different meanings.
>
>This patch changes code looking up OTR types to actually look
>into OTR type (that is pointer to method) and look the class
>type from type of the method's this pointer.
>
>Bootstrapped/regtested x86_64-linux, OK?
Ok if nobody objects.
Thanks,
Richard.
> * tree.c (obj_type_ref_class): New function.
> * tree.h (obj_type_ref_class): Declare.
> * ipa-prop.c (ipa_analyze_virtual_call_uses): Use it.
>
>
>Index: tree.c
>===
>--- tree.c (revision 201764)
>+++ tree.c (working copy)
>@@ -11864,6 +11892,21 @@ types_same_for_odr (tree type1, tree typ
> return true;
> }
>
>+/* REF is OBJ_TYPE_REF, return the class the ref corresponds to. */
>+
>+tree
>+obj_type_ref_class (tree ref)
>+{
>+ gcc_checking_assert (TREE_CODE (ref) == OBJ_TYPE_REF);
>+ ref = TREE_TYPE (ref);
>+ gcc_checking_assert (TREE_CODE (ref) == POINTER_TYPE);
>+ ref = TREE_TYPE (ref);
>+ gcc_checking_assert (TREE_CODE (ref) == METHOD_TYPE);
>+ ref = TREE_VALUE (TYPE_ARG_TYPES (ref));
>+ gcc_checking_assert (TREE_CODE (ref) == POINTER_TYPE);
>+ return TREE_TYPE (ref);
>+}
>+
>/* Try to find a base info of BINFO that would have its field decl at
>offset
>OFFSET within the BINFO type and which is of EXPECTED_TYPE. If it can
>be
>found, return, otherwise return NULL_TREE. */
>Index: tree.h
>===
>--- tree.h (revision 201764)
>+++ tree.h (working copy)
>@@ -5974,6 +5974,7 @@ extern location_t tree_nonartificial_loc
> extern tree block_ultimate_origin (const_tree);
>
> extern tree get_binfo_at_offset (tree, HOST_WIDE_INT, tree);
>+extern tree obj_type_ref_class (tree ref);
> extern bool types_same_for_odr (tree type1, tree type2);
> extern tree get_ref_base_and_extent (tree, HOST_WIDE_INT *,
>HOST_WIDE_INT *, HOST_WIDE_INT *);
>Index: ipa-prop.c
>===
>--- ipa-prop.c (revision 201764)
>+++ ipa-prop.c (working copy)
>@@ -1903,7 +1903,7 @@ ipa_analyze_virtual_call_uses (struct cg
> ii = cs->indirect_info;
> ii->offset = anc_offset;
> ii->otr_token = tree_low_cst (OBJ_TYPE_REF_TOKEN (target), 1);
>- ii->otr_type = TREE_TYPE (TREE_TYPE (OBJ_TYPE_REF_OBJECT (target)));
>+ ii->otr_type = obj_type_ref_class (target);
> ii->polymorphic = 1;
> }
>
[PATCH] Fix tree-call-cdce.c (PR tree-optimization/58165)
Hi!
On the following testcase we ICE because the builtin fn prototype doesn't
have throw () on it (glibc headers provide it, but some other C libraries
apparently don't) and thus we can have EH edges out of the builtin, and
call-cdce unconditionally split the block, leaving EH edge from an empty
bb and the required EH edge missing from the call.
In that case there is no point in splitting the block though, so fixed
thusly (not looking for last stmt, because that might be problematic with
-fcompare-debug).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.8?
2013-08-15 Jakub Jelinek
PR tree-optimization/58165
* tree-call-cdce.c (shrink_wrap_one_built_in_call): If
bi_call must be the last stmt in a bb, don't split_block, instead
use fallthru edge from it and give up if there is none.
Release conds vector when returning early.
* g++.dg/opt/pr58165.C: New test.
--- gcc/tree-call-cdce.c.jj 2013-08-13 12:20:33.0 +0200
+++ gcc/tree-call-cdce.c2013-08-15 14:54:28.328435719 +0200
@@ -726,15 +726,28 @@ shrink_wrap_one_built_in_call (gimple bi
return false and do not do any transformation for
the call. */
if (nconds == 0)
-return false;
+{
+ conds.release ();
+ return false;
+}
bi_call_bb = gimple_bb (bi_call);
- /* Now find the join target bb -- split
- bi_call_bb if needed. */
- bi_call_bsi = gsi_for_stmt (bi_call);
+ /* Now find the join target bb -- split bi_call_bb if needed. */
+ if (stmt_ends_bb_p (bi_call))
+{
+ /* If the call must be the last in the bb, don't split the block,
+it could e.g. have EH edges. */
+ join_tgt_in_edge_from_call = find_fallthru_edge (bi_call_bb->succs);
+ if (join_tgt_in_edge_from_call == NULL)
+ {
+ conds.release ();
+ return false;
+ }
+}
+ else
+join_tgt_in_edge_from_call = split_block (bi_call_bb, bi_call);
- join_tgt_in_edge_from_call = split_block (bi_call_bb, bi_call);
bi_call_bsi = gsi_for_stmt (bi_call);
join_tgt_bb = join_tgt_in_edge_from_call->dest;
--- gcc/testsuite/g++.dg/opt/pr58165.C.jj 2013-08-15 14:39:20.492586499
+0200
+++ gcc/testsuite/g++.dg/opt/pr58165.C 2013-08-15 14:38:43.0 +0200
@@ -0,0 +1,14 @@
+// PR tree-optimization/58165
+// { dg-do compile }
+// { dg-options "-O2" }
+
+extern "C" float sqrtf (float);
+
+struct A { A (); ~A (); };
+
+void
+foo (double d)
+{
+ A a;
+ sqrtf (d);
+}
Jakub
[PATCH] Fix ICEs with bogus computed goto (PR tree-optimization/58164)
Hi!
gimple_goto_dest is is_gimple_val, so can be ADDR_EXPR (though just for bad
testcases), and in that case we weren't walking it in some cases.
I've tried to reject ADDR_EXPRs in gimple_goto_dest, but that turned out to
be much larger patch and still incomplete.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.8?
2013-08-15 Jakub Jelinek
PR tree-optimization/58164
* gimple.c (walk_stmt_load_store_addr_ops): For visit_addr
walk gimple_goto_dest of GIMPLE_GOTO.
* gcc.c-torture/compile/pr58164.c: New test.
--- gcc/gimple.c.jj 2013-05-13 09:44:53.0 +0200
+++ gcc/gimple.c2013-08-15 15:22:06.745236769 +0200
@@ -4049,6 +4049,13 @@ walk_stmt_load_store_addr_ops (gimple st
ret |= visit_addr (stmt, TREE_OPERAND (op, 0), data);
}
}
+ else if (visit_addr
+ && gimple_code (stmt) == GIMPLE_GOTO)
+{
+ tree op = gimple_goto_dest (stmt);
+ if (TREE_CODE (op) == ADDR_EXPR)
+ ret |= visit_addr (stmt, TREE_OPERAND (op, 0), data);
+}
return ret;
}
--- gcc/testsuite/gcc.c-torture/compile/pr58164.c.jj2013-08-15
15:24:04.117313781 +0200
+++ gcc/testsuite/gcc.c-torture/compile/pr58164.c 2013-08-15
15:23:47.0 +0200
@@ -0,0 +1,8 @@
+/* PR tree-optimization/58164 */
+
+int
+foo (void)
+{
+ int x = 0;
+ goto *&x;
+}
Jakub
Re: [patch, Fortran] PR 58146, enable array slice compile-time bounds checking
Thomas, The two lines gfc_actual_arglist *args1; gfc_actual_arglist *args2; in gfc_dep_compare_expr must be removed. Otherwise the compilation aborts with .../../work/gcc/fortran/dependency.c: In function 'int gfc_dep_compare_expr(gfc_expr*, gfc_expr*)': ../../work/gcc/fortran/dependency.c:295:23: error: unused variable 'args1' [-Werror=unused-variable] gfc_actual_arglist *args1; ^ ../../work/gcc/fortran/dependency.c:296:23: error: unused variable 'args2' [-Werror=unused-variable] gfc_actual_arglist *args2; ^ TIA Dominique
Re: [Patch, Fortran] PR 58099: [4.8/4.9 Regression] [F03] over-zealous procedure-pointer error checking
Hi Mikael, >> here is a small regression-fix patch for a problem with procedure >> pointers and the PURE attribute, for details see the PR. In essence: >> gfc_compare_interfaces is asymmetric in the two interfaces it compares >> (e.g. regarding the PURE attribute), and we must not symmetrize it by >> calling it twice with exchanged arguments. >> >> Regtested on x86_64-unknown-linux-gnu. Ok for trunk? >> > This looks good to me thanks for the opinion ... > but I let Tobias have the final word as he > expressed some concerns in the PR audit trail. yes, I hope I could eradicate his doubts, but of course I'll wait for an ok (hoping he isn't still detached from the interwebs ;) Cheers, Janus
[PATCH] Fix bad interaction between GTY((user)) and incomplete declarations
There is a bug in gengtype's handling of types marked with GTY((user))
which can lead to gtype-desc.c containing broken test-and-mark functions
that fail to call the user-provided traversal functions, leading to
segfaults.
If the type is first encountered as an incomplete type declaration:
struct foo;
find_structure records the name of the type, setting its
kind to TYPE_STRUCT (not TYPE_USER_STRUCT).
Later, when the complete type is seen (with its GTY((user)) marking):
struct GTY((user)) foo
{
};
create_user_defined_type calls:
type_p ty = find_structure (type_name, TYPE_USER_STRUCT);
but the name is already found, erroneously of kind TYPE_STRUCT.
Since the parser treats it as a user struct, no attempt is made to
parse the fields.
Later, when write_func_for_structure writes out the marking function
for the type, this conditional:
if (orig_s->kind != TYPE_USER_STRUCT)
fails to treat the type as a "user" type, instead walking an empty
list of fields.
Hence it write out bogus marking functions into gtype-desc.c (with
analogous effects on PCH):
void
gt_ggc_mx_foo (void *x_p)
{
struct foo * const x = (struct foo *)x_p;
if (ggc_test_and_set_mark (x))
{
}
}
This code compiles and runs - until GC (or PCH) happens, when it
erroneously collects any objects that are only referenced by
affected types.
Note the erroneously empty body - the above should have read:
void
gt_ggc_mx_foo (void *x_p)
{
struct foo * const x = (struct foo *)x_p;
if (ggc_test_and_set_mark (x))
{
gt_ggc_mx (x);
}
}
i.e. with a call to the user-provided gt_ggc_mx function, otherwise
none of the references to affected GTY((user)) types are followed,
and only a subset of the reference graph is walked.
This patch fixes the "kind" of the type handled within
create_user_defined_type to ensure it is treated as a "user" type,
fixing the generated gtype-desc.c code.
Successfully bootstrapped and regrtested on x86_64-unknown-linux-gnu.
* gengtype.c (create_user_defined_type): Ensure that the kind
is set to TYPE_USER_STRUCT, fixing a bug seen when an incomplete
declaration is seen before the GTY((user)) marking.
---
gcc/gengtype.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/gcc/gengtype.c b/gcc/gengtype.c
index 2085496..9c12752 100644
--- a/gcc/gengtype.c
+++ b/gcc/gengtype.c
@@ -559,6 +559,12 @@ type_p
create_user_defined_type (const char *type_name, struct fileloc *pos)
{
type_p ty = find_structure (type_name, TYPE_USER_STRUCT);
+
+ /* We might have already seen an incomplete decl of the given type,
+ in which case we won't have yet seen a GTY((user)), and the type will
+ only have kind "TYPE_STRUCT". Mark it as a user struct. */
+ ty->kind = TYPE_USER_STRUCT;
+
ty->u.s.line = *pos;
ty->u.s.bitmap = get_lang_bitmap (pos->file);
do_typedef (type_name, ty, pos);
--
1.7.11.7
Re: [PATCH] Redesign pthread in LIB_SPEC for systems without libpthread
On 15/08/2013, at 10:49 PM, Alexander Ivchenko wrote:
> Could anybody please take a look? This is important for building gcc for
> android.
>
> ping^4
[Sorry for being cranky]
There is a reason why people are ignoring your patch -- the submission is not
very well structured.
First, the subject -- it is not accurate. "Redesign" implies a big change, you
would be better off with a heading like "Fix LIB_SPEC for Android".
Second, you do not fully describe the problem that you are trying to fix.
Third, you do not say how your patch fixes the problem.
Lastly, you do not mention which targets you have encountered the problem on
and tested the fix on. Since you are from Intel, I can guess that you use x86.
> 2013-04-02 Pavel Chupin
>
> Redesign pthread in LIB_SPEC for systems without libpthread
> * gcc/config/gnu-user.h: Remove pthread from
> GNU_USER_TARGET_LIB_SPEC
> but keep in default LIB_SPEC
> * gcc/config/linux-android.h: Add pthread to ANDROID_LIB_SPEC
>
> Is it OK for trunk?
> --- a/gcc/config/gnu-user.h
> +++ b/gcc/config/gnu-user.h
> @@ -74,11 +74,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
> If not, see
> #define CPLUSPLUS_CPP_SPEC "-D_GNU_SOURCE %(cpp)"
>
> #define GNU_USER_TARGET_LIB_SPEC \
> - "%{pthread:-lpthread} \
> - %{shared:-lc} \
> + "%{shared:-lc} \
Indentation seems off here, please double-check.
> %{!shared:%{mieee-fp:-lieee} %{profile:-lc_p}%{!profile:-lc}}"
> #undef LIB_SPEC
> -#define LIB_SPEC GNU_USER_TARGET_LIB_SPEC
> +#define LIB_SPEC "%{pthread:-lpthread} " GNU_USER_TARGET_LIB_SPEC
You need to similarly add "%{pthread:-lpthread} " to LIB_SPEC in
gcc/config/mips/gnu-user.h.
>
> #if defined(HAVE_LD_EH_FRAME_HDR)
> #define LINK_EH_SPEC "%{!static:--eh-frame-hdr} "
> diff --git a/gcc/config/linux-android.h b/gcc/config/linux-android.h
> index 831a19c..aaf1d34 100644
> --- a/gcc/config/linux-android.h
> +++ b/gcc/config/linux-android.h
> @@ -49,7 +49,8 @@
>"%{!frtti:%{!fno-rtti: -fno-rtti}}"
>
> #define ANDROID_LIB_SPEC \
> - "%{!static: -ldl}"
> + "%{!static: -ldl}" \
> + "%{pthread: -lc}"
Add a comment to explain why -pthread option triggers -lc for Android.
Please post the updated patch along with notes on which targets you have tested
it. Test coverage should include at least x86_64-linux-gnu and one of android
targets.
Thank you,
--
Maxim Kuvyrkov
www.kugelworks.com
Re: [PATCH, powerpc] PR 58160 -- fix power8 gpr load fusion to be safer
On Wed, Aug 14, 2013 at 6:55 PM, Michael Meissner wrote: > In running the spec 2006 testsuite with several options, we discovered that > -mtune=power8 (which turns on the -mpower8-fusion option) has a segmentation > fault in two benchmarks: > > 403.gcc when built with -O2 -mcpu=power7 -mtune=power8 -m32 > 435.gromacs when built with -O3 -funroll-loops -mcpu=power7 -mtune=power8 -m32 > > In the gcc case, I tracked it down to the fusion op in the function > strength_reduce in the file loop.c. It was wanting to use the result of the > addis instruction after the loop. > > It wanted to optimize: > lis 10,loop_dump_stream@ha > lwz 6,loop_dump_stream@l(6) > stw 10,24(1) > > to: > lis 6,loop_dump_stream@ha > lwz 6,loop_dump_stream@l(6) > stw 10,24(1) > > The problem is fusion was implemented using define_peephole, and the register > live notes are not correct when peepholes are done. > > I have written the following patch, and bootstrapped the compiler using > BOOT_CFLAGS='-g -O2 -mcpu=power7 -mtune=power8' to stress the code. There > were > no regressions in make check. Are these patches ok to install in the tree? > > I have the code for the function that causes the problem narrowed down to the > function that generates the bad code. However, I'm worried that this test > will > be very fragile, and it will need to be updated as the compiler changes. > > The compiler should be fixed so that it won't be tempted to spill the ELF_HIGH > for the addis on the stack, so that in the future, it could safely fusion the > addis and load operation. > > [gcc] > 2013-08-14 Michael Meissner > > PR target/58160 > * config/rs6000/predicates.md (fusion_gpr_mem_load): Allow the > memory rtx to contain ZERO_EXTEND and SIGN_EXTEND. > > * config/rs6000/rs6000-protos.h (fusion_gpr_load_p): Pass operands > array instead of each individual operand as a separate argument. > (emit_fusion_gpr_load): Likewise. > (expand_fusion_gpr_load): Add new function declaration. > > * config/rs6000/rs6000.c (fusion_gpr_load_p): Change the calling > signature to have the operands passed as an array, instead of as > separate arguments. Allow ZERO_EXTEND to be in the memory > address, and also SIGN_EXTEND if -mpower8-fusion-sign. Do not > depend on the register live/dead flags when peepholes are run. > (expand_fusion_gpr_load): New function to be called from the > peephole2 pass, to change the register that addis sets to be the > target register. > (emit_fusion_gpr_load): Change the calling signature to have the > operands passed as an array, instead of as separate arguments. > Allow ZERO_EXTEND to be in the memory address, and also > SIGN_EXTEND if -mpower8-fusion-sign. > > * config/rs6000/rs6000.md (UNSPEC_FUSION_GPR): Delete unused > unspec enumeration. > (power8 fusion peephole/peephole2): Rework the fusion peepholes to > adjust the register addis loads up in the peephole2 pass. Do not > depend on the register live/dead state when the peephole pass is > done. This is okay. Thanks, David
Re: Two testcases failing for check-debug
On Thu, Aug 15, 2013 at 5:53 PM, Jonathan Wakely wrote: > Why not use __s.data() in all cases, instead of &*__s.begin()? That's > safe even for an empty vector. Thanks, I didn't notice that vector's data() is mutable(since string's is not). Bootstrap Passed, and testsuites with -D_GLIBCXX_DEBUG, with or without -m32 passed. Could you please commit this patch for me? I lost my SSH private key :( Thanks! -- Tim Shen a.patch Description: Binary data
[PATCH 0/2] Port symtab/cgraph/varpool nodes to use C++ inheritance
Honza: following up from our IRC chat, I've ported the symtab, cgraph and varpool nodes from the current hand-coded inheritance-in-C scheme to being a C+ class hierarchy. I know you're in the middle of making lots of other changes to this code, so I've written a script to automate the parts of the conversion where appropriate - see below. In summary, struct GTY(()) symtab_node_base becomes: class GTY((user)) symtab_node_base and the subclasses: struct GTY(()) cgraph_node and: struct GTY(()) varpool_node become (respectively): struct GTY((user)) cgraph_node : public symtab_node_base and: class GTY((user)) varpool_node : public symtab_node_base The symtab_node_def union goes away, as do the "symbol" fields at the top of the two subclasses. I kept the existing names for things, and the "struct" for cgraph_node since it is often referred to as "struct cgraph_node". The patch is in three parts; all three are needed. * a fix for a bug in gengtype: http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00882.html This is needed to avoid generating a malfunctioning gtype-desc.c *some* of the time - in capriciously changing ways as the sources change... :( I sent it separately since I'm running into this issue with other changes I'm trying out; it "randomly" appears when trying to use GTY((user)). * Patch 1 of the 2 in this series is the hand-written part of the conversion. This makes the changes above, plus various workarounds for dealing with yet more issues in how gengtype handles GTY((user)). * Patch 2 of the 2 in this series is the autogenerated part. Currently to access the base symtab fields of a cgraph or varpool node, the code has e.g. node->symbol.decl whereas with C++ inheritance, the "symbol" field is no more, and we directly use the base-class field: node->decl The script that makes the patch is for Python (2.7), and is in: https://github.com/davidmalcolm/gcc-refactoring-scripts as refactor_symtab.py and there's a test suite in test_refactor_symtab.py. It's basically a glorified sed, but it also autogenerates the ChangeLog entries. It expects the gcc checkout to be in a sister directory named "src". Successfully bootstrapped and regtested on x86_64-unknown-linux-gnu (using all three patches together, showing same results as an unpatched source tree). How does this look? Are there other automated changes you'd like me to make? How should I go about getting this into trunk, given that you have many other changes to this code on the way? Thanks; hope this is helpful Dave David Malcolm (2): Convert symtab, cgraph and varpool nodes into a real class hierarchy Autogenerated fixes of "->symbol." to "->" gcc/ada/gcc-interface/trans.c | 2 +- gcc/ada/gcc-interface/utils.c | 2 +- gcc/asan.c| 10 +- gcc/c-family/c-gimplify.c | 2 +- gcc/c-family/c-pragma.c | 2 +- gcc/cfgexpand.c | 2 +- gcc/cgraph.c | 480 ++ gcc/cgraph.h | 150 +++-- gcc/cgraphbuild.c | 10 +- gcc/cgraphclones.c| 84 gcc/cgraphunit.c | 260 +++ gcc/config/i386/i386.c| 18 +- gcc/coverage.c| 4 +- gcc/cp/call.c | 2 +- gcc/cp/decl2.c| 24 +-- gcc/cp/tree.c | 4 +- gcc/dbxout.c | 2 +- gcc/dwarf2out.c | 4 +- gcc/gimple-fold.c | 8 +- gcc/gimplify.c| 4 +- gcc/ipa-cp.c | 66 +++--- gcc/ipa-inline-analysis.c | 48 ++--- gcc/ipa-inline-transform.c| 46 ++-- gcc/ipa-inline.c | 166 +++ gcc/ipa-prop.c| 56 ++--- gcc/ipa-pure-const.c | 54 ++--- gcc/ipa-ref-inline.h | 4 +- gcc/ipa-ref.c | 14 +- gcc/ipa-ref.h | 6 +- gcc/ipa-reference.c | 60 +++--- gcc/ipa-split.c | 26 +-- gcc/ipa-utils.c | 46 ++-- gcc/ipa.c | 410 ++-- gcc/is-a.h| 8 +- gcc/java/decl.c | 2 +- gcc/lto-cgraph.c | 224 ++-- gcc/lto-streamer-in.c | 4 +- gcc/lto-streamer-out.c| 38 ++-- gcc/lto-symtab.c | 198 - gcc/lto/lto-partition.c | 126 +-- gcc/lto/lto.c | 26 +-- gcc/passes.c | 28 +-- gcc/symtab.c | 372 gcc/toplev.c | 6 +- gcc/trans-mem.c | 94 - gcc/tree-eh.c | 4 +- gcc/tree-emutls.c | 38 ++-- gcc/tree-inline.c | 36 ++-- gcc/tree-nested.c | 10 +- gcc/tree-pretty-
[PATCH 1/2] Convert symtab, cgraph and varpool nodes into a real class hierarchy
This patch is the handwritten part of the conversion of these types
to C++; it requires the followup patch, which is autogenerated.
It converts:
struct GTY(()) symtab_node_base
to:
class GTY((user)) symtab_node_base
and converts:
struct GTY(()) cgraph_node
to:
struct GTY((user)) cgraph_node : public symtab_node_base
and:
struct GTY(()) varpool_node
to:
class GTY((user)) varpool_node : public symtab_node_base
dropping the symtab_node_def union.
Since gengtype is unable to cope with inheritance, we have to mark the
types with GTY((user)), and handcode the gty field-visiting functions.
Given the simple hierarchy, we don't need virtual functions for this.
Unfortunately doing so runs into various bugs in gengtype's handling
of GTY((user)), so the patch also includes workarounds for these bugs.
gengtype walks the graph of the *types* in the code, and produces
functions in gtype-desc.[ch] for all types that are reachable from a
GTY root.
However, it ignores the contents of GTY((user)) types when walking
this graph.
Hence if you have a subgraph of types that are only reachable
via fields in GTY((user)) types, gengtype won't generate helper code
for those types.
Ideally there would be some way to mark a GTY((user)) type to say
which types it references, to avoid having to mark these types as
GTY((user)).
For now, work around this issue by providing explicit roots of the
missing types, of dummy variables (see the bottom of cgraph.c)
gcc/
* cgraph.c (gt_ggc_mx): New.
(gt_pch_nx (symtab_node_base *)): New.
(gt_pch_nx (symtab_node_base *, gt_pointer_operator, void *)): New.
(dummy_call_site_hash): New.
(dummy_ipa_ref_list): New.
(dummy_cgraph_clone_info): New.
(dummy_ipa_replace_map_pointer): New.
(dummy_varpool_node_ptr): New.
* cgraph.h (symtab_node_base): Convert to a class;
add GTY((user)).
(gt_ggc_mx): New.
(gt_pch_nx (symtab_node_base *p)): New.
(gt_pch_nx (symtab_node_base *p, gt_pointer_operator op,
void *cookie)): New.
(cgraph_node): Inherit from symtab_node; convert to GTY((user)).
(varpool_node): Likewise.
(symtab_node_def): Remove.
(is_a_helper ::test (symtab_node_def *)): Convert to...
(is_a_helper ::test (symtab_node_base *)): ...this.
(is_a_helper ::test (symtab_node_def *)): Convert to...
(is_a_helper ::test (symtab_node_base *)): ...this.
* ipa-ref.h (symtab_node_def): Drop.
(symtab_node): Change underlying type from symtab_node_def to
symtab_node_base.
(const_symtab_node): Likwise.
* is-a.h: Update examples in comment.
* symtab.c (symtab_hash): Change symtab_node_def to symtab_node_base.
(assembler_name_hash): Likewise.
---
gcc/cgraph.c | 218 ++
gcc/cgraph.h | 48 ++---
gcc/ipa-ref.h | 6 +-
gcc/is-a.h| 6 +-
gcc/symtab.c | 4 +-
5 files changed, 247 insertions(+), 35 deletions(-)
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 50d13ab..5cb6a31 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3035,4 +3035,222 @@ cgraph_get_body (struct cgraph_node *node)
return true;
}
+/* GTY((user)) hooks for symtab_node_base (and its subclasses).
+ We could use virtual functions for this, but given the presence of the
+ "type" field and the trivial size of the class hierarchy, switches are
+ perhaps simpler and faster. */
+
+void gt_ggc_mx (symtab_node_base *x)
+{
+ /* Hand-written equivalent of the chain_next/chain_prev machinery, to
+ avoid deep call-stacks.
+
+ Locate the neighbors of x (within the linked-list) that haven't been
+ marked yet, so that x through xlimit give a range suitable for marking.
+ Note that x (on entry) itself has already been marked by the
+ gtype-desc.c code, so we first try its successor.
+ */
+ symtab_node_base * xlimit = x ? x->next : NULL;
+ while (ggc_test_and_set_mark (xlimit))
+ xlimit = xlimit->next;
+ if (x != xlimit)
+for (;;)
+ {
+symtab_node_base * const xprev = x->previous;
+if (xprev == NULL) break;
+x = xprev;
+(void) ggc_test_and_set_mark (xprev);
+ }
+ while (x != xlimit)
+{
+ /* Code common to all symtab nodes. */
+ gt_ggc_m_9tree_node (x->decl);
+ gt_ggc_mx_symtab_node_base (x->next);
+ gt_ggc_mx_symtab_node_base (x->previous);
+ gt_ggc_mx_symtab_node_base (x->next_sharing_asm_name);
+ gt_ggc_mx_symtab_node_base (x->previous_sharing_asm_name);
+ gt_ggc_mx_symtab_node_base (x->same_comdat_group);
+ gt_ggc_m_20vec_ipa_ref_t_va_gc_ (x->ref_list.references);
+ gt_ggc_m_9tree_node (x->alias_target);
+ gt_ggc_m_18lto_file_decl_data (x->lto_file_data);
+
+ /* Extra code, per subclass. */
+ switch (x->type)
+{
+case SYMTAB_FUNCTION:
+ {
+cgraph_node *cgn = static_ca
[PATCH 2/2] Autogenerated fixes of "->symbol." to "->"
This patch is 384KB in size, so to avoid exceeding mailing list limits I've uploaded it to: http://dmalcolm.fedorapeople.org/gcc/large-patches/6a680b99445e0a734f9270150534bb5b2bd77cdf-0002-Autogenerated-fixes-of-symbol.-to.patch The following is just the proposed ChangeLog entry and the diffstat. gcc/ Patch autogenerated by refactor_symtab.py from https://github.com/davidmalcolm/gcc-refactoring-scripts revision e1f5fb89719e99ffa17cdd15e77e90790572acf3 * asan.c (asan_finish_file): Update for conversion of symtab types to a true class hierarchy. * cfgexpand.c (estimated_stack_frame_size): Likewise. * cgraph.c (record_function_versions): Likewise. (cgraph_create_empty_node): Likewise. (cgraph_create_node): Likewise. (cgraph_create_function_alias): Likewise. (cgraph_same_body_alias): Likewise. (cgraph_add_thunk): Likewise. (cgraph_node_for_asm): Likewise. (cgraph_set_call_stmt): Likewise. (cgraph_create_edge_1): Likewise. (cgraph_turn_edge_to_speculative): Likewise. (cgraph_speculative_call_info): Likewise. (cgraph_resolve_speculation): Likewise. (cgraph_make_edge_direct): Likewise. (cgraph_redirect_edge_call_stmt_to_callee): Likewise. (cgraph_update_edges_for_call_stmt_node): Likewise. (cgraph_release_function_body): Likewise. (cgraph_remove_node): Likewise. (cgraph_mark_address_taken_node): Likewise. (cgraph_rtl_info): Likewise. (dump_cgraph_node): Likewise. (cgraph_function_body_availability): Likewise. (cgraph_node_cannot_be_local_p_1): Likewise. (cgraph_node_can_be_local_p): Likewise. (cgraph_for_node_thunks_and_aliases): Likewise. (cgraph_for_node_and_aliases): Likewise. (cgraph_make_node_local_1): Likewise. (cgraph_set_nothrow_flag_1): Likewise. (cgraph_set_const_flag_1): Likewise. (cgraph_set_pure_flag_1): Likewise. (cgraph_propagate_frequency_1): Likewise. (cgraph_propagate_frequency): Likewise. (cgraph_node_cannot_return): Likewise. (cgraph_can_remove_if_no_direct_calls_and_refs_p): Likewise. (cgraph_can_remove_if_no_direct_calls_p): Likewise. (cgraph_will_be_removed_from_program_if_no_direct_calls): Likewise. (verify_edge_count_and_frequency): Likewise. (verify_edge_corresponds_to_fndecl): Likewise. (verify_cgraph_node): Likewise. (cgraph_get_create_real_symbol_node): Likewise. (cgraph_get_body): Likewise. * cgraph.h (None): Likewise. (cgraph_turn_edge_to_speculative): Likewise. (varpool): Likewise. (cgraph_node_asm_name): Likewise. (varpool_first_variable): Likewise. (varpool_next_variable): Likewise. (varpool_first_static_initializer): Likewise. (varpool_next_static_initializer): Likewise. (varpool_first_defined_variable): Likewise. (varpool_next_defined_variable): Likewise. (cgraph_first_defined_function): Likewise. (cgraph_next_defined_function): Likewise. (cgraph_first_function): Likewise. (cgraph_next_function): Likewise. (cgraph_function_with_gimple_body_p): Likewise. (cgraph_first_function_with_gimple_body): Likewise. (cgraph_next_function_with_gimple_body): Likewise. (cgraph_only_called_directly_or_aliased_p): Likewise. (varpool_can_remove_if_no_refs): Likewise. (varpool_all_refs_explicit_p): Likewise. (symtab_alias_target): Likewise. (cgraph_edge_recursive_p): Likewise. (cgraph_mark_force_output_node): Likewise. (symtab_real_symbol_p): Likewise. * cgraphbuild.c (record_eh_tables): Likewise. (rebuild_cgraph_edges): Likewise. (cgraph_rebuild_references): Likewise. (remove_cgraph_callee_edges): Likewise. * cgraphclones.c (cgraph_clone_node): Likewise. (cgraph_create_virtual_clone): Likewise. (cgraph_find_replacement_node): Likewise. (update_call_expr): Likewise. (cgraph_copy_node_for_versioning): Likewise. (cgraph_materialize_clone): Likewise. (cgraph_materialize_all_clones): Likewise. * cgraphunit.c (decide_is_symbol_needed): Likewise. (enqueue_node): Likewise. (cgraph_process_new_functions): Likewise. (cgraph_reset_node): Likewise. (referred_to_p): Likewise. (cgraph_finalize_function): Likewise. (cgraph_add_new_function): Likewise. (analyze_function): Likewise. (cgraph_process_same_body_aliases): Likewise. (process_function_and_variable_attributes): Likewise. (varpool_finalize_decl): Likewise. (analyze_functions): Likewise. (handle_alias_pairs): Likewise. (mark_functions_to_output): Likewise. (expand_thunk): Likewise. (assem
Re: [PATCH] Fix bad interaction between GTY((user)) and incomplete declarations
> * gengtype.c (create_user_defined_type): Ensure that the kind > is set to TYPE_USER_STRUCT, fixing a bug seen when an incomplete > declaration is seen before the GTY((user)) marking. This is OK, thank you. -- Laurynas
Re: [PATCH] Fix tree-call-cdce.c (PR tree-optimization/58165)
Jakub Jelinek wrote:
>Hi!
>
>On the following testcase we ICE because the builtin fn prototype
>doesn't
>have throw () on it (glibc headers provide it, but some other C
>libraries
>apparently don't) and thus we can have EH edges out of the builtin, and
>call-cdce unconditionally split the block, leaving EH edge from an
>empty
>bb and the required EH edge missing from the call.
>
>In that case there is no point in splitting the block though, so fixed
>thusly (not looking for last stmt, because that might be problematic
>with
>-fcompare-debug).
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
>trunk/4.8?
>
Ok,.
Thanks,
Richard.
>2013-08-15 Jakub Jelinek
>
> PR tree-optimization/58165
> * tree-call-cdce.c (shrink_wrap_one_built_in_call): If
> bi_call must be the last stmt in a bb, don't split_block, instead
> use fallthru edge from it and give up if there is none.
> Release conds vector when returning early.
>
> * g++.dg/opt/pr58165.C: New test.
>
>--- gcc/tree-call-cdce.c.jj2013-08-13 12:20:33.0 +0200
>+++ gcc/tree-call-cdce.c 2013-08-15 14:54:28.328435719 +0200
>@@ -726,15 +726,28 @@ shrink_wrap_one_built_in_call (gimple bi
> return false and do not do any transformation for
> the call. */
> if (nconds == 0)
>-return false;
>+{
>+ conds.release ();
>+ return false;
>+}
>
> bi_call_bb = gimple_bb (bi_call);
>
>- /* Now find the join target bb -- split
>- bi_call_bb if needed. */
>- bi_call_bsi = gsi_for_stmt (bi_call);
>+ /* Now find the join target bb -- split bi_call_bb if needed. */
>+ if (stmt_ends_bb_p (bi_call))
>+{
>+ /* If the call must be the last in the bb, don't split the
>block,
>+ it could e.g. have EH edges. */
>+ join_tgt_in_edge_from_call = find_fallthru_edge
>(bi_call_bb->succs);
>+ if (join_tgt_in_edge_from_call == NULL)
>+ {
>+conds.release ();
>+return false;
>+ }
>+}
>+ else
>+join_tgt_in_edge_from_call = split_block (bi_call_bb, bi_call);
>
>- join_tgt_in_edge_from_call = split_block (bi_call_bb, bi_call);
> bi_call_bsi = gsi_for_stmt (bi_call);
>
> join_tgt_bb = join_tgt_in_edge_from_call->dest;
>--- gcc/testsuite/g++.dg/opt/pr58165.C.jj 2013-08-15 14:39:20.492586499
>+0200
>+++ gcc/testsuite/g++.dg/opt/pr58165.C 2013-08-15 14:38:43.0
>+0200
>@@ -0,0 +1,14 @@
>+// PR tree-optimization/58165
>+// { dg-do compile }
>+// { dg-options "-O2" }
>+
>+extern "C" float sqrtf (float);
>+
>+struct A { A (); ~A (); };
>+
>+void
>+foo (double d)
>+{
>+ A a;
>+ sqrtf (d);
>+}
>
> Jakub
Re: [PATCH] Fix ICEs with bogus computed goto (PR tree-optimization/58164)
Jakub Jelinek wrote:
>Hi!
>
>gimple_goto_dest is is_gimple_val, so can be ADDR_EXPR (though just for
>bad
>testcases), and in that case we weren't walking it in some cases.
>
>I've tried to reject ADDR_EXPRs in gimple_goto_dest, but that turned
>out to
>be much larger patch and still incomplete.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
>trunk/4.8?
Ok. Can you try properly verifying things in verify-gimple?
Thanks,
Richard.
>2013-08-15 Jakub Jelinek
>
> PR tree-optimization/58164
> * gimple.c (walk_stmt_load_store_addr_ops): For visit_addr
> walk gimple_goto_dest of GIMPLE_GOTO.
>
> * gcc.c-torture/compile/pr58164.c: New test.
>
>--- gcc/gimple.c.jj2013-05-13 09:44:53.0 +0200
>+++ gcc/gimple.c 2013-08-15 15:22:06.745236769 +0200
>@@ -4049,6 +4049,13 @@ walk_stmt_load_store_addr_ops (gimple st
> ret |= visit_addr (stmt, TREE_OPERAND (op, 0), data);
> }
> }
>+ else if (visit_addr
>+ && gimple_code (stmt) == GIMPLE_GOTO)
>+{
>+ tree op = gimple_goto_dest (stmt);
>+ if (TREE_CODE (op) == ADDR_EXPR)
>+ ret |= visit_addr (stmt, TREE_OPERAND (op, 0), data);
>+}
>
> return ret;
> }
>--- gcc/testsuite/gcc.c-torture/compile/pr58164.c.jj 2013-08-15
>15:24:04.117313781 +0200
>+++ gcc/testsuite/gcc.c-torture/compile/pr58164.c 2013-08-15
>15:23:47.0 +0200
>@@ -0,0 +1,8 @@
>+/* PR tree-optimization/58164 */
>+
>+int
>+foo (void)
>+{
>+ int x = 0;
>+ goto *&x;
>+}
>
> Jakub
