Add predicate to test if type contains polymorphic type
Hi,
this patch adds predicate contains_polymorphic_type_p. This can be used
in the ICF pass - if one of the types is contains_polymorphic_type_p,
then TYPE_MAIN_VARIANTs of both types should be the same, at least for
now. Of course it would be possible to compare types for equality and then
also polymorphic types for equality (i.e. that all virtual functions in
vtables are equivalent). I am not sure how much that matters
and we can definitely implement that incrementally.
I also changed ipa-prop to use the predicate - this lifts the unnecesary
restriction that type propagation needs to be done only on polymorphic types;
it makes perfect sense to propagate a knowledge that type contains fileds
that are polymorphic.
To do this in full generality the propagation engine has to be updated to
not work on binfos though.
Bootstrapped/regtested x86_64-linux, will commit it shortly.
Honza
* ipa-utils.h (method_class_type, vtable_pointer_value_to_binfo,
vtable_pointer_value_to_vtable): Constify.
(contains_polymorphic_type_p): Declare.
* ipa-devirt.c (method_class_type, vtable_pointer_value_to_binfo,
vtable_pointer_value_to_vtable): Constify.
(contains_polymorphic_type_p): New predicate.
* ipa-prop.c (ipa_set_jf_known_type): Allow types containing
polymorphic types.
(ipa_set_ancestor_jf): Likewise.
(detect_type_change): Return false in easy cases.
(compute_complex_assign_jump_func): Require type to contain
polymorphic type.
(compute_known_type_jump_func): Likewise.
Index: ipa-utils.h
===
--- ipa-utils.h (revision 212217)
+++ ipa-utils.h (working copy)
@@ -83,15 +83,16 @@ void dump_possible_polymorphic_call_targ
const ipa_polymorphic_call_context
&);
bool possible_polymorphic_call_target_p (tree, HOST_WIDE_INT,
const ipa_polymorphic_call_context &,
-struct cgraph_node *n);
-tree method_class_type (tree);
+struct cgraph_node *);
+tree method_class_type (const_tree);
tree get_polymorphic_call_info (tree, tree, tree *,
HOST_WIDE_INT *,
ipa_polymorphic_call_context *);
bool get_polymorphic_call_info_from_invariant (ipa_polymorphic_call_context *,
tree, tree, HOST_WIDE_INT);
-tree vtable_pointer_value_to_binfo (tree t);
-bool vtable_pointer_value_to_vtable (tree, tree *, unsigned HOST_WIDE_INT *);
+tree vtable_pointer_value_to_binfo (const_tree);
+bool vtable_pointer_value_to_vtable (const_tree, tree *, unsigned
HOST_WIDE_INT *);
+bool contains_polymorphic_type_p (const_tree);
/* Return vector containing possible targets of polymorphic call E.
If FINALP is non-NULL, store true if the list is complette.
Index: ipa-devirt.c
===
--- ipa-devirt.c(revision 212218)
+++ ipa-devirt.c(working copy)
@@ -742,7 +742,7 @@ dump_type_inheritance_graph (FILE *f)
Lookup this pointer and get its type.*/
tree
-method_class_type (tree t)
+method_class_type (const_tree t)
{
tree first_parm_type = TREE_VALUE (TYPE_ARG_TYPES (t));
gcc_assert (TREE_CODE (t) == METHOD_TYPE);
@@ -1187,6 +1187,31 @@ devirt_node_removal_hook (struct cgraph_
free_polymorphic_call_targets_hash ();
}
+/* Return true when TYPE contains an polymorphic type and thus is interesting
+ for devirtualization machinery. */
+
+bool
+contains_polymorphic_type_p (const_tree type)
+{
+ type = TYPE_MAIN_VARIANT (type);
+
+ if (RECORD_OR_UNION_TYPE_P (type))
+{
+ if (TYPE_BINFO (type)
+ && polymorphic_type_binfo_p (TYPE_BINFO (type)))
+ return true;
+ for (tree fld = TYPE_FIELDS (type); fld; fld = DECL_CHAIN (fld))
+ if (TREE_CODE (fld) == FIELD_DECL
+ && !DECL_ARTIFICIAL (fld)
+ && contains_polymorphic_type_p (TREE_TYPE (fld)))
+ return true;
+ return false;
+}
+ if (TREE_CODE (type) == ARRAY_TYPE)
+return contains_polymorphic_type_p (TREE_TYPE (type));
+ return false;
+}
+
/* CONTEXT->OUTER_TYPE is a type of memory object where object of EXPECTED_TYPE
is contained at CONTEXT->OFFSET. Walk the memory representation of
CONTEXT->OUTER_TYPE and find the outermost class type that match
@@ -1349,7 +1374,8 @@ subbinfo_with_vtable_at_offset (tree bin
Return false if T does not look like virtual table reference. */
bool
-vtable_pointer_value_to_vtable (tree t, tree *v, unsigned HOST_WIDE_INT
*offset)
+vtable_pointer_value_to_vtable (const_tree t, tree *v,
+ unsigned HOST_WIDE_INT *offset)
{
/* We expect &MEM[(void *)&virtual_table + 16B].
We obtain object's BINFO from the context of the virtua
[PATCH] Fix ICE on old style init of derived components
Hi!
As the testcase shows, we ICE on old style initialization of derived type
components. ifort also rejects these, I think it doesn't make sense to
support such initializations of derived type components.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.9?
2014-07-02 Jakub Jelinek
Fritz Reese
* decl.c (variable_decl): Reject old style initialization
for derived type components.
* gfortran.dg/oldstyle_5.f: New test.
--- gcc/fortran/decl.c.jj 2014-06-30 09:28:50.0 +0200
+++ gcc/fortran/decl.c 2014-07-01 16:47:19.466050044 +0200
@@ -1997,6 +1997,13 @@ variable_decl (int elem)
if (!gfc_notify_std (GFC_STD_GNU, "Old-style "
"initialization at %C"))
return MATCH_ERROR;
+ else if (gfc_current_state () == COMP_DERIVED)
+ {
+ gfc_error ("Invalid old style initialization for derived type "
+"component at %C");
+ m = MATCH_ERROR;
+ goto cleanup;
+ }
return match_old_style_init (name);
}
--- gcc/testsuite/gfortran.dg/oldstyle_5.f.jj 2014-07-01 16:50:40.449001427
+0200
+++ gcc/testsuite/gfortran.dg/oldstyle_5.f 2014-07-01 16:48:33.0
+0200
@@ -0,0 +1,8 @@
+C { dg-do compile }
+ TYPE T
+ INTEGER A(2)/1,2/ ! { dg-error "Invalid old style initialization for
derived type component" }
+ END TYPE
+ TYPE S
+ INTEGER B/1/ ! { dg-error "Invalid old style initialization for derived
type component" }
+ END TYPE
+ END
Jakub
[PATCH] Don't allow combination of read/write and earlyclobber constraint modifier
On 01-07-14 21:47, Jeff Law wrote:
On 07/01/14 13:27, Tom de Vries wrote:
So my question is: is the combination of '&' and '+' supported ? If so,
what is the exact semantics ? If not, should we warn or give an error ?
>
I don't think we can define any reasonable semantics for &+. My recommendation
would be for this to be considered a hard error.
[ move discussion from gcc ml to gcc-patches ml ]
Attached patch detects the combination of + and & constrains during genrecog,
and generates an error like this:
...
/home/vries/gcc_versions/devel/src/gcc/config/aarch64/aarch64-simd.md:1020:
operand 0 has in-out reload, incompatible with earlyclobber
/home/vries/gcc_versions/devel/src/gcc/config/aarch64/aarch64-simd.md:1020:
operand 0 has in-out reload, incompatible with earlyclobber
/home/vries/gcc_versions/devel/src/gcc/config/aarch64/aarch64-simd.md:1020:
operand 0 has in-out reload, incompatible with earlyclobber
make[2]: *** [s-recog] Error 1
...
The error triggers three times, once for each mode iterator element.
OK if x86_64 bootstrap succeeds ?
Thanks,
- Tom
2014-07-02 Tom de Vries
* genrecog.c (validate_pattern): Don't allow earlyclobber constraint
modifier with read/write constraint modifier.
diff --git a/gcc/genrecog.c b/gcc/genrecog.c
index 457b59c..ad709ee 100644
--- a/gcc/genrecog.c
+++ b/gcc/genrecog.c
@@ -481,6 +481,13 @@ validate_pattern (rtx pattern, rtx insn, rtx set, int set_code)
rtx_name[GET_CODE (insn)]);
}
+ if (constraints0 == '+'
+ && strchr (XSTR (pattern, 2), '&') != NULL)
+ error_with_line (pattern_lineno,
+ "operand %d has in-out reload, incompatible with"
+ " earlyclobber",
+ XINT (pattern, 0));
+
/* A MATCH_OPERAND that is a SET should have an output reload. */
else if (set && constraints0)
{
--
1.9.1
Re: [AArch64] Implement some vca*_f[32,64] intrinsics
Hi,
It seems some of the scan-assembler directives fail:
http://cbuild.validation.linaro.org/build/cross-validation/gcc/trunk/212196/aarch64-none-elf/diff-gcc-rh50-aarch64-none-elf-default-default-default.txt
Christophe.
On 1 July 2014 14:13, Marcus Shawcroft wrote:
> On 23 June 2014 15:30, Kyrill Tkachov wrote:
>> Hi all,
>>
>> This patch implements some absolute compare intrinsics in arm_neon.h.
>>
>> Execution tests are added.
>> Tested aarch64-none-elf, aarch64_be-none-elf, bootstrapped on aarch64 linux
>
>
> +/* { dg-do run } */
> +/* { dg-options "-save-temps -O3" } */
> +
> +#include "arm_neon.h"
> +#include
> +
>
> Drop the dependence on stdio.h please.
>
> +fprintf (stderr, "Expected: %ld, got %ld\n", expected, actual);
>
> No need to print the expected value, abort( i alone is fine. Dropping
> the printf will make the #include above go away.
>
> Ok with those changes.
>
> /Marcus
Re: [AArch64] Implement some vca*_f[32,64] intrinsics
On 02/07/14 08:59, Christophe Lyon wrote:
Hi,
It seems some of the scan-assembler directives fail:
http://cbuild.validation.linaro.org/build/cross-validation/gcc/trunk/212196/aarch64-none-elf/diff-gcc-rh50-aarch64-none-elf-default-default-default.txt
Hi Christophe
Yes, turns out removing the printf at the end lets gcc fold away parts
of the test.
It's just a testism, will send a patch out soon.
Thanks,
Kyrill
Christophe.
On 1 July 2014 14:13, Marcus Shawcroft wrote:
On 23 June 2014 15:30, Kyrill Tkachov wrote:
Hi all,
This patch implements some absolute compare intrinsics in arm_neon.h.
Execution tests are added.
Tested aarch64-none-elf, aarch64_be-none-elf, bootstrapped on aarch64 linux
+/* { dg-do run } */
+/* { dg-options "-save-temps -O3" } */
+
+#include "arm_neon.h"
+#include
+
Drop the dependence on stdio.h please.
+fprintf (stderr, "Expected: %ld, got %ld\n", expected, actual);
No need to print the expected value, abort( i alone is fine. Dropping
the printf will make the #include above go away.
Ok with those changes.
/Marcus
Re: combination of read/write and earlyclobber constraint modifier
On 02-07-14 08:23, Marc Glisse wrote: In the first example you gave, looking at the pattern (no match_dup, setting the full register), it seems that it may have wanted "=&" instead of "+&". [ move discussion from gcc ml to gcc-patches ml ] Marcus, The +& constraint on operand 0 of vec_unpack_trunc_ seems wrong, since the template does not use the operand as input. This patch fixes that. OK for trunk if aarch64 build & regtest succeeds ? Thanks, - Tom 2014-07-02 Tom de Vries * config/aarch64/aarch64-simd.md (define_insn "vec_unpack_trunc_"): Fix constraint. diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md index 1c32f0c..0377de4 100644 --- a/gcc/config/aarch64/aarch64-simd.md +++ b/gcc/config/aarch64/aarch64-simd.md @@ -1018,7 +1018,7 @@ ;; For quads. (define_insn "vec_pack_trunc_" - [(set (match_operand: 0 "register_operand" "+&w") + [(set (match_operand: 0 "register_operand" "=&w") (vec_concat: (truncate: (match_operand:VQN 1 "register_operand" "w")) (truncate: (match_operand:VQN 2 "register_operand" "w"] -- 1.9.1
[PATCH] Fix various undefined behaviors in gcc found by bootstrap-ubsan
Hi!
Most of this probably doesn't need explanation, just the
expand_sdiv_pow2 change - shift_cost only tracks shift counts to
MAX_BITS_PER_WORD (but, for consistency between e.g. i?86 and x86_64 -m32
we'd better use BITS_PER_WORD instead as in most other expmed.c places),
when called with larger values it is out of bounds access. I believe
for multiword accesses, even when we don't have costs for it, and masking
will be typically cheaper (e.g. for double word you need two word ANDs,
while for double word shift unless there is a specialized instruction
for double word shifts one needs either a library routine or 3 shifts and
IOR).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2014-07-02 Jakub Jelinek
* gcov-io.c (gcov_read_words): Don't call memmove if excess is 0.
* data-streamer-in.c (streamer_read_hwi): Shift UHWI 1 instead of
HWI 1 and negate the unsigned value.
* expmed.c (expand_sdiv_pow2): For modes wider than word always
use AND instead of shift.
* wide-int-print.cc (print_decs): Negate UHWI instead of HWI.
c-family/
* c-ada-spec.c (dump_ada_nodes): Don't call qsort if
comments->count <= 1, as comments->entries might be NULL.
--- gcc/gcov-io.c.jj2014-05-30 10:51:15.0 +0200
+++ gcc/gcov-io.c 2014-06-30 12:09:54.276063826 +0200
@@ -489,14 +489,15 @@ gcov_read_words (unsigned words)
if (excess < words)
{
gcov_var.start += gcov_var.offset;
-#if IN_LIBGCOV
if (excess)
{
+#if IN_LIBGCOV
memcpy (gcov_var.buffer, gcov_var.buffer + gcov_var.offset, 4);
- }
#else
- memmove (gcov_var.buffer, gcov_var.buffer + gcov_var.offset, excess * 4);
+ memmove (gcov_var.buffer, gcov_var.buffer + gcov_var.offset,
+ excess * 4);
#endif
+ }
gcov_var.offset = 0;
gcov_var.length = excess;
#if IN_LIBGCOV
--- gcc/data-streamer-in.c.jj 2014-01-03 11:40:29.0 +0100
+++ gcc/data-streamer-in.c 2014-06-30 11:59:52.000221301 +0200
@@ -174,7 +174,7 @@ streamer_read_hwi (struct lto_input_bloc
if ((byte & 0x80) == 0)
{
if ((shift < HOST_BITS_PER_WIDE_INT) && (byte & 0x40))
- result |= - ((HOST_WIDE_INT)1 << shift);
+ result |= - (HOST_WIDE_INT_1U << shift);
return result;
}
--- gcc/expmed.c.jj 2014-06-27 09:03:01.0 +0200
+++ gcc/expmed.c2014-06-30 11:46:00.947593030 +0200
@@ -3795,8 +3795,9 @@ expand_sdiv_pow2 (enum machine_mode mode
temp = gen_reg_rtx (mode);
temp = emit_store_flag (temp, LT, op0, const0_rtx, mode, 0, -1);
- if (shift_cost (optimize_insn_for_speed_p (), mode, ushift)
- > COSTS_N_INSNS (1))
+ if (GET_MODE_BITSIZE (mode) >= BITS_PER_WORD
+ || shift_cost (optimize_insn_for_speed_p (), mode, ushift)
+> COSTS_N_INSNS (1))
temp = expand_binop (mode, and_optab, temp, gen_int_mode (d - 1, mode),
NULL_RTX, 0, OPTAB_LIB_WIDEN);
else
--- gcc/c-family/c-ada-spec.c.jj2014-05-11 22:20:59.0 +0200
+++ gcc/c-family/c-ada-spec.c 2014-06-30 12:05:06.931560475 +0200
@@ -636,8 +636,9 @@ dump_ada_nodes (pretty_printer *pp, cons
comments = cpp_get_comments (parse_in);
/* Sort the comments table by sloc. */
- qsort (comments->entries, comments->count, sizeof (cpp_comment),
-compare_comment);
+ if (comments->count > 1)
+qsort (comments->entries, comments->count, sizeof (cpp_comment),
+ compare_comment);
/* Interleave comments and declarations in line number order. */
i = j = 0;
--- gcc/wide-int-print.cc.jj2014-05-11 22:21:04.0 +0200
+++ gcc/wide-int-print.cc 2014-06-30 12:02:11.349485371 +0200
@@ -62,7 +62,8 @@ print_decs (const wide_int_ref &wi, char
|| (wi.get_len () == 1))
{
if (wi::neg_p (wi))
- sprintf (buf, "-" HOST_WIDE_INT_PRINT_UNSIGNED, -wi.to_shwi ());
+ sprintf (buf, "-" HOST_WIDE_INT_PRINT_UNSIGNED,
+-(unsigned HOST_WIDE_INT) wi.to_shwi ());
else
sprintf (buf, HOST_WIDE_INT_PRINT_DEC, wi.to_shwi ());
}
Jakub
[PATCH] RTEMS: Add multilibs for ARM
This change is necessary to support Cortex-R based chips in RTEMS. This patch should be applied to GCC 4.8, 4.9 and mainline. I do not have write access, so in case this gets approved, please commit it for me. gcc/ChangeLog 2014-07-02 Sebastian Huber * config/arm/t-rtems-eabi: Add mthumb/march=armv7-r/mfpu=vfpv3-d16/mfloat-abi=hard, mbig-endian/mthumb/march=armv7-r, and mbig-endian/mthumb/march=armv7-r/mfpu=vfpv3-d16/mfloat-abi=hard multilibs. --- gcc/config/arm/t-rtems-eabi | 84 +- 1 files changed, 82 insertions(+), 2 deletions(-) diff --git a/gcc/config/arm/t-rtems-eabi b/gcc/config/arm/t-rtems-eabi index d81fbf7..c896e61 100644 --- a/gcc/config/arm/t-rtems-eabi +++ b/gcc/config/arm/t-rtems-eabi @@ -1,47 +1,127 @@ # Custom RTEMS EABI multilibs -MULTILIB_OPTIONS = mthumb march=armv6-m/march=armv7-a/march=armv7-r/march=armv7-m mfpu=neon mfloat-abi=hard -MULTILIB_DIRNAMES = thumb armv6-m armv7-a armv7-r armv7-m neon hard +MULTILIB_OPTIONS = mbig-endian mthumb march=armv6-m/march=armv7-a/march=armv7-r/march=armv7-m mfpu=neon/mfpu=vfpv3-d16 mfloat-abi=hard +MULTILIB_DIRNAMES = eb thumb armv6-m armv7-a armv7-r armv7-m neon vfpv3-d16 hard # Enumeration of multilibs MULTILIB_EXCEPTIONS = +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv6-m/mfpu=neon/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv6-m/mfpu=neon +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv6-m/mfpu=vfpv3-d16/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv6-m/mfpu=vfpv3-d16 +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv6-m/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv6-m +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-a/mfpu=neon/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-a/mfpu=neon +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-a/mfpu=vfpv3-d16/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-a/mfpu=vfpv3-d16 +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-a/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-a +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-r/mfpu=neon/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-r/mfpu=neon +# MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-r/mfpu=vfpv3-d16/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-r/mfpu=vfpv3-d16 +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-r/mfloat-abi=hard +# MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-r +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-m/mfpu=neon/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-m/mfpu=neon +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-m/mfpu=vfpv3-d16/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-m/mfpu=vfpv3-d16 +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-m/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/march=armv7-m +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/mfpu=neon/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/mfpu=neon +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/mfpu=vfpv3-d16/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/mfpu=vfpv3-d16 +MULTILIB_EXCEPTIONS += mbig-endian/mthumb/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/mthumb +MULTILIB_EXCEPTIONS += mbig-endian/march=armv6-m/mfpu=neon/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/march=armv6-m/mfpu=neon +MULTILIB_EXCEPTIONS += mbig-endian/march=armv6-m/mfpu=vfpv3-d16/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/march=armv6-m/mfpu=vfpv3-d16 +MULTILIB_EXCEPTIONS += mbig-endian/march=armv6-m/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/march=armv6-m +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-a/mfpu=neon/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-a/mfpu=neon +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-a/mfpu=vfpv3-d16/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-a/mfpu=vfpv3-d16 +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-a/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-a +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-r/mfpu=neon/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-r/mfpu=neon +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-r/mfpu=vfpv3-d16/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-r/mfpu=vfpv3-d16 +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-r/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-r +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-m/mfpu=neon/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-m/mfpu=neon +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-m/mfpu=vfpv3-d16/mfloat-abi=hard +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-m/mfpu=vfpv3-d16 +MULTILIB_EXCEPTIONS += mbig-endian/march=armv7-m/mfloat-abi=hard +MULTILIB_EXCEPTIONS
Re: [patch 1/4] change specific int128 -> generic intN
> The whole point of using _PRECISION is to have the size be exactly the > same as the mode (bitsize is bigger than the mode for partial-int > modes). TYPE_SIZE_UNIT should be its storage size, right? If the > type is not a multiple of BITS_PER_UNIT, the actual size and > stored-in-memory size are going to be different. But you cannot use the PRECISION of a mode to set a size, since the SIZE of the mode can be used by the machine to access the object in that mode, so you'll end up with types whose TYPE_SIZE is smaller than the access size. You need to use TYPE_PRECISION here. > The problem in the old code is that BITSIZE is not the size in bits of > the type, it's the size in bytes times BITS_PER_UNIT. This meant that > all partial-int modes would be converted into their non-partial modes > eventually. Most of my patch is about avoiding that. Yes, BITSIZE is the size rounded up to BITS_PER_UNIT, but why is that a problem exactly? Do you have modes whose size is not multiple of the unit? > I'm not reducing it, I'm removing a case where it's rounded up. > Rounding up a 20-bit PSImode gives you SImode eventually. Yes, you are reducing it, the precision of bitsize must be at least that of sizetype + BITS_PER_UNIT_LOG + 1. -- Eric Botcazou
Re: Fix finding reg-sets of call insn in collect_fn_hard_reg_usage
> So in order to known whether it's safe and optimal to use regs_ever_live > instead, the question is whether the passes after pass_free_cfg (are allowed > to) add or remove sets or clobbers of call_really_used_regs. I don't know > the full answer there. pass_machine_reorg is run after pass_free_cfg and it can do pretty much what it wants so I'd think that the answer is yes. -- Eric Botcazou
[GOMP4, RFC] OpenMP4 offload support for Intel PHI targets.
Hello, I would like to announce creation of a dedicated branch gomp4-offload to speed up review of FE-independent offload-related features. This branch includes: - set of necessary patches from gomp4 branch - set of patches which we were developed internally and were unable to share - yet to be checked-into-trunk `liboffloadmic' (review started here: https://gcc.gnu.org/ml/gcc-patches/2014-06/msg02133.html) - 10 new tests for offload written using OpenMP4, which replicates `OpenMP4 examples' This branch should be capable to perform offload to Intel targets (Xeon PHI) to help people to perform experiments emulator of MIC's software stack is included. This emulator lies under `liboffloadmic' and reproduces MIC's HW and SW stack behavior allowing to perform non-fallback execution of target regions using the host machine. We're going to start efforts to review and integrate these changes to GCC main trunk. We appreciate your feedback! Many changes are vexed, especially autogen-related since we're dummies here. Currently bootstrap is broken (because of few warnings) and will be fixed soon. Here is a manual describing how to build compiler and run OpenMP offload apps: 1. Building accel compiler: /gcc_src/configure --disable-bootstrap --enable-accelerator=intelmic --enable-as-accelerator-for=x86_64-unknown-linux-gnu --enable-liboffloadmic=target --prefix=/install_mic make make install 2. Building host compiler: export LIBOFFLOAD_TARGET_PATH="/install_mic/lib64" /gcc_src/configure --disable-bootstrap --enable-accelerator=intelmic --enable-liboffloadmic=host --prefix=/install_host make make install 3. Building an application: export LD_LIBRARY_PATH="/install_host/lib64" export MKOFFLOAD="/install_mic/libexec/gcc/x86_64-unknown-linux-gnu/4.10.0/accel/x86_64-unknown-linux-gnu/mkoffload" /install_host/bin/gcc -fopenmp -flto test.c 4. Running an application: export LIBGOMP_PLUGIN_PATH="/gcc_build/x86_64-unknown-linux-gnu/libgomp/plugins/intelmic" export MIC_LD_LIBRARY_PATH="/install_mic/lib64/" ./a.out 5. make check. Run tests (w/ emulator) export LD_LIBRARY_PATH="/install_host/lib64" export MKOFFLOAD="/install_mic/libexec/gcc/x86_64-unknown-linux-gnu/4.10.0/accel/x86_64-unknown-linux-gnu/mkoffload" export LIBGOMP_PLUGIN_PATH="/gcc_build/x86_64-unknown-linux-gnu/libgomp/plugins/intelmic" export MIC_LD_LIBRARY_PATH="/install_mic/lib64/" make check-target-libgomp Do not delete build dir of the host compiler, since 'make install' for libgomp plugin is not working yet. Also there might be an issue with building libgomp plugin when using relative path to the 'configure'. It is GIT-only branch, name is: kyukhin/gomp4-offload URL: https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/heads/kyukhin/gomp4-offload -- Thanks, K
[GOMP4, RFC] Support of offloading for Intel MIC targets.
Hello, I would like to announce creation of a dedicated branch gomp4-offload to speed up review of FE-independent offload-related features. This branch includes: - set of necessary patches from gomp4 branch - set of patches which we were developed internally and were unable to share - yet to be checked-into-trunk `liboffloadmic' (review started here: https://gcc.gnu.org/ml/gcc-patches/2014-06/msg02133.html) - 10 new tests for offload written using OpenMP4, which replicates `OpenMP4 examples' This branch should be capable to perform offload to Intel targets (Xeon PHI) to help people to perform experiments emulator of MIC's software stack is included. This emulator lies under `liboffloadmic' and reproduces MIC's HW and SW stack behavior allowing to perform non-fallback execution of target regions using the host machine. We're going to start efforts to review and integrate these changes to GCC main trunk. We appreciate your feedback! Many changes are vexed, especially autogen-related since we're dummies here. Currently bootstrap is broken (because of few warnings) and will be fixed soon. Here is a manual describing how to build compiler and run OpenMP offload apps: 1. Building accel compiler: /gcc_src/configure --disable-bootstrap --enable-accelerator=intelmic --enable-as-accelerator-for=x86_64-unknown-linux-gnu --enable-liboffloadmic=target --prefix=/install_mic make make install 2. Building host compiler: export LIBOFFLOAD_TARGET_PATH="/install_mic/lib64" /gcc_src/configure --disable-bootstrap --enable-accelerator=intelmic --enable-liboffloadmic=host --prefix=/install_host make make install 3. Building an application: export LD_LIBRARY_PATH="/install_host/lib64" export MKOFFLOAD="/install_mic/libexec/gcc/x86_64-unknown-linux-gnu/4.10.0/accel/x86_64-unknown-linux-gnu/mkoffload" /install_host/bin/gcc -fopenmp -flto test.c 4. Running an application: export LIBGOMP_PLUGIN_PATH="/gcc_build/x86_64-unknown-linux-gnu/libgomp/plugins/intelmic" export MIC_LD_LIBRARY_PATH="/install_mic/lib64/" ./a.out 5. make check. Run tests (w/ emulator) export LD_LIBRARY_PATH="/install_host/lib64" export MKOFFLOAD="/install_mic/libexec/gcc/x86_64-unknown-linux-gnu/4.10.0/accel/x86_64-unknown-linux-gnu/mkoffload" export LIBGOMP_PLUGIN_PATH="/gcc_build/x86_64-unknown-linux-gnu/libgomp/plugins/intelmic" export MIC_LD_LIBRARY_PATH="/install_mic/lib64/" make check-target-libgomp Do not delete build dir of the host compiler, since 'make install' for libgomp plugin is not working yet. Also there might be an issue with building libgomp plugin when using relative path to the 'configure'. It is GIT-only branch, name is: kyukhin/gomp4-offload URL: https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/heads/kyukhin/gomp4-offload -- Thanks, K
Re: [PATCH] Fix ICE on old style init of derived components
Jakub Jelinek wrote: > As the testcase shows, we ICE on old style initialization of derived type > components. ifort also rejects these, I think it doesn't make sense to > support such initializations of derived type components. Side remark: Cray ftn also rejects it, while PGI's compiler accepts it. > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.9? Looks good to me. Thanks! Tobias > 2014-07-02 Jakub Jelinek > Fritz Reese > > * decl.c (variable_decl): Reject old style initialization > for derived type components. > > * gfortran.dg/oldstyle_5.f: New test.
Re: [GOMP4, RFC] Support of offloading for Intel MIC targets.
On 02 Jul 12:45, Kirill Yukhin wrote: > Hello, Pls disregard this mail and use previous one. Sorry. -- Thanks, K
[PATCH, libgfortran]: Fix support_fpu_rounding_mode and add -mieee flags for alpha
Hello!
Attached patch fixes some fallout from IEEE support and enables -mieee
for alpha. Also, the patch removes -O0 from dg-additiona-options in
IEEE testsuite, as this always override default optimization flag.
libgfortran/ChangeLog:
2014-07-02 Uros Bizjak
* configure.host (ieee_flags): Add -mieee for alpha*.
* config/fpu-glibc.h (support_fpu_rounding_mode): Correctly handle
GFC_FPE_UPWARD, GFC_FPE_DOWNWARD and GFC_FPE_TOWARDZERO.
* config/fpu-aix.h (support_fpu_rounding_mode): Ditto.
testsuite/ChangeLog:
2014-07-02 Uros Bizjak
* gfortran.dg/ieee/ieee_1.F90 (dg-additional-options): Remove -O0.
* gfortran.dg/ieee/ieee_rounding_1.f90 (dg-additional-options): Add.
Patch was tested on alphaev68-linux-gnu (uses fpu-glibc.h) and
x86_64-linux-gnu. Patch was committed to mainline SVN.
BTW: On alpha, it is possible to enable underflow handling:
#ifdef __USE_GNU
/* On later hardware, and later kernels for earlier hardware, we can forcibly
underflow denormal inputs and outputs. This can speed up certain programs
significantly, usually without affecting accuracy. */
enum
{
FE_MAP_DMZ =1UL << 12, /* Map denorm inputs to zero */
#define FE_MAP_DMZ FE_MAP_DMZ
FE_MAP_UMZ =1UL << 13, /* Map underflowed outputs to zero */
#define FE_MAP_UMZ FE_MAP_UMZ
};
#endif
FX, if you care for this option, I can help test the patch and
corresponding testcases.
Uros.
Index: libgfortran/configure.host
===
--- libgfortran/configure.host (revision 212221)
+++ libgfortran/configure.host (working copy)
@@ -48,6 +48,8 @@ esac
# Some targets require additional compiler options for NaN/Inf.
ieee_flags=
case "${host_cpu}" in
+ alpha*)
+ieee_flags="-mieee" ;;
sh*)
ieee_flags="-mieee" ;;
esac
Index: libgfortran/config/fpu-aix.h
===
--- libgfortran/config/fpu-aix.h(revision 212221)
+++ libgfortran/config/fpu-aix.h(working copy)
@@ -372,18 +372,21 @@ support_fpu_rounding_mode (int mode)
return 0;
#endif
+ case GFC_FPE_UPWARD:
#ifdef FE_UPWARD
return 1;
#else
return 0;
#endif
+ case GFC_FPE_DOWNWARD:
#ifdef FE_DOWNWARD
return 1;
#else
return 0;
#endif
+ case GFC_FPE_TOWARDZERO:
#ifdef FE_TOWARDZERO
return 1;
#else
Index: libgfortran/config/fpu-glibc.h
===
--- libgfortran/config/fpu-glibc.h (revision 212221)
+++ libgfortran/config/fpu-glibc.h (working copy)
@@ -387,18 +387,21 @@ support_fpu_rounding_mode (int mode)
return 0;
#endif
+ case GFC_FPE_UPWARD:
#ifdef FE_UPWARD
return 1;
#else
return 0;
#endif
+ case GFC_FPE_DOWNWARD:
#ifdef FE_DOWNWARD
return 1;
#else
return 0;
#endif
+ case GFC_FPE_TOWARDZERO:
#ifdef FE_TOWARDZERO
return 1;
#else
Index: gcc/testsuite/gfortran.dg/ieee/ieee_1.F90
===
--- gcc/testsuite/gfortran.dg/ieee/ieee_1.F90 (revision 212221)
+++ gcc/testsuite/gfortran.dg/ieee/ieee_1.F90 (working copy)
@@ -1,5 +1,5 @@
! { dg-do run }
-! { dg-additional-options "-ffree-line-length-none -O0" }
+! { dg-additional-options "-ffree-line-length-none" }
!
! Use dg-additional-options rather than dg-options to avoid overwriting the
! default IEEE options which are passed by ieee.exp and necessary.
Index: gcc/testsuite/gfortran.dg/ieee/ieee_rounding_1.f90
===
--- gcc/testsuite/gfortran.dg/ieee/ieee_rounding_1.f90 (revision 212221)
+++ gcc/testsuite/gfortran.dg/ieee/ieee_rounding_1.f90 (working copy)
@@ -1,4 +1,5 @@
! { dg-do run }
+! { dg-additional-options "-mfp-rounding-mode=d" { target alpha*-*-* } }
use, intrinsic :: ieee_features, only : ieee_rounding
use, intrinsic :: ieee_arithmetic
[PATCH][AArch64][committed] Delete unused variable i
Hi all,
I've committed this patch as obvious with r212225. It fixes aarch64
bootstrap.
Cheers,
Kyrill
2014-07-02 Kyrylo Tkachov
* config/aarch64/aarch64.c (aarch64_expand_vec_perm): Delete unused
variable i.diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index af9ff3a..3aba204 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -8734,7 +8734,7 @@ void
aarch64_expand_vec_perm (rtx target, rtx op0, rtx op1, rtx sel)
{
enum machine_mode vmode = GET_MODE (target);
- unsigned int i, nelt = GET_MODE_NUNITS (vmode);
+ unsigned int nelt = GET_MODE_NUNITS (vmode);
bool one_vector_p = rtx_equal_p (op0, op1);
rtx mask;
Re: [PATCH, libgfortran]: Fix support_fpu_rounding_mode and add -mieee flags for alpha
Dear Uros,
Thanks very much for the patch. I have a few questions:
> the patch removes -O0 from dg-additiona-options in IEEE testsuite, as this
> always override default optimization flag.
That was my purpose: this test can fail with optimization (on x86_64, IIRC),
hence the -O0 should override the default optimization flags in the testsuite.
>* gfortran.dg/ieee/ieee_rounding_1.f90 (dg-additional-options): Add.
If -mfp-rounding-mode=d is required on alpha for IEEE conformance, it should be
added to add-ieee-options in lib/fortran-torture.exp, shouldn’t it? Rather than
enabled on a case-by-case basis. Also, we might want to document these
target-specific options here:
https://gcc.gnu.org/onlinedocs/gfortran/IEEE-modules.html
> BTW: On alpha, it is possible to enable underflow handling:
>
> #ifdef __USE_GNU
> /* On later hardware, and later kernels for earlier hardware, we can forcibly
> underflow denormal inputs and outputs. This can speed up certain programs
> significantly, usually without affecting accuracy. */
> enum
> {
>FE_MAP_DMZ =1UL << 12, /* Map denorm inputs to zero */
> #define FE_MAP_DMZ FE_MAP_DMZ
>
>FE_MAP_UMZ =1UL << 13, /* Map underflowed outputs to zero */
> #define FE_MAP_UMZ FE_MAP_UMZ
> };
> #endif
>
> FX, if you care for this option, I can help test the patch and
> corresponding testcases.
Yes, that’s interesting. What libc function do you call with those
FE_MAP_{D,U}MZ values?
I would also like to enable FTZ for i386/x86_64 at some point, but my issue
there is that it’s not “universal”, ie it’s only for SSE math if I understand
correctly. One point of view is that it’s better than nothing (especially now
that SSE math is probably the most used mode), but another point of view is
that it wouldn’t be standard conforming.
FX
[PATCH] Power/GCC: Subword atomic operation endianness check bug fix
Hi, This change: r199935 | amodra | 2013-06-11 07:17:50 +0100 (Tue, 11 Jun 2013) | 4 lines * config/rs6000/rs6000.c (rs6000_adjust_atomic_subword): Calculate correct shift value in little-endian mode. fixed subword atomic operations on little-endian Power targets, but while the effect of the change happens to be correct, the way it is achieved is not. This code operates on SImode values, however refers to WORDS_BIG_ENDIAN to check the endianness. This is not right, quoting our internal manual: "[...] The memory order of bytes is defined by two target macros, `WORDS_BIG_ENDIAN' and `BYTES_BIG_ENDIAN': * `WORDS_BIG_ENDIAN', if set to 1, says that byte number zero is part of the most significant word; otherwise, it is part of the least significant word. * `BYTES_BIG_ENDIAN', if set to 1, says that byte number zero is the most significant byte within a word; otherwise, it is the least significant byte within a word." so WORDS_BIG_ENDIAN should only ever be referred when operating on multi-word values, not single-word ones such as SImode ones are. As it happens both macros have the same value for the Power target, so the result of the check works out the same, but nevertheless it's not correct and can be misleading to the casual reader. The fix below has been regression tested with the powerpc-eabi target and the following multilibs: -mcpu=603e -mcpu=603e -msoft-float -mcpu=8540 -mfloat-gprs=single -mspe=yes -mabi=spe -mcpu=8540 -mfloat-gprs=single -mspe=yes -mabi=spe -msoft-float -mcpu=8548 -mfloat-gprs=double -mspe=yes -mabi=spe -mcpu=8548 -mfloat-gprs=double -mspe=yes -mabi=spe -mlittle -mcpu=8548 -mfloat-gprs=double -mspe=yes -mabi=spe -msoft-float -mcpu=7400 -maltivec -mabi=altivec as well as the powerpc-linux-gnu target and the following multilibs: -mcpu=603e -mcpu=603e -msoft-float -mcpu=8540 -mfloat-gprs=single -mspe=yes -mabi=spe -mcpu=8548 -mfloat-gprs=double -mspe=yes -mabi=spe -mcpu=7400 -maltivec -mabi=altivec -mcpu=e5500 -m64 OK to apply? 2014-07-02 Maciej W. Rozycki gcc/ * config/rs6000/rs6000.c (rs6000_adjust_atomic_subword): Use BYTES_BIG_ENDIAN rather than WORDS_BIG_ENDIAN to check for byte endianness. Maciej gcc-ppc-atomic-le.diff Index: gcc-fsf-trunk-quilt/gcc/config/rs6000/rs6000.c === --- gcc-fsf-trunk-quilt.orig/gcc/config/rs6000/rs6000.c 2014-06-10 21:46:36.628975329 +0100 +++ gcc-fsf-trunk-quilt/gcc/config/rs6000/rs6000.c 2014-06-11 16:35:08.917560846 +0100 @@ -19891,7 +19891,7 @@ rs6000_adjust_atomic_subword (rtx orig_m shift = gen_reg_rtx (SImode); addr = gen_lowpart (SImode, addr); emit_insn (gen_rlwinm (shift, addr, GEN_INT (3), GEN_INT (shift_mask))); - if (WORDS_BIG_ENDIAN) + if (BYTES_BIG_ENDIAN) shift = expand_simple_binop (SImode, XOR, shift, GEN_INT (shift_mask), shift, 1, OPTAB_LIB_WIDEN); *pshift = shift;
[PATCH] Don't run guality.exp tests with LTO_TORTURE_OPTIONS.
Hi,
While writing new guality.exp tests I noticed they often just fail
when ran with -flto, even though they PASS in all other cases. When
I asked on irc about this I was told that LTO was known to not play
well with DWARF debuginfo anyway. If that is the case, it seems better
to disable -flto for guality.exp for now since it just adds noise
(and unfortunately guality.exp already has lots of FAILs). The attached
patch does this. It changes the make check-c RUNTESTFLAGS=guality.exp
results (against gdb git master) from:
# of expected passes3121
# of unexpected failures180
# of unexpected successes 33
# of expected failures 15
# of unsupported tests 78
to:
# of expected passes2389
# of unexpected failures110
# of unexpected successes 26
# of expected failures 10
# of unsupported tests 54
That looks like an improvement to me, even though the number of FAILs
is still a bit high. Does this look a good thing to apply? Or are people
actively working on reducing the number of LTO guality.exp failures and
can we expect them to turn into PASSes soon anyway?
Thanks,
Mark
gcc/testsuite/ChangeLog
* gcc.dg/guality/guality.exp: Remove LTO_TORTURE_OPTIONS from
set-torture-options. Only use DG_TORTURE_OPTIONS.
---
gcc/testsuite/ChangeLog |5 +
gcc/testsuite/gcc.dg/guality/guality.exp |6 ++
2 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index b364f40..421e006 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2014-07-02 Mark Wielaard
+
+ * gcc.dg/guality/guality.exp: Remove LTO_TORTURE_OPTIONS from
+ set-torture-options. Only use DG_TORTURE_OPTIONS.
+
2014-07-02 Uros Bizjak
* gfortran.dg/ieee/ieee_1.F90 (dg-additional-options): Remove -O0.
diff --git a/gcc/testsuite/gcc.dg/guality/guality.exp
b/gcc/testsuite/gcc.dg/guality/guality.exp
index 5e714dd..7991661 100644
--- a/gcc/testsuite/gcc.dg/guality/guality.exp
+++ b/gcc/testsuite/gcc.dg/guality/guality.exp
@@ -47,6 +47,12 @@ if {[check_guality "
return 0;
}
"]} {
+ # Override default torture-options which include LTO_TORTURE_OPTIONS
+ # Currently LTO and the guality tests that depend on debuginfo don't mix.
+ global DG_TORTURE_OPTIONS
+ torture-init
+ set-torture-options $DG_TORTURE_OPTIONS
+
gcc-dg-runtest [lsort [glob $srcdir/$subdir/*.c]] ""
gcc-dg-runtest [lsort [glob $srcdir/c-c++-common/guality/*.c]] "-Wc++-compat"
}
--
1.7.1
Re: [PATCH, libgfortran]: Fix support_fpu_rounding_mode and add -mieee flags for alpha
On Wed, Jul 2, 2014 at 11:16 AM, FX wrote:
> Thanks very much for the patch. I have a few questions:
>
>> the patch removes -O0 from dg-additiona-options in IEEE testsuite, as this
>> always override default optimization flag.
>
> That was my purpose: this test can fail with optimization (on x86_64, IIRC),
> hence the -O0 should override the default optimization flags in the testsuite.
I did test this on x86_64-linux-gnu (with and without -m32) and there
were no problems with the testrun.
>>* gfortran.dg/ieee/ieee_rounding_1.f90 (dg-additional-options): Add.
>
> If -mfp-rounding-mode=d is required on alpha for IEEE conformance, it should
> be added to add-ieee-options in lib/fortran-torture.exp, shouldn’t it? Rather
> than enabled on a case-by-case basis. Also, we might want to document these
> target-specific options here:
> https://gcc.gnu.org/onlinedocs/gfortran/IEEE-modules.html
I'm not sure if this is good idea ATM, since very few testcases (one
exactly ;) ) require strict rounding modes. Due to this, I don't think
dynamic rounding mode should be enabled for all testcases, as it comes
with a price. We can review this in future, if many testcases depend
on this option.
>> BTW: On alpha, it is possible to enable underflow handling:
>>
>> #ifdef __USE_GNU
>> /* On later hardware, and later kernels for earlier hardware, we can forcibly
>> underflow denormal inputs and outputs. This can speed up certain programs
>> significantly, usually without affecting accuracy. */
>> enum
>> {
>>FE_MAP_DMZ =1UL << 12, /* Map denorm inputs to zero */
>> #define FE_MAP_DMZ FE_MAP_DMZ
>>
>>FE_MAP_UMZ =1UL << 13, /* Map underflowed outputs to zero */
>> #define FE_MAP_UMZ FE_MAP_UMZ
>> };
>> #endif
>>
>> FX, if you care for this option, I can help test the patch and
>> corresponding testcases.
>
> Yes, that’s interesting. What libc function do you call with those
> FE_MAP_{D,U}MZ values?
Hm, it looks that this functionality is handled through fesetenv, but
I'm not 100% sure.
> I would also like to enable FTZ for i386/x86_64 at some point, but my issue
> there is that it’s not “universal”, ie it’s only for SSE math if I understand
> correctly. One point of view is that it’s better than nothing (especially now
> that SSE math is probably the most used mode), but another point of view is
> that it wouldn’t be standard conforming.
It is possible to test _SSE_MATH_ and _SSE2_MATH_ here?
Uros.
[PATCH, i386] Add prefixes avoidance tuning for silvermont target
Hi,
Silvermont processors have penalty for instructions having 4+ bytes of prefixes
(including escape bytes in opcode). This situation happens when REX prefix is
used in SSE4 instructions. This patch tries to avoid such situation by
preferring xmm0-xmm7 usage over xmm8-xmm15 in those instructions. I achieved
it by adding new tuning flag and new alternatives affected by tuning.
SSE4 instructions are not very widely used by GCC but I see some significant
gains caused by this patch (tested on Avoton on -O3).
DENmark (EEMBC2.0) +2%
TCPmark (EEMBC2.0) +32%
SPEC2000 +0,5%
Bootstrapped and tested on linux-x86_64.
Does it look OK for trunk?
Thanks,
Ilya
--
gcc/
2014-07-02 Ilya Enkovich
* config/i386/constraints.md (Yr): New.
* config/i386/i386.h (reg_class): Add NO_REX_SSE_REGS.
(REG_CLASS_NAMES): Likewise.
(REG_CLASS_CONTENTS): Likewise.
* config/i386/sse.md (*vec_concatv2sf_sse4_1): Add alternatives
which use only NO_REX_SSE_REGS.
(vec_set_0): Likewise.
(*vec_setv4sf_sse4_1): Likewise.
(sse4_1_insertps): Likewise.
(*sse4_1_extractps): Likewise.
(*sse4_1_mulv2siv2di3): Likewise.
(*_mul3): Likewise.
(*sse4_1_3): Likewise.
(*sse4_1_3): Likewise.
(*sse4_1_eqv2di3): Likewise.
(sse4_2_gtv2di3): Likewise.
(*vec_extractv4si): Likewise.
(*vec_concatv2si_sse4_1): Likewise.
(vec_concatv2di): Likewise.
(_blend): Likewise.
(_blendv): Likewise.
(_dp): Likewise.
(_movntdqa): Likewise.
(_mpsadbw): Likewise.
(sse4_1_packusdw): Likewise.
(_pblendvb): Likewise.
(sse4_1_pblendw): Likewise.
(sse4_1_phminposuw): Likewise.
(sse4_1_v8qiv8hi2): Likewise.
(sse4_1_v4qiv4si2): Likewise.
(sse4_1_v4hiv4si2): Likewise.
(sse4_1_v2qiv2di2): Likewise.
(sse4_1_v2hiv2di2): Likewise.
(sse4_1_v2siv2di2): Likewise.
(sse4_1_ptest): Likewise.
(_round): Likewise.
(sse4_1_round): Likewise.
* config/i386/subst.md (mask_prefix4): New.
* config/i386/x86-tune.def (X86_TUNE_AVOID_4BYTE_PREFIXES): New.
gcc/testsuites/
2014-07-02 Ilya Enkovich
* gcc.target/i386/sse2-init-v2di-2.c: Adjust to changed
vec_concatv2di template.
diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md
index 8e0a583..d6a0ea9 100644
--- a/gcc/config/i386/constraints.md
+++ b/gcc/config/i386/constraints.md
@@ -105,6 +105,8 @@
;; d Integer register when integer DFmode moves are enabled
;; x Integer register when integer XFmode moves are enabled
;; f x87 register when 80387 floating point arithmetic is enabled
+;; r SSE regs not requiring REX prefix when prefixes avoidance is enabled
+;; and all SSE regs otherwise
(define_register_constraint "Yz" "TARGET_SSE ? SSE_FIRST_REG : NO_REGS"
"First SSE register (@code{%xmm0}).")
@@ -147,6 +149,10 @@
"(ix86_fpmath & FPMATH_387) ? FLOAT_REGS : NO_REGS"
"@internal Any x87 register when 80387 FP arithmetic is enabled.")
+(define_register_constraint "Yr"
+ "TARGET_SSE ? (X86_TUNE_AVOID_4BYTE_PREFIXES ? NO_REX_SSE_REGS :
ALL_SSE_REGS) : NO_REGS"
+ "@internal Lower SSE register when avoiding REX prefix and all SSE registers
otherwise.")
+
;; We use the B prefix to denote any number of internal operands:
;; s Sibcall memory operand, not valid for TARGET_X32
;; w Call memory operand, not valid for TARGET_X32
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 9e3ef94..54239e9 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -1282,6 +1282,7 @@ enum reg_class
FP_TOP_REG, FP_SECOND_REG, /* %st(0) %st(1) */
FLOAT_REGS,
SSE_FIRST_REG,
+ NO_REX_SSE_REGS,
SSE_REGS,
EVEX_SSE_REGS,
ALL_SSE_REGS,
@@ -1339,6 +1340,7 @@ enum reg_class
"FP_TOP_REG", "FP_SECOND_REG", \
"FLOAT_REGS", \
"SSE_FIRST_REG",\
+ "NO_REX_SSE_REGS", \
"SSE_REGS", \
"EVEX_SSE_REGS",\
"ALL_SSE_REGS", \
@@ -1378,6 +1380,7 @@ enum reg_class
{ 0x0200, 0x0, 0x0 }, /* FP_SECOND_REG */ \
{ 0xff00, 0x0, 0x0 }, /* FLOAT_REGS */\
{ 0x20, 0x0, 0x0 }, /* SSE_FIRST_REG */ \
+{ 0x1fe0, 0x00, 0x0 }, /* NO_REX_SSE_REGS */ \
{ 0x1fe0, 0x1fe000, 0x0 }, /* SSE_REGS */ \
{ 0x0,0xffe0, 0x1f }, /* EVEX_SSE_REGS */ \
{ 0x1fe0,0xe000, 0x1f }, /* ALL_SSE_REGS */ \
diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
index d907353..1e561c2 100644
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -5585,26 +5585,28 @@
;; Although insertps takes register source, we prefer
Re: [PATH] Intel offload library
> * Don't duplicate the logic for what's a hosted POSIX system; refactor it > to a common fragment in config/ (I guess it needs to be a shell script > fragment there rather than an actual autoconf macro, since you're using > that logic in configure.tgt which is itself such a fragment). I've attached the patch with changes related to the point. Do you mean something like this? -- Best regards, Andrey Turetskiy liboffload_config.patch Description: Binary data
Re: [PATCH] [ARM] [RFC] Fix longstanding push_minipool_fix ICE (PR49423, lp1296601)
On 30 June 2014 14:26, Richard Earnshaw wrote: > On 30/06/14 13:53, Charles Baylis wrote: >> I see two options to fix it - one is to teach the back-end to >> successfully generate code for this insn, and the other is to teach >> the back-end that such an insn is not valid. My proposed patch does >> the former. The latter can presumably be achieved by providing a >> different kind of memory constraint which disallows constant pool >> references for these insns although I haven't tried this yet. > > I think we should be doing the latter (not permitting these operations). > If we wanted to do the former, we could just add an offset range for > the insn. > > The reason we don't want the former is that the offset ranges are too > small and overly constrain literal pool placement. The attached patch adds a 'Uh' constraint, which means the same as 'm', except that literal pool references are not allowed. Patterns which generate ldr[s]b or ldr[s]h have been updated to use it, and the pool_range attributes have been removed from those patterns. Bootstrapped and make-checked with no regressions on qemu for arm-unknown-linux-gnueabihf. Charles Baylis PR target/49423 * config/arm/arm-protos.h (arm_legitimate_address_p, arm_is_constant_pool_ref): Add prototypes. * config/arm/arm.c (arm_legitimate_address_p): Remove static. (arm_is_constant_pool_ref) New function. * config/arm/arm.md (unaligned_loadhis, arm_zero_extendhisi2_v6, arm_zero_extendqisi2_v6): Use Uh constraint for memory operand. (arm_extendhisi2, arm_extendhisi2_v6): Use Uh constraint for memory operand and remove pool_range and neg_pool_range attributes. (arm_extendqihi_insn, arm_extendqisi, arm_extendqisi_v6): Remove pool_range and neg_pool_range attributes. * config/arm/constraints.md (Uh): New constraint. (Uq): Don't allow constant pool references. OK for trunk? 0001-Fix-push_minipool_fix-ICE.patch Description: application/download
Re: [GOMP4, RFC] Support of offloading for Intel MIC targets.
Hello Kirill, Kirill Yukhin wrote: > This branch should be capable to perform offload to Intel targets (Xeon PHI) Which Xeon PHI does it support? Knights Corner, Knights Landing, both? Tobias
Re: Warn when returning the address of a temporary (middle-end) v2
On Mon, Jun 30, 2014 at 11:37:50PM +0200, Marc Glisse wrote: > On Mon, 30 Jun 2014, Jeff Law wrote: > > >On 06/29/14 03:22, Marc Glisse wrote: > >> > >>After looking at PR 61597, I updated the 2 conditions to: > >> > >>+ if ((TREE_CODE (valbase) == VAR_DECL > >>+ && !is_global_var (valbase)) > >>+ || TREE_CODE (valbase) == PARM_DECL) > >> > >>a PARM_DECL is a local variable and returning its address is wrong, > >>isn't it? > >Right. It can live in either a caller or callee allocated slot. > > The "caller" case scares me a bit. Is it really wrong to return the > address in that case? The slot still exists after returning if the > caller is responsible for it. At least on powerpc64, which uses a caller allocated parameter save area, returning the address of something in the parameter save area merits a warning. The ABIs explicitly state that the parameter save area is not preserved over function calls. Also note that anything left in a caller allocated parameter save area will potentially be trashed by arguments written for the next call. -- Alan Modra Australia Development Lab, IBM
Re: [PATCH] Power/GCC: Subword atomic operation endianness check bug fix
On Wed, Jul 02, 2014 at 11:05:23AM +0100, Maciej W. Rozycki wrote: > As it > happens both macros have the same value for the Power target Thanks! Looks good to me. As you note above it is effectively a documentation fix. > --- gcc-fsf-trunk-quilt.orig/gcc/config/rs6000/rs6000.c 2014-06-10 > 21:46:36.628975329 +0100 > +++ gcc-fsf-trunk-quilt/gcc/config/rs6000/rs6000.c2014-06-11 > 16:35:08.917560846 +0100 > @@ -19891,7 +19891,7 @@ rs6000_adjust_atomic_subword (rtx orig_m >shift = gen_reg_rtx (SImode); >addr = gen_lowpart (SImode, addr); >emit_insn (gen_rlwinm (shift, addr, GEN_INT (3), GEN_INT (shift_mask))); > - if (WORDS_BIG_ENDIAN) > + if (BYTES_BIG_ENDIAN) > shift = expand_simple_binop (SImode, XOR, shift, GEN_INT (shift_mask), >shift, 1, OPTAB_LIB_WIDEN); >*pshift = shift; -- Alan Modra Australia Development Lab, IBM
Re: [GOMP4, RFC] Support of offloading for Intel MIC targets.
Hello Tobias On 02 Jul 14:11, Tobias Burnus wrote: > Kirill Yukhin wrote: > > This branch should be capable to perform offload to Intel targets (Xeon PHI) > > Which Xeon PHI does it support? Knights Corner, Knights Landing, both? Currently liboffloadmic supports KNC. Future update will add support for KNL. However, GCC (main trunk and this branch) does not support code generation for KNC and we don't plan to do that. That is why we've implemented the emulator, which works on x86_64 and current GCC+liboffloadmic. Our plan for 4.10 is to support offload to KNL, no support for KNC is planned so far. It should be mentioned that non-liboffloadmic part works for any MIC target: KNC, KNL, ... -- Thanks, K
Re: Warn when returning the address of a temporary (middle-end) v2
On Wed, 2 Jul 2014, Alan Modra wrote: On Mon, Jun 30, 2014 at 11:37:50PM +0200, Marc Glisse wrote: On Mon, 30 Jun 2014, Jeff Law wrote: On 06/29/14 03:22, Marc Glisse wrote: After looking at PR 61597, I updated the 2 conditions to: + if ((TREE_CODE (valbase) == VAR_DECL + && !is_global_var (valbase)) + || TREE_CODE (valbase) == PARM_DECL) a PARM_DECL is a local variable and returning its address is wrong, isn't it? Right. It can live in either a caller or callee allocated slot. The "caller" case scares me a bit. Is it really wrong to return the address in that case? The slot still exists after returning if the caller is responsible for it. At least on powerpc64, which uses a caller allocated parameter save area, returning the address of something in the parameter save area merits a warning. Note that the patch not only warns, it also returns NULL instead of the pointer (at least the current version does), so it really needs to be certain. The ABIs explicitly state that the parameter save area is not preserved over function calls. Good, that makes me much more confident, thanks. Also note that anything left in a caller allocated parameter save area will potentially be trashed by arguments written for the next call. -- Marc Glisse
Re: [PATCH ARM]Handle REG addressing mode in output_move_neon explicitly
On Wed, Jul 2, 2014 at 7:48 AM, Ramana Radhakrishnan wrote: > On Mon, May 5, 2014 at 8:21 AM, bin.cheng wrote: >> >> >>> -Original Message- >>> From: Richard Earnshaw >>> Sent: Thursday, May 01, 2014 10:03 PM >>> To: Bin Cheng >>> Cc: gcc-patches@gcc.gnu.org >>> Subject: Re: [PATCH ARM]Handle REG addressing mode in >>> output_move_neon explicitly >>> >>> On 29/04/14 04:02, bin.cheng wrote: >>> > Hi, >>> > Function output_move_neon now generates vld1.64 for memory ref like >>> > "dx <- [r1:SI]", this is bogus because it requires at least 64-bit >>> > alignment for 32-bit aligned memory ref. It works now because GCC >>> > doesn't generate such insns in the first place, but things are going >>> > to change if memset/memcpy calls are inlined by using neon instructions. >>> > >>> >>> V[LD/ST]1.64 only need to be 64-bit aligned if strict alignment is >> enabled. We >>> normally assume that not to be the case. The exception to this is when an >> theoretically, this doesn't make the problem go away, right? >> >>> explicit alignment check is used in the address expression (the :64 >> suffix), >>> which causes the address to be checked for strict alignment at all times. >>> >>> Do you have a testcase? >> I can't provide a test case without the memset inlining patch. >> > Are the tests in the memset inlining patch now sufficient to expose > the problem or do we need another test ? Yes, it's covered by the 4th/7th test cases in memset inlining patch. Thanks, bin
Re: [GOMP4, RFC] Support of offloading for Intel MIC targets.
Hello Kirill, On Wed, Jul 02, 2014 at 04:35:01PM +0400, Kirill Yukhin wrote: > However, GCC (main trunk and this branch) does not support code generation > for KNC > and we don't plan to do that. Hmm, I had hoped that the work would include the forward porting of HJ's patches from https://software.intel.com/en-us/articles/intel-manycore-platform-software-stack-mpss to the current trunk. In my understanding, that's all what is needed, isn't it? Thanks for the reply, even though that was not exactly the answer I had hoped for. Tobias PS: Contrary to GCC, Binutils has KNC support since a few years.
Re: [GOMP4, RFC] Support of offloading for Intel MIC targets.
On 02 Jul 15:06, Tobias Burnus wrote: > Hmm, I had hoped that the work would include the forward porting of HJ's > patches from > https://software.intel.com/en-us/articles/intel-manycore-platform-software-stack-mpss > to the current trunk. In my understanding, that's all what is needed, isn't > it? That compiler only supports KNC ABI. No KNC instructions are supported, only x87. Biggest problem is that KNL (which are in GCC from 4.9) new instructions and KNC ones intersect which make rebasing on top of trunk equal to rewriting. We still have no demand for such no-new-insns GCC. -- Thanks, K
Re: [PATCH] Don't allow combination of read/write and earlyclobber constraint modifier
On 02/07/14 08:52, Tom de Vries wrote:
> On 01-07-14 21:47, Jeff Law wrote:
>> On 07/01/14 13:27, Tom de Vries wrote:
>>> So my question is: is the combination of '&' and '+' supported ? If so,
>>> what is the exact semantics ? If not, should we warn or give an error ?
> >
>> I don't think we can define any reasonable semantics for &+. My
>> recommendation
>> would be for this to be considered a hard error.
>>
>
Why would this be any different in behaviour from use of operand tie
constraints to early-clobber?
In my view, it says that this operand is safe from the early-clobber
limitation, but other operands are not and need to be in other registers.
Eg op0 ("=&r") op1("0") op2("r") says that op1 must be the same register
as op0, but op2 must be a different register, so A = A B is ok, but
A = A A is not.
R.
> [ move discussion from gcc ml to gcc-patches ml ]
>
> Attached patch detects the combination of + and & constrains during genrecog,
> and generates an error like this:
> ...
> /home/vries/gcc_versions/devel/src/gcc/config/aarch64/aarch64-simd.md:1020:
> operand 0 has in-out reload, incompatible with earlyclobber
> /home/vries/gcc_versions/devel/src/gcc/config/aarch64/aarch64-simd.md:1020:
> operand 0 has in-out reload, incompatible with earlyclobber
> /home/vries/gcc_versions/devel/src/gcc/config/aarch64/aarch64-simd.md:1020:
> operand 0 has in-out reload, incompatible with earlyclobber
> make[2]: *** [s-recog] Error 1
> ...
> The error triggers three times, once for each mode iterator element.
>
> OK if x86_64 bootstrap succeeds ?
>
> Thanks,
> - Tom
>
>
> 0004-Don-t-allow-earlyclobber-modifier-with-read-write-mo.patch
>
>
> 2014-07-02 Tom de Vries
>
> * genrecog.c (validate_pattern): Don't allow earlyclobber constraint
> modifier with read/write constraint modifier.
>
> diff --git a/gcc/genrecog.c b/gcc/genrecog.c
> index 457b59c..ad709ee 100644
> --- a/gcc/genrecog.c
> +++ b/gcc/genrecog.c
> @@ -481,6 +481,13 @@ validate_pattern (rtx pattern, rtx insn, rtx set, int
> set_code)
> rtx_name[GET_CODE (insn)]);
> }
>
> + if (constraints0 == '+'
> + && strchr (XSTR (pattern, 2), '&') != NULL)
> + error_with_line (pattern_lineno,
> +"operand %d has in-out reload, incompatible with"
> +" earlyclobber",
> +XINT (pattern, 0));
> +
> /* A MATCH_OPERAND that is a SET should have an output reload. */
> else if (set && constraints0)
> {
>
libgo patch committed: Don't explicitly free tiny blocks
This patch fixes libgo to not explicitly free tiny blocks when deleting
an entry from a map. The memory allocator now has a special case for
tiny objects (less than 16 bytes), and they can not be explicitly freed,
only garbage collected. This is PR go/61620. Bootstrapped and ran Go
testsuite on x86_64-unknown-linux-gnu. Committed to mainline.
Ian
diff -r a45b1aa17901 libgo/runtime/go-map-delete.c
--- a/libgo/runtime/go-map-delete.c Tue Jul 01 16:18:58 2014 -0700
+++ b/libgo/runtime/go-map-delete.c Wed Jul 02 07:20:02 2014 -0700
@@ -8,6 +8,7 @@
#include
#include "runtime.h"
+#include "malloc.h"
#include "go-alloc.h"
#include "go-assert.h"
#include "map.h"
@@ -47,7 +48,8 @@
if (equalfn (key, entry + key_offset, key_size))
{
*pentry = *(void **) entry;
- __go_free (entry);
+ if (descriptor->__entry_size >= TinySize)
+ __go_free (entry);
map->__element_count -= 1;
break;
}
Re: [PATCH] Fix ICE on old style init of derived components
On Wed, Jul 02, 2014 at 09:48:18AM +0200, Jakub Jelinek wrote: > Hi! > > As the testcase shows, we ICE on old style initialization of derived type > components. ifort also rejects these, I think it doesn't make sense to > support such initializations of derived type components. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.9? > Yes. -- Steve
Re: [patch 1/4] change specific int128 -> generic intN
> Do you have modes whose size is not multiple of the unit? Yes. That's exactly the problem I'm trying to solve here. I'm making partial int modes have real corresponding types, and they can be any bit size, with target PS*modes to match. The MSP430, for example, has 20-bit modes, 20-bit operands, and __int20. Rounding up to byte sizes forces everything into an emulated SImode which makes code size huge and performance much worse. Thus, in these cases, TYPE_SIZE and TYPE_SIZE_UNIT no longer have a "* BITS_PER_UNIT" mathematical relationship.
Re: [C++ Patch] PR 51448, 53618, 58059
.. consider this patch withdrawn. I believe that something is going wrong indeed as part of most_specialized_instantiation but the details need to be figured out. I'm now focusing on fn_type_unification via get_bindings. Paolo.
More informative ODR warnings
Hi,
this patch adds structural comparsion into ODR warnings, so we do not rely
on types_compatible_p to checks if the individual variants of same
name looks same. This allows us to give more precise reason for the
mismatch and also be more strict than canonical type merging.
Function odr_types_equivalent_p is based on canonical type hash equivalency
from lto.c. Originally I wanted to share the implementation, but details
seems sufficiently different to justify a fresh copy of the whole monster.
To avoid false warnings on types containg va_list pointer, I added
the disucssed hack to type_in_anonymous_namespace_p to return false
on types that do not have public stub but no context (since all anonymous
types produced by FE have as a context something)
Bootstrapped/regtested x86_64-linux, will commit it later this week if there
are no complains.
Currently we warn only about polymorphic types. With my experimental patch for
full ODR type merging we get the following warnings (minus the information
about conflicting units). Those seems to be real bugs in firefox.
/aux/hubicka/firefox/netwerk/sctp/datachannel/DataChannel.h:64:0: warning:
field ‘mSpa’ (of type ‘struct BufferedMsg’) violates one definition rule
[-Wodr]
struct sctp_sendv_spa *mSpa;
^
../../../../dist/include/mozilla/net/DataChannel.h:64:0: note: a field of same
name but different type is defined in another translation unit
/aux/hubicka/firefox/netwerk/streamconv/converters/nsMultiMixedConv.cpp:466:0:
warning: field ‘mBuffer’ (of type ‘struct AutoFree’) violates one definition
rule [-Wodr]
char *mBuffer;
^
/aux/hubicka/firefox/dom/base/nsJSEnvironment.cpp:307:0: note: a field with
different name is defined in another translation unit
void *mPtr;
^
lto1: note: Conflicting compilation units:
/aux/hubicka/firefox/dom/base/nsJSEnvironment.cpp and
/aux/hubicka/build-firefox491-inst4/netwerk/streamconv/converters/Unified_cpp_netwerk_streamconv_converters0.cpp
../../dist/include/zipstruct.h:47:25: warning: field ‘MOZ_Z_crc32’ (of type
‘struct ZipCentral_’) violates one definition rule [-Wodr]
../../dist/include/zipstruct.h:47:0: note: a field with different name is
defined in another translation unit
lto1: note: Conflicting compilation units:
/aux/hubicka/build-firefox491-inst4/dom/file/Unified_cpp_dom_file0.cpp and
/aux/hubicka/firefox/xpcom/build/FileLocation.cpp
/aux/hubicka/firefox/modules/libjar/zipstruct.h:24:0: warning: field
‘MOZ_Z_crc32’ (of type ‘struct ZipLocal_’) violates one definition rule
[-Wodr]
unsigned char crc32 [4];
^
../../dist/include/zipstruct.h:24:0: note: a field with different name is
defined in another translation unit
lto1: note: Conflicting compilation units:
/aux/hubicka/build-firefox491-inst4/dom/file/Unified_cpp_dom_file0.cpp and
/aux/hubicka/firefox/modules/libjar/nsZipArchive.cpp
../../dist/include/mp4_demuxer/audio_decoder_config.h:112:0: warning: field
‘sample_format_’ (of type ‘struct AudioDecoderConfig’) violates one definition
rule [-Wodr]
../../../dist/include/mp4_demuxer/audio_decoder_config.h:112:0: note: a field
of same name but different type is defined in another translation unit
SampleFormat sample_format_;
^
../../dist/include/mp4_demuxer/audio_decoder_config.h:41:0: note: type
‘SampleFormat’ should match type ‘AVSampleFormat’
../../../dist/include/mp4_demuxer/audio_decoder_config.h:41:0: note: the
incompatible type is defined here
enum SampleFormat {
^
/aux/hubicka/firefox/modules/libpref/src/nsPrefBranch.cpp:51:16: warning: field
‘parent’ (of type ‘struct EnumerateData’) violates one definition rule [-Wodr]
const char *parent;
^
/aux/hubicka/firefox/layout/base/nsPresArena.cpp:123:0: note: a field with
different name is defined in another translation unit
nsArenaMemoryStats* stats;
^
lto1: note: Conflicting compilation units:
/aux/hubicka/firefox/layout/base/nsPresArena.cpp and
/aux/hubicka/build-firefox491-inst4/modules/libpref/src/Unified_cpp_modules_libpref_src0.cpp
./glslang_lex.cpp:815:0: warning: field ‘yyextra_r’ (of type ‘struct yyguts_t’)
violates one definition rule [-Wodr]
./Tokenizer.cpp:575:0: note: a field of same name but different type is defined
in another translation unit
lto1: note: Conflicting compilation units:
/aux/hubicka/build-firefox491-inst4/gfx/angle/Unified_cpp_gfx_angle3.cpp and
/aux/hubicka/firefox/gfx/angle/src/compiler/glslang_lex.cpp
/aux/hubicka/firefox/rdf/base/src/nsInMemoryDataSource.cpp:148:0: warning:
field ‘mHdr’ (of type ‘struct Entry’) violates one definition rule [-Wodr]
PLDHashEntryHdr mHdr;
^
/aux/hubicka/firefox/gfx/skia/trunk/src/core/SkFlattenable.cpp:67:0: note: a
field with different name is defined in another translation unit
const char* fName;
^
lto1: note: Conflicting compilation units:
/aux/hubicka/firefox/gfx/skia/trunk/src/core/SkFlattenable.cpp and
/aux/hubicka/build-firefox491-inst4/rdf/base/src/Unified_cpp_rdf_base_src0.cpp
/
Re: [C++ Patch] PR 51448, 53618, 58059
Hi again, On 07/02/2014 05:06 PM, Paolo Carlini wrote: .. consider this patch withdrawn. I believe that something is going wrong indeed as part of most_specialized_instantiation but the details need to be figured out. I'm now focusing on fn_type_unification via get_bindings. In fact my typo above most_specialized_instantiation vs most_specialized class reveals something: we don't seem to have an analogous for *classes* of the mechanism implemented in fn_type_unification for functions, and all the testcases I'm handling involve *classes*. Paolo.
Re: Fix finding reg-sets of call insn in collect_fn_hard_reg_usage
On 07/02/14 02:32, Eric Botcazou wrote: So in order to known whether it's safe and optimal to use regs_ever_live instead, the question is whether the passes after pass_free_cfg (are allowed to) add or remove sets or clobbers of call_really_used_regs. I don't know the full answer there. pass_machine_reorg is run after pass_free_cfg and it can do pretty much what it wants so I'd think that the answer is yes. Agreed -- until such time as we decide to place some limits on the machine dependent reorg pass. Right now I think the only limit is the result of that pass must be valid RTL. Pretty weak :( Jeff
Re: [PATCH, i386] Add prefixes avoidance tuning for silvermont target
Ilya Enkovich writes: > Silvermont processors have penalty for instructions having 4+ bytes of > prefixes (including escape bytes in opcode). This situation happens > when REX prefix is used in SSE4 instructions. This patch tries to > avoid such situation by preferring xmm0-xmm7 usage over xmm8-xmm15 in > those instructions. I achieved it by adding new tuning flag and new > alternatives affected by tuning. Why make it a tuning flag? Shouldn't this help unconditionally for code size everywhere? Or is there some drawback? -Andi -- a...@linux.intel.com -- Speaking for myself only
Re: [PATCH, i386] Add prefixes avoidance tuning for silvermont target
On Wed, Jul 02, 2014 at 09:21:25AM -0700, Andi Kleen wrote: > Ilya Enkovich writes: > > > Silvermont processors have penalty for instructions having 4+ bytes of > > prefixes (including escape bytes in opcode). This situation happens > > when REX prefix is used in SSE4 instructions. This patch tries to > > avoid such situation by preferring xmm0-xmm7 usage over xmm8-xmm15 in > > those instructions. I achieved it by adding new tuning flag and new > > alternatives affected by tuning. > > Why make it a tuning flag? Shouldn't this help unconditionally for code > size everywhere? Or is there some drawback? I don't think it will make code smaller, if you already have some value in xmm8..xmm15 register, then by not allowing those registers directly on SSE4 insns just means it reloading and larger code. BTW, is that change needed also when emitting AVX insns instead of SSE4? Jakub
Re: [PATCH] Fix various undefined behaviors in gcc found by bootstrap-ubsan
On Jul 2, 2014, at 1:14 AM, Jakub Jelinek wrote:
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
wide-int part, Ok.
> * wide-int-print.cc (print_decs): Negate UHWI instead of HWI.
> --- gcc/wide-int-print.cc.jj 2014-05-11 22:21:04.0 +0200
> +++ gcc/wide-int-print.cc 2014-06-30 12:02:11.349485371 +0200
> @@ -62,7 +62,8 @@ print_decs (const wide_int_ref &wi, char
> || (wi.get_len () == 1))
> {
> if (wi::neg_p (wi))
> - sprintf (buf, "-" HOST_WIDE_INT_PRINT_UNSIGNED, -wi.to_shwi ());
> + sprintf (buf, "-" HOST_WIDE_INT_PRINT_UNSIGNED,
> + -(unsigned HOST_WIDE_INT) wi.to_shwi ());
> else
> sprintf (buf, HOST_WIDE_INT_PRINT_DEC, wi.to_shwi ());
> }
Re: [PATCH, i386] Add prefixes avoidance tuning for silvermont target
Hello! > Silvermont processors have penalty for instructions having 4+ bytes of > prefixes (including escape > bytes in opcode). This situation happens when REX prefix is used in SSE4 > instructions. This > patch tries to avoid such situation by preferring xmm0-xmm7 usage over > xmm8-xmm15 in those > instructions. I achieved it by adding new tuning flag and new alternatives > affected by tuning. > SSE4 instructions are not very widely used by GCC but I see some significant > gains caused by > this patch (tested on Avoton on -O3). > 2014-07-02 Ilya Enkovich > * config/i386/constraints.md (Yr): New. > * config/i386/i386.h (reg_class): Add NO_REX_SSE_REGS. > (REG_CLASS_NAMES): Likewise. > (REG_CLASS_CONTENTS): Likewise. > * config/i386/sse.md (*vec_concatv2sf_sse4_1): Add alternatives > which use only NO_REX_SSE_REGS. You don't need to add alternatives, just change existing alternatives from "x" to "Yr". The allocator will handle reduced register set just fine. BTW: I think that "Yr" is a very confusing name for the alternative. I'd suggest "Ya". Uros.
Re: [PATCH, i386] Add prefixes avoidance tuning for silvermont target
On Jul 2, 2014, at 9:27 AM, Jakub Jelinek wrote: > On Wed, Jul 02, 2014 at 09:21:25AM -0700, Andi Kleen wrote: >> Ilya Enkovich writes: >> >>> Silvermont processors have penalty for instructions having 4+ bytes of >>> prefixes (including escape bytes in opcode). This situation happens >>> when REX prefix is used in SSE4 instructions. This patch tries to >>> avoid such situation by preferring xmm0-xmm7 usage over xmm8-xmm15 in >>> those instructions. I achieved it by adding new tuning flag and new >>> alternatives affected by tuning. >> >> Why make it a tuning flag? Shouldn't this help unconditionally for code >> size everywhere? Or is there some drawback? > > I don't think it will make code smaller, if you already have some value in > xmm8..xmm15 register, then by not allowing those registers directly on SSE4 > insns just means it reloading and larger code. I can’t help but think a better way to do this is to explain the costs of the REX registers as being more expensive, then let the register allocator prefer the cheaper registers. You then leave them all as valid, which I think is better than disappearing 1/2 of the registers. Is it really cheaper to spill and reload those than use a REX register?
Re: More informative ODR warnings
- Original Message - > /aux/hubicka/firefox/netwerk/sctp/datachannel/DataChannel.h:64:0: warning: > field ‘mSpa’ (of type ‘struct BufferedMsg’) violates one definition rule > [-Wodr] Can we reword this warning? The "of type 'struct BufferedMsg'" could be easily taken to mean that the type of the field is 'struct BufferedMsg', rather than the intended meaning. Perhaps "within type 'struct BufferedMsg'"? -Nathan
Re: [Patch, libgomp] libgomp.fortran/fortran.exp - add -fintrinsic-modules-path ${blddir}
Hi!
Reopening this oldie:
On Wed, 19 Dec 2012 16:38:41 +0100, Tobias Burnus wrote:
> The attached patch adds
> -fintrinsic-modules-path ${blddir}
> otherwise, the compiler might have trouble finding the libraries using
> "use, INTRINSIC :: omp_lib". Without "intrinsic" it searches the "-I"
> directories.
>
> (The compiler supports multiple -fintrinsic-modules-path, cf.
> gfc_add_intrinsic_modules_path; however, the only physically existing
> intrinsic .mod files are those of libgomp. ISO_C_binding and
> ISO_Fortran_env do only virtually exist in the compiler.)
>
> Tested on x86-64-gnu-linux.
> 2012-12-19 Tobias Burnus
>
> * testsuite/libgomp.fortran/fortran.exp: Set
> -fintrinsic-modules-path.
>
> diff --git a/libgomp/testsuite/libgomp.fortran/fortran.exp
> b/libgomp/testsuite/libgomp.fortran/fortran.exp
> index 5fa42f4..68440d18 100644
> --- a/libgomp/testsuite/libgomp.fortran/fortran.exp
> +++ b/libgomp/testsuite/libgomp.fortran/fortran.exp
> @@ -14,6 +14,7 @@ set quadmath_library_path "../libquadmath/.libs"
> dg-init
>
> if { $blddir != "" } {
> +lappend ALWAYS_CFLAGS "additional_flags=-fintrinsic-modules-path
> ${blddir}"
> # Look for a static libgfortran first.
> if [file exists "${blddir}/${lang_library_path}/libgfortran.a"] {
> set lang_test_file "${lang_library_path}/libgfortran.a"
Doing build-tree testing, this results in several test becoming
UNSUPPORTED (instead of PASS), in particular those that do things like
»dg-require-effective-target tls_runtime« or »dg-do run { target
vect_simd_clones }«, and we then run into:
spawn [...]/build/gcc/xgcc -B[...]/build/gcc/
-B[...]/build/x86_64-unknown-linux-gnu/./libgomp/
-B[...]/build/x86_64-unknown-linux-gnu/./libgomp/.libs
-I[...]/build/x86_64-unknown-linux-gnu/./libgomp
-I[...]/source/libgomp/testsuite/.. -fmessage-length=0
-fno-diagnostics-show-caret -fdiagnostics-color=never -fopenmp
-fintrinsic-modules-path=[...]/build/x86_64-unknown-linux-gnu/./libgomp
-B[...]/build/x86_64-unknown-linux-gnu/./libgomp/../libquadmath/.libs/ -O2
-mavx -B[...]/build/x86_64-unknown-linux-gnu/./libgomp/../libgfortran/.libs -c
-o avx32186.o avx32186.c
cc1: warning: command line option
'-fintrinsic-modules-path=[...]/build/x86_64-unknown-linux-gnu/./libgomp' is
valid for Fortran but not for C
output is:
cc1: warning: command line option
'-fintrinsic-modules-path=[...]/build/x86_64-unknown-linux-gnu/./libgomp' is
valid for Fortran but not for C
UNSUPPORTED: libgomp.fortran/declare-simd-1.f90
UNSUPPORTED: libgomp.fortran/declare-simd-2.f90
Is there any obvious cure for this? (Obviously, reverting that change
isn't the solution, because then libgomp.fortran/use_intrinsic_1.f90
FAILs.)
I think this issue also is part of what has been reported in
, but not yet properly addressed.
Grüße,
Thomas
pgpGtx7kfir4Y.pgp
Description: PGP signature
Re: [patch] gcc fstack-protector-explicit
On Tue, Jul 1, 2014 at 6:34 PM, Daniel Gutson wrote: > On Tue, Jul 1, 2014 at 2:25 PM, Jeff Law wrote: >> On 03/19/14 08:06, Marcos Díaz wrote: >>> >>> Well, finally I have the assignment, could you please review this patch? >> >> Thanks. >> >> My first thought was that if we've marked the function with an explicit >> static protector attribute, then it ought to be protected regardless of any >> flags. Is there some reason to require the -fstack-protect-explicit? > > They can work separately, since the logic is: > > if NOT stack-protect-explicit >a function can be protected by the current logic OR it has the attribute >(a function may be not automatically protected with the current logic) > ELSE // stack-protect-explicit >only functions marked with the attribute will be protected. > If there isn't any stack-protect flag (strong, common or explicit) the attribute has no effect > IOW, when no stack-protect-explicit, the functions may not be > protected due to current logic, so the attribute acts as an override > to request protection. > >> >> The patch itself is relatively simple and I don't see anything that looks >> terribly wrong at first glance. I think we just need to make sure we're on >> the same page WRT needing the -fstack-protect-explicit flag. >> >> jeff >> >> > > > > -- > > Daniel F. Gutson > Chief Engineering Officer, SPD > > > San Lorenzo 47, 3rd Floor, Office 5 > > Córdoba, Argentina > > > Phone: +54 351 4217888 / +54 351 4218211 > > Skype: dgutson -- __ Marcos Díaz Software Engineer San Lorenzo 47, 3rd Floor, Office 5 Córdoba, Argentina Phone: +54 351 4217888 / +54 351 4218211/ +54 351 7617452 Skype: markdiaz22
Re: More informative ODR warnings
On Jul 2, 2014, at 10:52 AM, Nathan Froyd wrote: > - Original Message - >> /aux/hubicka/firefox/netwerk/sctp/datachannel/DataChannel.h:64:0: warning: >> field ‘mSpa’ (of type ‘struct BufferedMsg’) violates one definition rule >> [-Wodr] > > Can we reword this warning? The "of type 'struct BufferedMsg'" could be > easily taken to mean that the type of the field is 'struct BufferedMsg', > rather than the intended meaning. Perhaps "within type 'struct BufferedMsg’"? type is redundant with struct, all structs are types, so "within ‘struct” would be slightly shorter, as would just “in ‘struct”.
Re: More informative ODR warnings
> On Jul 2, 2014, at 10:52 AM, Nathan Froyd wrote: > > - Original Message - > >> /aux/hubicka/firefox/netwerk/sctp/datachannel/DataChannel.h:64:0: warning: > >> field ‘mSpa’ (of type ‘struct BufferedMsg’) violates one definition rule > >> [-Wodr] > > > > Can we reword this warning? The "of type 'struct BufferedMsg'" could be > > easily taken to mean that the type of the field is 'struct BufferedMsg', > > rather than the intended meaning. Perhaps "within type 'struct > > BufferedMsg’"? > > type is redundant with struct, all structs are types, so "within ‘struct” > would be slightly shorter, as would just “in ‘struct”. Yep, within `struct foo` sounds better. I will update the patch. Any other suggestions are welcome - it is not completely easy to give useful warnings here. At least we are better than previous poitining out two equivalent location and saying they are not compatible Honza
Re: [PATCH, i386] Add prefixes avoidance tuning for silvermont target
Mike Stump writes: Silvermont processors have penalty for instructions having 4+ bytes of prefixes (including escape bytes in opcode). This situation happens when REX prefix is used in SSE4 instructions. This patch tries to avoid such situation by preferring xmm0-xmm7 usage over xmm8-xmm15 in those instructions. I achieved it by adding new tuning flag and new alternatives affected by tuning. >>> >>> Why make it a tuning flag? Shouldn't this help unconditionally for code >>> size everywhere? Or is there some drawback? >> >> I don't think it will make code smaller, if you already have some value in >> xmm8..xmm15 register, then by not allowing those registers directly on SSE4 >> insns just means it reloading and larger code. > > I can’t help but think a better way to do this is to explain the costs > of the REX registers as being more expensive, then let the register > allocator prefer the cheaper registers. You then leave them all as > valid, which I think is better than disappearing 1/2 of the registers. Yes that would sound like a much better strategy. BTW I thought gcc already did that for the integer registers to avoid unnecessary prefixes, but maybe I misremember. Copying Honza. -Andi -- a...@linux.intel.com -- Speaking for myself only
[wwwdocs,Java] Rework java/ towards relative links and remove redundant CSS references
2014-07-02 Gerald Pfeifer * news.html: Make a number of links relative. * status.html: Ditto. * libgcj-classpath-compare.html: Remove redundant link to gcc.css. * gui-compare/libgcj-classpath-compare.html: Ditto. Committed. Yes, again this takes care of a number of http to https conversions, but making them unnecessary to begin with. ;-) Gerald Index: java/news.html === RCS file: /cvs/gcc/wwwdocs/htdocs/java/news.html,v retrieving revision 1.17 diff -u -r1.17 news.html --- java/news.html 28 Jun 2014 07:45:14 - 1.17 +++ java/news.html 2 Jul 2014 15:31:27 - @@ -178,7 +178,7 @@ July 31, 2003 Andrew Haley has checked in a major patch to the -http://gcc.gnu.org/projects/tree-ssa/";>tree-ssa branch +tree-ssa branch that allows us to convert entire functions to trees when reading from .class files. This is an important step towards better high-level optimizations for Java. @@ -202,7 +202,7 @@ May 14, 2003 -http://gcc.gnu.org/gcc-3.3/";>GCC 3.3 has been released. +GCC 3.3 has been released. This release includes many bug fixes, support for assert, and JDBC 3.0 support in java.sql and javax.sql, among other things. @@ -477,9 +477,9 @@ December 8, 2000 The libgcj repository is now https://gcc.gnu.org/ml/java/2000-12/msg00095.html";> -closed: We're moving our sources over to the gcc +closed: We're moving our sources over to the GCC https://gcc.gnu.org/ml/java-announce/2000/msg3.html";> -tree. Read http://gcc.gnu.org/svn.html";>here +tree. Read here how you will soon check your sources out. Index: java/status.html === RCS file: /cvs/gcc/wwwdocs/htdocs/java/status.html,v retrieving revision 1.32 diff -u -r1.32 status.html --- java/status.html1 Nov 2012 22:17:47 - 1.32 +++ java/status.html2 Jul 2014 15:31:27 - @@ -93,10 +93,10 @@ You can also see http://gcc.gnu.org/java/libgcj-classpath-compare.html";>a +href="libgcj-classpath-compare.html">a comparison of our classes with Classpath's. Differences here are merged from time to time. You can also see -http://gcc.gnu.org/java/gui-compare/libgcj-classpath-compare.html";> + a comparison of the GUI branch with Classpath. Index: java/libgcj-classpath-compare.html === RCS file: /cvs/gcc/wwwdocs/htdocs/java/libgcj-classpath-compare.html,v retrieving revision 1.308 diff -u -r1.308 libgcj-classpath-compare.html --- java/libgcj-classpath-compare.html 1 Dec 2004 17:31:54 - 1.308 +++ java/libgcj-classpath-compare.html 2 Jul 2014 15:31:27 - @@ -1,5 +1,4 @@ libgcj -vs- Classpath (No GUI Classes) -http://gcc.gnu.org/gcc.css";> libgcj -vs- Classpath (No GUI Classes) This page compares the "current" cvs libgcj against the "current" cvs Classpath. It was generated using the gen-classpath-compare Index: java/gui-compare/libgcj-classpath-compare.html === RCS file: /cvs/gcc/wwwdocs/htdocs/java/gui-compare/libgcj-classpath-compare.html,v retrieving revision 1.34 diff -u -r1.34 libgcj-classpath-compare.html --- java/gui-compare/libgcj-classpath-compare.html 1 Dec 2004 17:31:55 - 1.34 +++ java/gui-compare/libgcj-classpath-compare.html 2 Jul 2014 15:31:27 - @@ -1,5 +1,4 @@ libgcj -vs- Classpath (GUI Classes only) -http://gcc.gnu.org/gcc.css";> libgcj -vs- Classpath (GUI Classes only) This page compares the "current" cvs libgcj against the "current" cvs Classpath. It was generated using the gen-classpath-compare
Re: [PATCH] Power/GCC: Subword atomic operation endianness check bug fix
gcc/ * config/rs6000/rs6000.c (rs6000_adjust_atomic_subword): Use BYTES_BIG_ENDIAN rather than WORDS_BIG_ENDIAN to check for byte endianness. This patch is okay. Thanks for noticing it. Thanks, David
Re: [Patch, libgomp] libgomp.fortran/fortran.exp - add -fintrinsic-modules-path ${blddir}
Thomas Schwinge wrote:
Reopening this oldie:
index 5fa42f4..68440d18 100644
--- a/libgomp/testsuite/libgomp.fortran/fortran.exp
+++ b/libgomp/testsuite/libgomp.fortran/fortran.exp
@@ -14,6 +14,7 @@ set quadmath_library_path "../libquadmath/.libs"
dg-init
if { $blddir != "" } {
+lappend ALWAYS_CFLAGS "additional_flags=-fintrinsic-modules-path ${blddir}"
How about the following (only lightly tested). I wonder why I didn't use
it before – but it looks obvious.
Tobias
--- a/libgomp/testsuite/libgomp.fortran/fortran.exp
+++ b/libgomp/testsuite/libgomp.fortran/fortran.exp
@@ -48,5 +48,5 @@ if { $lang_test_file_found } {
|| [file exists
"${blddir}/${quadmath_library_path}/libquadmath.${shlib_ext}"] } {
- lappend ALWAYS_CFLAGS
"ldflags=-L${blddir}/${quadmath_library_path}/"
+ lappend ALWAYS_FFLAGS
"ldflags=-L${blddir}/${quadmath_library_path}/"
# Allow for spec subsitution.
- lappend ALWAYS_CFLAGS
"additional_flags=-B${blddir}/${quadmath_library_path}/"
+ lappend ALWAYS_FFLAGS
"additional_flags=-B${blddir}/${quadmath_library_path}/"
set ld_library_path
"$always_ld_library_path:${blddir}/${lang_library_path}:${blddir}/${quadmath_library_path}"
Re: More informative ODR warnings
On Wed, Jul 02, 2014 at 05:15:20PM +0200, Jan Hubicka wrote:
> Hi,
> this patch adds structural comparsion into ODR warnings, so we do not rely
> on types_compatible_p to checks if the individual variants of same
> name looks same. This allows us to give more precise reason for the
> mismatch and also be more strict than canonical type merging.
>
> Function odr_types_equivalent_p is based on canonical type hash equivalency
> from lto.c. Originally I wanted to share the implementation, but details
> seems sufficiently different to justify a fresh copy of the whole monster.
>
> To avoid false warnings on types containg va_list pointer, I added
> the disucssed hack to type_in_anonymous_namespace_p to return false
> on types that do not have public stub but no context (since all anonymous
> types produced by FE have as a context something)
>
> Bootstrapped/regtested x86_64-linux, will commit it later this week if there
> are no complains.
>
> Currently we warn only about polymorphic types. With my experimental patch
> for
> full ODR type merging we get the following warnings (minus the information
> about conflicting units). Those seems to be real bugs in firefox.
I can't find the code for the SampleFormat thing, but the rest of them
look like ODR violations to me.
> /aux/hubicka/firefox/netwerk/sctp/datachannel/DataChannel.h:64:0: warning:
> field ‘mSpa’ (of type ‘struct BufferedMsg’) violates one definition rule
> [-Wodr]
>struct sctp_sendv_spa *mSpa;
> ^
> ../../../../dist/include/mozilla/net/DataChannel.h:64:0: note: a field of
> same name but different type is defined in another translation unit
> /aux/hubicka/firefox/netwerk/streamconv/converters/nsMultiMixedConv.cpp:466:0:
> warning: field ‘mBuffer’ (of type ‘struct AutoFree’) violates one definition
> rule [-Wodr]
>char *mBuffer;
> ^
> /aux/hubicka/firefox/dom/base/nsJSEnvironment.cpp:307:0: note: a field with
> different name is defined in another translation unit
it seems to me this doesn't get at the real issue that the type names
are the same but the fields are different. maybe "a type of the same
name with different fields defined here"?
>void *mPtr;
> ^
> lto1: note: Conflicting compilation units:
> /aux/hubicka/firefox/dom/base/nsJSEnvironment.cpp and
> /aux/hubicka/build-firefox491-inst4/netwerk/streamconv/converters/Unified_cpp_netwerk_streamconv_converters0.cpp
> ../../dist/include/zipstruct.h:47:25: warning: field ‘MOZ_Z_crc32’ (of type
> ‘struct ZipCentral_’) violates one definition rule [-Wodr]
> ../../dist/include/zipstruct.h:47:0: note: a field with different name is
> defined in another translation unit
is it worth poking into why this case doesn't have the caret stuff?
> + /* In Firefox it is a common bug to have same types but in
> + different namespaces. Be a bit more informative on
> + this. */
hm? I make no claim to being a spec lawyer, but I thought something like
namespace foo {
struct bar { int x; };
}
namespace baz {
struct bar { float b; };
}
was legal? or are you refering to some other construct?
> + if (TYPE_CONTEXT (t1) && TYPE_CONTEXT (t2)
> + && (((TREE_CODE (TYPE_CONTEXT (t1)) == NAMESPACE_DECL)
> + != (TREE_CODE (TYPE_CONTEXT (t2)) == NAMESPACE_DECL))
> +|| (TREE_CODE (TYPE_CONTEXT (t1)) == NAMESPACE_DECL
> +&& (DECL_NAME (TYPE_CONTEXT (t1)) !=
> +DECL_NAME (TYPE_CONTEXT (t2))
imho that would be a lot more readable with some temporary variables,
but shrug
> +inform (DECL_SOURCE_LOCATION (TYPE_NAME (t1)),
> + "type %qT should match type %qT but is defined "
> + "in different namespace ",
> + t1, t2);
> + else
> +inform (DECL_SOURCE_LOCATION (TYPE_NAME (t1)),
> + "type %qT should match type %qT",
> + t1, t2);
> + inform (DECL_SOURCE_LOCATION (TYPE_NAME (t2)),
> + "the incompatible type is defined here");
I didn't see any of these come up in the list of warnings at the
beginning of your mail?
> @@ -445,31 +955,23 @@ add_type_duplicate (odr_type val, tree t
> {
>bool merge = true;
>bool base_mismatch = false;
> - bool warned = 0;
>unsigned int i,j;
> + bool warned = 0;
false?
Trev
Re: More informative ODR warnings
>
> I can't find the code for the SampleFormat thing, but the rest of them
> look like ODR violations to me.
I think it is some define renaming the field, I was also puzled by this one.
>
> > /aux/hubicka/firefox/netwerk/sctp/datachannel/DataChannel.h:64:0: warning:
> > field ‘mSpa’ (of type ‘struct BufferedMsg’) violates one definition rule
> > [-Wodr]
> >struct sctp_sendv_spa *mSpa;
> > ^
> > ../../../../dist/include/mozilla/net/DataChannel.h:64:0: note: a field of
> > same name but different type is defined in another translation unit
> > /aux/hubicka/firefox/netwerk/streamconv/converters/nsMultiMixedConv.cpp:466:0:
> > warning: field ‘mBuffer’ (of type ‘struct AutoFree’) violates one
> > definition rule [-Wodr]
> >char *mBuffer;
> > ^
> > /aux/hubicka/firefox/dom/base/nsJSEnvironment.cpp:307:0: note: a field with
> > different name is defined in another translation unit
>
> it seems to me this doesn't get at the real issue that the type names
> are the same but the fields are different. maybe "a type of the same
> name with different fields defined here"?
This is what I print when I see name mismatch. It usually means completely
different structure/field.
It speaks of fields names rather than type names. Better wording is welcome.
>
> >void *mPtr;
> > ^
> > lto1: note: Conflicting compilation units:
> > /aux/hubicka/firefox/dom/base/nsJSEnvironment.cpp and
> > /aux/hubicka/build-firefox491-inst4/netwerk/streamconv/converters/Unified_cpp_netwerk_streamconv_converters0.cpp
> > ../../dist/include/zipstruct.h:47:25: warning: field ‘MOZ_Z_crc32’ (of type
> > ‘struct ZipCentral_’) violates one definition rule [-Wodr]
> > ../../dist/include/zipstruct.h:47:0: note: a field with different name is
> > defined in another translation unit
>
> is it worth poking into why this case doesn't have the caret stuff?
>
> > + /* In Firefox it is a common bug to have same types but in
> > + different namespaces. Be a bit more informative on
> > + this. */
>
> hm? I make no claim to being a spec lawyer, but I thought something like
> namespace foo {
> struct bar { int x; };
> }
>
> namespace baz {
> struct bar { float b; };
> }
> was legal? or are you refering to some other construct?
This is legal as long as you don't define
struct foo {
struct bar a;
}
Where foo is same name but bar is once from foo and once from baz.
There are cases where "struct bar" is in an namespace in one unit, but
it is toplevel in another.
>
> > + if (TYPE_CONTEXT (t1) && TYPE_CONTEXT (t2)
> > + && (((TREE_CODE (TYPE_CONTEXT (t1)) == NAMESPACE_DECL)
> > + != (TREE_CODE (TYPE_CONTEXT (t2)) == NAMESPACE_DECL))
> > + || (TREE_CODE (TYPE_CONTEXT (t1)) == NAMESPACE_DECL
> > + && (DECL_NAME (TYPE_CONTEXT (t1)) !=
> > + DECL_NAME (TYPE_CONTEXT (t2))
>
> imho that would be a lot more readable with some temporary variables,
> but shrug
What? stepping away from old-school GCC writting style???
>
> > +inform (DECL_SOURCE_LOCATION (TYPE_NAME (t1)),
> > + "type %qT should match type %qT but is defined "
> > + "in different namespace ",
> > + t1, t2);
> > + else
> > +inform (DECL_SOURCE_LOCATION (TYPE_NAME (t1)),
> > + "type %qT should match type %qT",
> > + t1, t2);
> > + inform (DECL_SOURCE_LOCATION (TYPE_NAME (t2)),
> > + "the incompatible type is defined here");
>
> I didn't see any of these come up in the list of warnings at the
> beginning of your mail?
I get it here:
../../dist/include/mp4_demuxer/audio_decoder_config.h:112:0: warning: field
�~@~Xsample_format_�~@~Y (of type �~@~Xstruct AudioDecoderConfig�~@~Y) violates
one definition rule [-Wodr]
../../../dist/include/mp4_demuxer/audio_decoder_config.h:112:0: note: a field
of same name but different type is defined in another translation unit
SampleFormat sample_format_;
^
../../dist/include/mp4_demuxer/audio_decoder_config.h:41:0: note: type
�~@~XSampleFormat�~@~Y should match type �~@~XAVSampleFormat�~@~Y
../../../dist/include/mp4_demuxer/audio_decoder_config.h:41:0: note: the
incompatible type is defined here
enum SampleFormat {
Which suggest that autdio_decoder_config.h is sometimes included with define
turing SampleFormat to XSampleFormat
and sometimes to XAVSampleFormat. Not the most readable warning around, but
probably can't do much better.
The carret diagnostics should not get confused by relative names - I was
thinking about turing all locations into absolute names, but
that is probably not coolest thing either.
>
> > @@ -445,31 +955,23 @@ add_type_duplicate (odr_type val, tree t
> > {
> >bool merge = true;
> >bool base_mismatch = false;
> > - bool warned = 0;
> >unsigned int i,j;
> > + bool warned = 0;
>
> false?
Yep.
Thanks!
I will prepare updated patch with all the comments.
Honza
>
> Trev
[GOOGLE] Define libgcov interface for distinguishing -ftest-coverage from -fprofile-generate
The following patch adds support for a new libgcov interface,
__gcov_profiling_enabled, that can be used by applications to
determine whether a binary has been instrumented for test coverage or
profile optimization.
Passes regression tests and manual testing with different options. Ok
for google branches?
Thanks,
Teresa
2014-07-02 Teresa Johnson
Google ref b/15378201.
* gcc/tree-profile.c (gcov_test_coverage_decl): Declare.
(tree_init_instrumentation): Initialize gcov_test_coverage_decl.
* libgcc/libgcov-driver.c (__gcov_dummy_ref7): Define.
* libgcc/libgcov-interface.c (__gcov_profiling_enabled): New function.
Index: gcc/tree-profile.c
===
--- gcc/tree-profile.c (revision 212044)
+++ gcc/tree-profile.c (working copy)
@@ -164,6 +164,9 @@ static GTY(()) tree gcov_sample_counter_decl = NUL
/* extern gcov_unsigned_t __gcov_profile_prefix */
static tree GTY(()) gcov_profile_prefix_decl = NULL_TREE;
+/* extern gcov_unsigned_t __gcov_test_coverage */
+static tree GTY(()) gcov_test_coverage_decl = NULL_TREE;
+
/* extern gcov_unsigned_t __gcov_sampling_period */
static GTY(()) tree gcov_sampling_period_decl = NULL_TREE;
@@ -498,6 +501,27 @@ tree_init_instrumentation (void)
DECL_INITIAL (gcov_profile_prefix_decl) = prefix_ptr;
varpool_finalize_decl (gcov_profile_prefix_decl);
}
+
+ if (!gcov_test_coverage_decl)
+{
+ /* Initialize __gcov_test_coverage to 1 if -ftest-coverage
+ specified, 0 otherwise. Used by libgcov to determine whether
+ a binary was instrumented for coverage or profile optimization. */
+ gcov_test_coverage_decl = build_decl (
+ UNKNOWN_LOCATION,
+ VAR_DECL,
+ get_identifier ("__gcov_test_coverage"),
+ get_gcov_unsigned_t ());
+ TREE_PUBLIC (gcov_test_coverage_decl) = 1;
+ DECL_ARTIFICIAL (gcov_test_coverage_decl) = 1;
+ DECL_COMDAT_GROUP (gcov_test_coverage_decl)
+ = DECL_ASSEMBLER_NAME (gcov_test_coverage_decl);
+ TREE_STATIC (gcov_test_coverage_decl) = 1;
+ DECL_INITIAL (gcov_test_coverage_decl) = build_int_cst (
+ get_gcov_unsigned_t (),
+ flag_test_coverage ? 1 : 0);
+ varpool_finalize_decl (gcov_test_coverage_decl);
+}
}
/* Initialization function for FDO sampling. */
Index: libgcc/libgcov-driver.c
===
--- libgcc/libgcov-driver.c (revision 212044)
+++ libgcc/libgcov-driver.c (working copy)
@@ -79,6 +79,8 @@ extern unsigned int __gcov_sampling_enabled (void)
char *(*__gcov_dummy_ref5)(void) = &__gcov_sampling_enabled;
extern void __gcov_flush (void);
char *(*__gcov_dummy_ref6)(void) = &__gcov_flush;
+extern unsigned int __gcov_profiling_enabled (void);
+char *(*__gcov_dummy_ref7)(void) = &__gcov_profiling_enabled;
/* Default callback function for profile instrumentation callback. */
extern void __coverage_callback (gcov_type, int);
Index: libgcc/libgcov-interface.c
===
--- libgcc/libgcov-interface.c (revision 212044)
+++ libgcc/libgcov-interface.c (working copy)
@@ -116,6 +116,20 @@ __gcov_dump (void)
set_gcov_dump_complete ();
}
+/* Emitted in coverage.c. */
+extern gcov_unsigned_t __gcov_test_coverage;
+
+unsigned int __gcov_profiling_enabled (void);
+
+/* Function that can be called from application to distinguish binaries
+ instrumented from coverage fro those instrumented for profiling
+ (e.g. -fprofile-generate). */
+
+unsigned int __gcov_profiling_enabled (void)
+{
+ return !__gcov_test_coverage;
+}
+
#endif /* L_gcov_dump */
#ifdef L_gcov_sampling
--
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
Re: [GOOGLE] Define libgcov interface for distinguishing -ftest-coverage from -fprofile-generate
Should the interface be something like:
int __gcov_profiling_for_test_coverage(void)?
David
On Wed, Jul 2, 2014 at 12:55 PM, Teresa Johnson wrote:
> The following patch adds support for a new libgcov interface,
> __gcov_profiling_enabled, that can be used by applications to
> determine whether a binary has been instrumented for test coverage or
> profile optimization.
>
> Passes regression tests and manual testing with different options. Ok
> for google branches?
>
> Thanks,
> Teresa
>
> 2014-07-02 Teresa Johnson
>
> Google ref b/15378201.
> * gcc/tree-profile.c (gcov_test_coverage_decl): Declare.
> (tree_init_instrumentation): Initialize gcov_test_coverage_decl.
> * libgcc/libgcov-driver.c (__gcov_dummy_ref7): Define.
> * libgcc/libgcov-interface.c (__gcov_profiling_enabled): New function.
>
> Index: gcc/tree-profile.c
> ===
> --- gcc/tree-profile.c (revision 212044)
> +++ gcc/tree-profile.c (working copy)
> @@ -164,6 +164,9 @@ static GTY(()) tree gcov_sample_counter_decl = NUL
> /* extern gcov_unsigned_t __gcov_profile_prefix */
> static tree GTY(()) gcov_profile_prefix_decl = NULL_TREE;
>
> +/* extern gcov_unsigned_t __gcov_test_coverage */
> +static tree GTY(()) gcov_test_coverage_decl = NULL_TREE;
> +
> /* extern gcov_unsigned_t __gcov_sampling_period */
> static GTY(()) tree gcov_sampling_period_decl = NULL_TREE;
>
> @@ -498,6 +501,27 @@ tree_init_instrumentation (void)
>DECL_INITIAL (gcov_profile_prefix_decl) = prefix_ptr;
>varpool_finalize_decl (gcov_profile_prefix_decl);
> }
> +
> + if (!gcov_test_coverage_decl)
> +{
> + /* Initialize __gcov_test_coverage to 1 if -ftest-coverage
> + specified, 0 otherwise. Used by libgcov to determine whether
> + a binary was instrumented for coverage or profile optimization. */
> + gcov_test_coverage_decl = build_decl (
> + UNKNOWN_LOCATION,
> + VAR_DECL,
> + get_identifier ("__gcov_test_coverage"),
> + get_gcov_unsigned_t ());
> + TREE_PUBLIC (gcov_test_coverage_decl) = 1;
> + DECL_ARTIFICIAL (gcov_test_coverage_decl) = 1;
> + DECL_COMDAT_GROUP (gcov_test_coverage_decl)
> + = DECL_ASSEMBLER_NAME (gcov_test_coverage_decl);
> + TREE_STATIC (gcov_test_coverage_decl) = 1;
> + DECL_INITIAL (gcov_test_coverage_decl) = build_int_cst (
> + get_gcov_unsigned_t (),
> + flag_test_coverage ? 1 : 0);
> + varpool_finalize_decl (gcov_test_coverage_decl);
> +}
> }
>
> /* Initialization function for FDO sampling. */
> Index: libgcc/libgcov-driver.c
> ===
> --- libgcc/libgcov-driver.c (revision 212044)
> +++ libgcc/libgcov-driver.c (working copy)
> @@ -79,6 +79,8 @@ extern unsigned int __gcov_sampling_enabled (void)
> char *(*__gcov_dummy_ref5)(void) = &__gcov_sampling_enabled;
> extern void __gcov_flush (void);
> char *(*__gcov_dummy_ref6)(void) = &__gcov_flush;
> +extern unsigned int __gcov_profiling_enabled (void);
> +char *(*__gcov_dummy_ref7)(void) = &__gcov_profiling_enabled;
>
> /* Default callback function for profile instrumentation callback. */
> extern void __coverage_callback (gcov_type, int);
> Index: libgcc/libgcov-interface.c
> ===
> --- libgcc/libgcov-interface.c (revision 212044)
> +++ libgcc/libgcov-interface.c (working copy)
> @@ -116,6 +116,20 @@ __gcov_dump (void)
>set_gcov_dump_complete ();
> }
>
> +/* Emitted in coverage.c. */
> +extern gcov_unsigned_t __gcov_test_coverage;
> +
> +unsigned int __gcov_profiling_enabled (void);
> +
> +/* Function that can be called from application to distinguish binaries
> + instrumented from coverage fro those instrumented for profiling
> + (e.g. -fprofile-generate). */
> +
> +unsigned int __gcov_profiling_enabled (void)
> +{
> + return !__gcov_test_coverage;
> +}
> +
> #endif /* L_gcov_dump */
>
> #ifdef L_gcov_sampling
>
>
> --
> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
Strenghten assumption about dynamic type changes (placement new)
Hi,
this patch strengthens detect_type_change and makes it cheaper based on the
following observation.
We propagate types from places we know instances are created across pointers
passed to functions. Once non-POD type is created at a given memory location,
one can not change its type by placement_new into something else.
This makes it easier to check if types may be altered from function start to
a given call site (information needed to produce jump function) by checking
whether function in question is a constructor or destructor and the parameter
is THIS pointer. In other cases we can not retype anymore.
Jason, this assumes that one can not destroy the type and re-construct same
type at the same spot. Is this valid for THIS pointer of normal methods and is
this valid in general? If so, we will need to walk inline stack of the call
statement and watch for inlined constructor, as we may be within the
destructing/reconstructing sequence. This is not hard to do and I want to do it
anyway to disprove the fact htat local variable may be in
construction/destruction - that can be true only for calls that are within the
inlined constructor/destructor.
Bootstrapped/regtested x86_64-linux, will commit it if the above question is
resolved.
Honza
* ipa-prop.c (param_type_may_change_p): New function.
(detect_type_change_from_memory_writes): Break out from
(detect_type_change): ... here; use param_type_may_change_p.
(detect_type_change_ssa): Use param_type_may_change_p.
Index: ipa-prop.c
===
--- ipa-prop.c (revision 212234)
+++ ipa-prop.c (working copy)
@@ -731,18 +731,45 @@ check_stmt_for_type_change (ao_ref *ao A
return false;
}
+/* See if ARG is PARAM_DECl describing instance passed by pointer
+ or reference in FUNCTION. Return false if the dynamic type may not change.
+ Generally functions are not allowed to change type of such instances,
+ only excetions are C++ constructors and destructors. */
+
+static bool
+param_type_may_change_p (tree function, tree arg)
+{
+ if (TREE_CODE (arg) == SSA_NAME
+ && SSA_NAME_IS_DEFAULT_DEF (arg)
+ && TREE_CODE (SSA_NAME_VAR (arg)) == PARM_DECL)
+{
+ if (TREE_CODE (TREE_TYPE (function)) == METHOD_TYPE
+ && !DECL_CXX_CONSTRUCTOR_P (function)
+ && !DECL_CXX_DESTRUCTOR_P (function)
+ && (SSA_NAME_VAR (arg) == DECL_ARGUMENTS (function)))
+ return false;
+
+ if (SSA_NAME_VAR (arg) != DECL_ARGUMENTS (function))
+ return false;
+}
+ return true;
+}
/* Detect whether the dynamic type of ARG of COMP_TYPE has changed (before
callsite CALL) by looking for assignments to its virtual table pointer. If
it is, return true and fill in the jump function JFUNC with relevant type
information or set it to unknown. ARG is the object itself (not a pointer
to it, unless dereferenced). BASE is the base of the memory access as
- returned by get_ref_base_and_extent, as is the offset. */
+ returned by get_ref_base_and_extent, as is the offset.
+
+ This is helper function for detect_type_change and detect_type_change_ssa
+ that does the heavy work which is usually unnecesary. */
static bool
-detect_type_change (tree arg, tree base, tree comp_type, gimple call,
- struct ipa_jump_func *jfunc, HOST_WIDE_INT offset)
+detect_type_change_from_memory_writes (tree arg, tree base, tree comp_type,
+ gimple call, struct ipa_jump_func *jfunc,
+ HOST_WIDE_INT offset)
{
struct type_change_info tci;
ao_ref ao;
@@ -753,25 +780,6 @@ detect_type_change (tree arg, tree base,
comp_type = TYPE_MAIN_VARIANT (comp_type);
- if (!flag_devirtualize)
-return false;
-
- /* C++ methods are not allowed to change THIS pointer unless they
- are constructors or destructors. */
- if (TREE_CODE(base) == MEM_REF
- && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME
- && SSA_NAME_IS_DEFAULT_DEF (TREE_OPERAND (base, 0))
- && TREE_CODE (SSA_NAME_VAR (TREE_OPERAND (base, 0))) == PARM_DECL
- && TREE_CODE (TREE_TYPE (current_function_decl)) == METHOD_TYPE
- && !DECL_CXX_CONSTRUCTOR_P (current_function_decl)
- && !DECL_CXX_DESTRUCTOR_P (current_function_decl)
- && (SSA_NAME_VAR (TREE_OPERAND (base, 0))
- == DECL_ARGUMENTS (current_function_decl)))
-{
- gcc_assert (comp_type);
- return false;
-}
-
/* Const calls cannot call virtual methods through VMT and so type changes do
not matter. */
if (!flag_devirtualize || !gimple_vuse (call)
@@ -809,6 +817,27 @@ detect_type_change (tree arg, tree base,
return true;
}
+/* Detect whether the dynamic type of ARG of COMP_TYPE may have changed.
+ If it is, return true and fill in the jump function JFUNC with relevant type
+ information or set it to unknown. ARG is the object its
Re: [PATCH] Only transform rotate to rotatert and v.v. if target has both
> OK. Thanks, committed. > It would be useful if you could add one or more tests to the > testsuite to confirm proper behaviour when only one of ROTATE/ROTATERT > is defined. I'll send a patch in a minute that tests this (as well as some other things) for rs6000. Segher
Re: [GOOGLE] Define libgcov interface for distinguishing -ftest-coverage from -fprofile-generate
On Wed, Jul 2, 2014 at 1:15 PM, Xinliang David Li wrote:
> Should the interface be something like:
>
> int __gcov_profiling_for_test_coverage(void)?
I was equating the term "profiling" with -fprofile-generate, as
opposed to -ftest-coverage instrumentation. But I can change it to
this if you think that is clearer.
Teresa
>
> David
>
>
> On Wed, Jul 2, 2014 at 12:55 PM, Teresa Johnson wrote:
>> The following patch adds support for a new libgcov interface,
>> __gcov_profiling_enabled, that can be used by applications to
>> determine whether a binary has been instrumented for test coverage or
>> profile optimization.
>>
>> Passes regression tests and manual testing with different options. Ok
>> for google branches?
>>
>> Thanks,
>> Teresa
>>
>> 2014-07-02 Teresa Johnson
>>
>> Google ref b/15378201.
>> * gcc/tree-profile.c (gcov_test_coverage_decl): Declare.
>> (tree_init_instrumentation): Initialize gcov_test_coverage_decl.
>> * libgcc/libgcov-driver.c (__gcov_dummy_ref7): Define.
>> * libgcc/libgcov-interface.c (__gcov_profiling_enabled): New
>> function.
>>
>> Index: gcc/tree-profile.c
>> ===
>> --- gcc/tree-profile.c (revision 212044)
>> +++ gcc/tree-profile.c (working copy)
>> @@ -164,6 +164,9 @@ static GTY(()) tree gcov_sample_counter_decl = NUL
>> /* extern gcov_unsigned_t __gcov_profile_prefix */
>> static tree GTY(()) gcov_profile_prefix_decl = NULL_TREE;
>>
>> +/* extern gcov_unsigned_t __gcov_test_coverage */
>> +static tree GTY(()) gcov_test_coverage_decl = NULL_TREE;
>> +
>> /* extern gcov_unsigned_t __gcov_sampling_period */
>> static GTY(()) tree gcov_sampling_period_decl = NULL_TREE;
>>
>> @@ -498,6 +501,27 @@ tree_init_instrumentation (void)
>>DECL_INITIAL (gcov_profile_prefix_decl) = prefix_ptr;
>>varpool_finalize_decl (gcov_profile_prefix_decl);
>> }
>> +
>> + if (!gcov_test_coverage_decl)
>> +{
>> + /* Initialize __gcov_test_coverage to 1 if -ftest-coverage
>> + specified, 0 otherwise. Used by libgcov to determine whether
>> + a binary was instrumented for coverage or profile optimization. */
>> + gcov_test_coverage_decl = build_decl (
>> + UNKNOWN_LOCATION,
>> + VAR_DECL,
>> + get_identifier ("__gcov_test_coverage"),
>> + get_gcov_unsigned_t ());
>> + TREE_PUBLIC (gcov_test_coverage_decl) = 1;
>> + DECL_ARTIFICIAL (gcov_test_coverage_decl) = 1;
>> + DECL_COMDAT_GROUP (gcov_test_coverage_decl)
>> + = DECL_ASSEMBLER_NAME (gcov_test_coverage_decl);
>> + TREE_STATIC (gcov_test_coverage_decl) = 1;
>> + DECL_INITIAL (gcov_test_coverage_decl) = build_int_cst (
>> + get_gcov_unsigned_t (),
>> + flag_test_coverage ? 1 : 0);
>> + varpool_finalize_decl (gcov_test_coverage_decl);
>> +}
>> }
>>
>> /* Initialization function for FDO sampling. */
>> Index: libgcc/libgcov-driver.c
>> ===
>> --- libgcc/libgcov-driver.c (revision 212044)
>> +++ libgcc/libgcov-driver.c (working copy)
>> @@ -79,6 +79,8 @@ extern unsigned int __gcov_sampling_enabled (void)
>> char *(*__gcov_dummy_ref5)(void) = &__gcov_sampling_enabled;
>> extern void __gcov_flush (void);
>> char *(*__gcov_dummy_ref6)(void) = &__gcov_flush;
>> +extern unsigned int __gcov_profiling_enabled (void);
>> +char *(*__gcov_dummy_ref7)(void) = &__gcov_profiling_enabled;
>>
>> /* Default callback function for profile instrumentation callback. */
>> extern void __coverage_callback (gcov_type, int);
>> Index: libgcc/libgcov-interface.c
>> ===
>> --- libgcc/libgcov-interface.c (revision 212044)
>> +++ libgcc/libgcov-interface.c (working copy)
>> @@ -116,6 +116,20 @@ __gcov_dump (void)
>>set_gcov_dump_complete ();
>> }
>>
>> +/* Emitted in coverage.c. */
>> +extern gcov_unsigned_t __gcov_test_coverage;
>> +
>> +unsigned int __gcov_profiling_enabled (void);
>> +
>> +/* Function that can be called from application to distinguish binaries
>> + instrumented from coverage fro those instrumented for profiling
>> + (e.g. -fprofile-generate). */
>> +
>> +unsigned int __gcov_profiling_enabled (void)
>> +{
>> + return !__gcov_test_coverage;
>> +}
>> +
>> #endif /* L_gcov_dump */
>>
>> #ifdef L_gcov_sampling
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
--
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
Re: [GOOGLE] Define libgcov interface for distinguishing -ftest-coverage from -fprofile-generate
The reason is that test coverage is using the same underlying
profiling. Returning false when calling '__gcov_profiling_enabled())
in coverage mode is a little misleading.
David
On Wed, Jul 2, 2014 at 1:22 PM, Teresa Johnson wrote:
> On Wed, Jul 2, 2014 at 1:15 PM, Xinliang David Li wrote:
>> Should the interface be something like:
>>
>> int __gcov_profiling_for_test_coverage(void)?
>
> I was equating the term "profiling" with -fprofile-generate, as
> opposed to -ftest-coverage instrumentation. But I can change it to
> this if you think that is clearer.
>
> Teresa
>
>>
>> David
>>
>>
>> On Wed, Jul 2, 2014 at 12:55 PM, Teresa Johnson wrote:
>>> The following patch adds support for a new libgcov interface,
>>> __gcov_profiling_enabled, that can be used by applications to
>>> determine whether a binary has been instrumented for test coverage or
>>> profile optimization.
>>>
>>> Passes regression tests and manual testing with different options. Ok
>>> for google branches?
>>>
>>> Thanks,
>>> Teresa
>>>
>>> 2014-07-02 Teresa Johnson
>>>
>>> Google ref b/15378201.
>>> * gcc/tree-profile.c (gcov_test_coverage_decl): Declare.
>>> (tree_init_instrumentation): Initialize gcov_test_coverage_decl.
>>> * libgcc/libgcov-driver.c (__gcov_dummy_ref7): Define.
>>> * libgcc/libgcov-interface.c (__gcov_profiling_enabled): New
>>> function.
>>>
>>> Index: gcc/tree-profile.c
>>> ===
>>> --- gcc/tree-profile.c (revision 212044)
>>> +++ gcc/tree-profile.c (working copy)
>>> @@ -164,6 +164,9 @@ static GTY(()) tree gcov_sample_counter_decl = NUL
>>> /* extern gcov_unsigned_t __gcov_profile_prefix */
>>> static tree GTY(()) gcov_profile_prefix_decl = NULL_TREE;
>>>
>>> +/* extern gcov_unsigned_t __gcov_test_coverage */
>>> +static tree GTY(()) gcov_test_coverage_decl = NULL_TREE;
>>> +
>>> /* extern gcov_unsigned_t __gcov_sampling_period */
>>> static GTY(()) tree gcov_sampling_period_decl = NULL_TREE;
>>>
>>> @@ -498,6 +501,27 @@ tree_init_instrumentation (void)
>>>DECL_INITIAL (gcov_profile_prefix_decl) = prefix_ptr;
>>>varpool_finalize_decl (gcov_profile_prefix_decl);
>>> }
>>> +
>>> + if (!gcov_test_coverage_decl)
>>> +{
>>> + /* Initialize __gcov_test_coverage to 1 if -ftest-coverage
>>> + specified, 0 otherwise. Used by libgcov to determine whether
>>> + a binary was instrumented for coverage or profile optimization.
>>> */
>>> + gcov_test_coverage_decl = build_decl (
>>> + UNKNOWN_LOCATION,
>>> + VAR_DECL,
>>> + get_identifier ("__gcov_test_coverage"),
>>> + get_gcov_unsigned_t ());
>>> + TREE_PUBLIC (gcov_test_coverage_decl) = 1;
>>> + DECL_ARTIFICIAL (gcov_test_coverage_decl) = 1;
>>> + DECL_COMDAT_GROUP (gcov_test_coverage_decl)
>>> + = DECL_ASSEMBLER_NAME (gcov_test_coverage_decl);
>>> + TREE_STATIC (gcov_test_coverage_decl) = 1;
>>> + DECL_INITIAL (gcov_test_coverage_decl) = build_int_cst (
>>> + get_gcov_unsigned_t (),
>>> + flag_test_coverage ? 1 : 0);
>>> + varpool_finalize_decl (gcov_test_coverage_decl);
>>> +}
>>> }
>>>
>>> /* Initialization function for FDO sampling. */
>>> Index: libgcc/libgcov-driver.c
>>> ===
>>> --- libgcc/libgcov-driver.c (revision 212044)
>>> +++ libgcc/libgcov-driver.c (working copy)
>>> @@ -79,6 +79,8 @@ extern unsigned int __gcov_sampling_enabled (void)
>>> char *(*__gcov_dummy_ref5)(void) = &__gcov_sampling_enabled;
>>> extern void __gcov_flush (void);
>>> char *(*__gcov_dummy_ref6)(void) = &__gcov_flush;
>>> +extern unsigned int __gcov_profiling_enabled (void);
>>> +char *(*__gcov_dummy_ref7)(void) = &__gcov_profiling_enabled;
>>>
>>> /* Default callback function for profile instrumentation callback. */
>>> extern void __coverage_callback (gcov_type, int);
>>> Index: libgcc/libgcov-interface.c
>>> ===
>>> --- libgcc/libgcov-interface.c (revision 212044)
>>> +++ libgcc/libgcov-interface.c (working copy)
>>> @@ -116,6 +116,20 @@ __gcov_dump (void)
>>>set_gcov_dump_complete ();
>>> }
>>>
>>> +/* Emitted in coverage.c. */
>>> +extern gcov_unsigned_t __gcov_test_coverage;
>>> +
>>> +unsigned int __gcov_profiling_enabled (void);
>>> +
>>> +/* Function that can be called from application to distinguish binaries
>>> + instrumented from coverage fro those instrumented for profiling
>>> + (e.g. -fprofile-generate). */
>>> +
>>> +unsigned int __gcov_profiling_enabled (void)
>>> +{
>>> + return !__gcov_test_coverage;
>>> +}
>>> +
>>> #endif /* L_gcov_dump */
>>>
>>> #ifdef L_gcov_sampling
>>>
>>>
>>> --
>>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
[PATCH] rs6000: Fix the shift patterns, and add test
Firstly, it adds back the split conditions that I accidentally removed. Without it the dot insns are never generated, or rather, always split back to a separate compare instruction. Secondly, the shift amount should be SI always, not GPR, or GCC will insert a zero-extend at expand time that it cannot get rid of later. The test tests whether dot-form instructions are generated for both "dot" and "dot2" cases, that is, with just a CC output or also a GPR output; for all four basic shifts, with a register amount or an immediate amount. It also tests for superfluous zero-extends. This also tests if combine "simplifies" the rotates to right-rotates, which it shouldn't do anymore. Bootstrapped and tested as usual. Okay to commit? Segher 2014-07-02 Segher Boessenkool gcc/ * config/rs6000/rs6000.md (rotl3, ashl3, lshr3, ashr3): Correct mode of operands[2]. (rotl3_dot, rotl3_dot2, ashl3_dot, ashl3_dot2, lshr3_dot, lshr3_dot2, ashr3_dot, ashr3_dot2): Correct mode of operands[2]. Fix split condition. gcc/testsuite/ * gcc.target/powerpc/shift-dot.c: New test. --- gcc/config/rs6000/rs6000.md | 42 gcc/testsuite/gcc.target/powerpc/shift-dot.c | 49 2 files changed, 70 insertions(+), 21 deletions(-) create mode 100644 gcc/testsuite/gcc.target/powerpc/shift-dot.c diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index 8e9039e..83aaa58 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -3874,7 +3874,7 @@ (define_insn "*extzvdi_internal2" (define_insn "rotl3" [(set (match_operand:GPR 0 "gpc_reg_operand" "=r") (rotate:GPR (match_operand:GPR 1 "gpc_reg_operand" "r") - (match_operand:GPR 2 "reg_or_cint_operand" "rn")))] + (match_operand:SI 2 "reg_or_cint_operand" "rn")))] "" "rotl%I2 %0,%1,%2" [(set_attr "type" "shift") @@ -3893,14 +3893,14 @@ (define_insn "*rotlsi3_64" (define_insn_and_split "*rotl3_dot" [(set (match_operand:CC 3 "cc_reg_operand" "=x,?y") (compare:CC (rotate:GPR (match_operand:GPR 1 "gpc_reg_operand" "r,r") - (match_operand:GPR 2 "reg_or_cint_operand" "rn,rn")) + (match_operand:SI 2 "reg_or_cint_operand" "rn,rn")) (const_int 0))) (clobber (match_scratch:GPR 0 "=r,r"))] "mode == Pmode && rs6000_gen_cell_microcode" "@ rotl%I2. %0,%1,%2 #" - "&& reload_completed" + "&& reload_completed && cc_reg_not_cr0_operand (operands[3], CCmode)" [(set (match_dup 0) (rotate:GPR (match_dup 1) (match_dup 2))) @@ -3916,7 +3916,7 @@ (define_insn_and_split "*rotl3_dot" (define_insn_and_split "*rotl3_dot2" [(set (match_operand:CC 3 "cc_reg_operand" "=x,?y") (compare:CC (rotate:GPR (match_operand:GPR 1 "gpc_reg_operand" "r,r") - (match_operand:GPR 2 "reg_or_cint_operand" "rn,rn")) + (match_operand:SI 2 "reg_or_cint_operand" "rn,rn")) (const_int 0))) (set (match_operand:GPR 0 "gpc_reg_operand" "=r,r") (rotate:GPR (match_dup 1) @@ -3925,7 +3925,7 @@ (define_insn_and_split "*rotl3_dot2" "@ rotl%I2. %0,%1,%2 #" - "&& reload_completed" + "&& reload_completed && cc_reg_not_cr0_operand (operands[3], CCmode)" [(set (match_dup 0) (rotate:GPR (match_dup 1) (match_dup 2))) @@ -4353,7 +4353,7 @@ (define_split (define_insn "ashl3" [(set (match_operand:GPR 0 "gpc_reg_operand" "=r") (ashift:GPR (match_operand:GPR 1 "gpc_reg_operand" "r") - (match_operand:GPR 2 "reg_or_cint_operand" "rn")))] + (match_operand:SI 2 "reg_or_cint_operand" "rn")))] "" "sl%I2 %0,%1,%2" [(set_attr "type" "shift") @@ -4372,14 +4372,14 @@ (define_insn "*ashlsi3_64" (define_insn_and_split "*ashl3_dot" [(set (match_operand:CC 3 "cc_reg_operand" "=x,?y") (compare:CC (ashift:GPR (match_operand:GPR 1 "gpc_reg_operand" "r,r") - (match_operand:GPR 2 "reg_or_cint_operand" "rn,rn")) + (match_operand:SI 2 "reg_or_cint_operand" "rn,rn")) (const_int 0))) (clobber (match_scratch:GPR 0 "=r,r"))] "mode == Pmode && rs6000_gen_cell_microcode" "@ sl%I2. %0,%1,%2 #" - "&& reload_completed" + "&& reload_completed && cc_reg_not_cr0_operand (operands[3], CCmode)" [(set (match_dup 0) (ashift:GPR (match_dup 1) (match_dup 2))) @@ -4395,7 +4395,7 @@ (define_insn_and_split "*ashl3_dot" (define_insn_and_split "*ashl3_dot2" [(set (match_operand:CC 3 "cc_reg_operand" "=x,?y") (compare:CC (ashift:GPR (match_operand:GPR 1 "gpc_reg_operand" "r,r") - (match_operand:GPR 2 "reg_or_cint_operand" "rn,rn")) + (ma
[PATCH] Memory leak in parallel/unique_copy
Hi, In parallel/unique_copy.h __counter is never deleted. I'm also trying to follow from other posts how to submit a patch but is well possible I missed some of the conventions. Many apologies if that's the case. libstdc++-v3/ * include/parallel/unique_copy.h: prevent memory leak of __counter Index: libstdc++-v3/include/parallel/unique_copy.h === --- libstdc++-v3/include/parallel/unique_copy.h (revision 212239) +++ libstdc++-v3/include/parallel/unique_copy.h (working copy) @@ -171,6 +171,7 @@ for (_ThreadIndex __t = 0; __t < __num_threads + 1; __t++) __end_output += __counter[__t]; + delete[] __counter; delete[] __borders; return __result + __end_output;
Re: More informative ODR warnings
On Wed, Jul 02, 2014 at 09:28:03PM +0200, Jan Hubicka wrote:
> >
> > I can't find the code for the SampleFormat thing, but the rest of them
> > look like ODR violations to me.
>
> I think it is some define renaming the field, I was also puzled by this one.
> >
> > > /aux/hubicka/firefox/netwerk/sctp/datachannel/DataChannel.h:64:0:
> > > warning: field ‘mSpa’ (of type ‘struct BufferedMsg’) violates one
> > > definition rule [-Wodr]
> > >struct sctp_sendv_spa *mSpa;
> > > ^
> > > ../../../../dist/include/mozilla/net/DataChannel.h:64:0: note: a field of
> > > same name but different type is defined in another translation unit
> > > /aux/hubicka/firefox/netwerk/streamconv/converters/nsMultiMixedConv.cpp:466:0:
> > > warning: field ‘mBuffer’ (of type ‘struct AutoFree’) violates one
> > > definition rule [-Wodr]
> > >char *mBuffer;
> > > ^
> > > /aux/hubicka/firefox/dom/base/nsJSEnvironment.cpp:307:0: note: a field
> > > with different name is defined in another translation unit
> >
> > it seems to me this doesn't get at the real issue that the type names
> > are the same but the fields are different. maybe "a type of the same
> > name with different fields defined here"?
>
> This is what I print when I see name mismatch. It usually means completely
> different structure/field.
> It speaks of fields names rather than type names. Better wording is welcome.
I was sort of suggesting that part I quoted, but its not great either.
maybe
note: First differing member of $whatever defined here
?
> > >void *mPtr;
> > > ^
> > > lto1: note: Conflicting compilation units:
> > > /aux/hubicka/firefox/dom/base/nsJSEnvironment.cpp and
> > > /aux/hubicka/build-firefox491-inst4/netwerk/streamconv/converters/Unified_cpp_netwerk_streamconv_converters0.cpp
> > > ../../dist/include/zipstruct.h:47:25: warning: field ‘MOZ_Z_crc32’ (of
> > > type ‘struct ZipCentral_’) violates one definition rule [-Wodr]
> > > ../../dist/include/zipstruct.h:47:0: note: a field with different name is
> > > defined in another translation unit
> >
> > is it worth poking into why this case doesn't have the caret stuff?
> >
> > > + /* In Firefox it is a common bug to have same types but in
> > > + different namespaces. Be a bit more informative on
> > > + this. */
> >
> > hm? I make no claim to being a spec lawyer, but I thought something like
> > namespace foo {
> > struct bar { int x; };
> > }
> >
> > namespace baz {
> > struct bar { float b; };
> > }
> > was legal? or are you refering to some other construct?
>
> This is legal as long as you don't define
>
> struct foo {
> struct bar a;
> }
>
> Where foo is same name but bar is once from foo and once from baz.
h, that seems like a kind of confusing thing to write.
> There are cases where "struct bar" is in an namespace in one unit, but
> it is toplevel in another.
> >
> > > + if (TYPE_CONTEXT (t1) && TYPE_CONTEXT (t2)
> > > + && (((TREE_CODE (TYPE_CONTEXT (t1)) == NAMESPACE_DECL)
> > > + != (TREE_CODE (TYPE_CONTEXT (t2)) == NAMESPACE_DECL))
> > > +|| (TREE_CODE (TYPE_CONTEXT (t1)) == NAMESPACE_DECL
> > > +&& (DECL_NAME (TYPE_CONTEXT (t1)) !=
> > > +DECL_NAME (TYPE_CONTEXT (t2))
> >
> > imho that would be a lot more readable with some temporary variables,
> > but shrug
>
> What? stepping away from old-school GCC writting style???
absolutely ;)
> > > +inform (DECL_SOURCE_LOCATION (TYPE_NAME (t1)),
> > > + "type %qT should match type %qT but is defined "
> > > + "in different namespace ",
> > > + t1, t2);
> > > + else
> > > +inform (DECL_SOURCE_LOCATION (TYPE_NAME (t1)),
> > > + "type %qT should match type %qT",
> > > + t1, t2);
> > > + inform (DECL_SOURCE_LOCATION (TYPE_NAME (t2)),
> > > + "the incompatible type is defined here");
> >
> > I didn't see any of these come up in the list of warnings at the
> > beginning of your mail?
>
> I get it here:
yeah, should have read more carefully.
Trev
> ../../dist/include/mp4_demuxer/audio_decoder_config.h:112:0: warning: field
> �~@~Xsample_format_�~@~Y (of type �~@~Xstruct AudioDecoderConfig�~@~Y)
> violates one definition rule [-Wodr]
> ../../../dist/include/mp4_demuxer/audio_decoder_config.h:112:0: note: a field
> of same name but different type is defined in another translation unit
>SampleFormat sample_format_;
> ^
> ../../dist/include/mp4_demuxer/audio_decoder_config.h:41:0: note: type
> �~@~XSampleFormat�~@~Y should match type �~@~XAVSampleFormat�~@~Y
> ../../../dist/include/mp4_demuxer/audio_decoder_config.h:41:0: note: the
> incompatible type is defined here
> enum SampleFormat {
>
> Which suggest that autdio_decoder_config.h is sometimes included with define
> turing SampleFormat to XSampleFormat
> and sometimes to XAVSampleFormat. Not the most readable warning around, but
> probably can't do much better.
>
> The carret diagnostics should not get confused by relative names - I was
> thi
Re: [PATCH] Fix various undefined behaviors in gcc found by bootstrap-ubsan
On 07/02/14 02:14, Jakub Jelinek wrote: Hi! Most of this probably doesn't need explanation, just the expand_sdiv_pow2 change - shift_cost only tracks shift counts to MAX_BITS_PER_WORD (but, for consistency between e.g. i?86 and x86_64 -m32 we'd better use BITS_PER_WORD instead as in most other expmed.c places), when called with larger values it is out of bounds access. I believe for multiword accesses, even when we don't have costs for it, and masking will be typically cheaper (e.g. for double word you need two word ANDs, while for double word shift unless there is a specialized instruction for double word shifts one needs either a library routine or 3 shifts and IOR). Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2014-07-02 Jakub Jelinek * gcov-io.c (gcov_read_words): Don't call memmove if excess is 0. * data-streamer-in.c (streamer_read_hwi): Shift UHWI 1 instead of HWI 1 and negate the unsigned value. * expmed.c (expand_sdiv_pow2): For modes wider than word always use AND instead of shift. * wide-int-print.cc (print_decs): Negate UHWI instead of HWI. c-family/ * c-ada-spec.c (dump_ada_nodes): Don't call qsort if comments->count <= 1, as comments->entries might be NULL. OK for the trunk. Thanks, Jeff
Re: Warn when returning the address of a temporary (middle-end) v2
On 07/02/14 06:19, Alan Modra wrote: On Mon, Jun 30, 2014 at 11:37:50PM +0200, Marc Glisse wrote: On Mon, 30 Jun 2014, Jeff Law wrote: On 06/29/14 03:22, Marc Glisse wrote: After looking at PR 61597, I updated the 2 conditions to: + if ((TREE_CODE (valbase) == VAR_DECL + && !is_global_var (valbase)) + || TREE_CODE (valbase) == PARM_DECL) a PARM_DECL is a local variable and returning its address is wrong, isn't it? Right. It can live in either a caller or callee allocated slot. The "caller" case scares me a bit. Is it really wrong to return the address in that case? The slot still exists after returning if the caller is responsible for it. At least on powerpc64, which uses a caller allocated parameter save area, returning the address of something in the parameter save area merits a warning. The ABIs explicitly state that the parameter save area is not preserved over function calls. Also note that anything left in a caller allocated parameter save area will potentially be trashed by arguments written for the next call. Similarly for the PA ABIs. If something returned the address of one of those slots, we'd definitely want a warning. I'd be amazed if that wasn't the case for every ABI with a caller allocated parameter save-back area. jeff
Re: Warn when returning the address of a temporary (middle-end) v2
On 06/30/14 15:37, Marc Glisse wrote: On Mon, 30 Jun 2014, Jeff Law wrote: On 06/29/14 03:22, Marc Glisse wrote: After looking at PR 61597, I updated the 2 conditions to: + if ((TREE_CODE (valbase) == VAR_DECL + && !is_global_var (valbase)) + || TREE_CODE (valbase) == PARM_DECL) a PARM_DECL is a local variable and returning its address is wrong, isn't it? Right. It can live in either a caller or callee allocated slot. The "caller" case scares me a bit. Is it really wrong to return the address in that case? The slot still exists after returning if the caller is responsible for it. The slot exists, but its contents are undefined. The caller never even knows if the callee ever flushed an object back to those slots or if it did flush back, which objects were flushed and in what state. There was a time when I was pondering using those slots for saving callee saved registers on the PA so that leafs that needed a few stack slots wouldn't need to allococate a new frame, instead it'd just use those convenient 4 words. But this turned out to just not be important and it would totally hose the unwinding mechanisms that were in use on the PA at the time. Jeff
Re: [PATCH] Don't run guality.exp tests with LTO_TORTURE_OPTIONS.
I think that makes sense; I'm not aware of anyone working on improving LTO debugging. Jason
[PATCH] remove useless unused attributes in i386 code
From: Trevor Saunders
Hi,
So apparently its not entirely obvious to everyone that removing the names of
unused arguments is the best way of dealing with warnings about unused
arguments in places like hooks where the argument is required for some reason.
Apparently this is somewhat less contravercial when the names are removed
consistantly throughout a section of code, so this patch does that for
config/i386/
It should suprise nobody that there was a couple cases where the argument was
used, but still marked attribute unused.
bootstrapped and regtested on x86_64-unknown-linux-gnu, with no regressions,
and a --target=i386-mingw32 run of make all-gcc completes without any unused
warnings. ok for trunk?
Trev
gcc/
* config/i386/driver-i386.c: Remove names of unused arguments and
unnecessary unused attributes.
* config/i386/host-mingw32.c: Likewise.
* config/i386/i386.c: Likewise.
* config/i386/winnt-stubs.c: Likewise.
* config/i386/winnt.c: Likewise.
diff --git a/gcc/config/i386/driver-i386.c b/gcc/config/i386/driver-i386.c
index 4cd0b3d..1c6385f 100644
--- a/gcc/config/i386/driver-i386.c
+++ b/gcc/config/i386/driver-i386.c
@@ -920,8 +920,7 @@ done:
-march and -mtune "native" target and will leave to the newly
built compiler to generate code for its default target. */
-const char *host_detect_local_cpu (int argc ATTRIBUTE_UNUSED,
- const char **argv ATTRIBUTE_UNUSED)
+const char *host_detect_local_cpu (int, const char **)
{
return NULL;
}
diff --git a/gcc/config/i386/host-mingw32.c b/gcc/config/i386/host-mingw32.c
index fc01ceb..c71d25d 100644
--- a/gcc/config/i386/host-mingw32.c
+++ b/gcc/config/i386/host-mingw32.c
@@ -83,7 +83,7 @@ mingw32_gt_pch_alloc_granularity (void)
open file descriptor if the host would like to probe with mmap. */
static void *
-mingw32_gt_pch_get_address (size_t size, int fd ATTRIBUTE_UNUSED)
+mingw32_gt_pch_get_address (size_t size, int)
{
void* res;
size = (size + va_granularity - 1) & ~(va_granularity - 1);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 8046c67..01d3f2c 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -5226,9 +5226,8 @@ x86_elf_aligned_common (FILE *file,
ASM_OUTPUT_ALIGNED_BSS. */
void
-x86_output_aligned_bss (FILE *file, tree decl ATTRIBUTE_UNUSED,
- const char *name, unsigned HOST_WIDE_INT size,
- int align)
+x86_output_aligned_bss (FILE *file, tree decl, const char *name,
+ unsigned HOST_WIDE_INT size, int align)
{
if ((ix86_cmodel == CM_MEDIUM || ix86_cmodel == CM_MEDIUM_PIC)
&& size > (unsigned int)ix86_section_threshold)
@@ -5358,7 +5357,7 @@ ix86_function_ok_for_sibcall (tree decl, tree exp)
static tree
ix86_handle_cconv_attribute (tree *node, tree name,
tree args,
- int flags ATTRIBUTE_UNUSED,
+ int,
bool *no_add_attrs)
{
if (TREE_CODE (*node) != FUNCTION_TYPE
@@ -5502,8 +5501,7 @@ ix86_handle_cconv_attribute (tree *node, tree name,
attributes that we expect elsewhere. */
static tree
-ix86_handle_tm_regparm_attribute (tree *node, tree name ATTRIBUTE_UNUSED,
- tree args ATTRIBUTE_UNUSED,
+ix86_handle_tm_regparm_attribute (tree *node, tree, tree,
int flags, bool *no_add_attrs)
{
tree alt;
@@ -7542,7 +7540,7 @@ ix86_function_arg (cumulative_args_t cum_v, enum
machine_mode omode,
static bool
ix86_pass_by_reference (cumulative_args_t cum_v, enum machine_mode mode,
- const_tree type, bool named ATTRIBUTE_UNUSED)
+ const_tree type, bool)
{
CUMULATIVE_ARGS *cum = get_cumulative_args (cum_v);
@@ -7971,8 +7969,7 @@ ix86_function_value_1 (const_tree valtype, const_tree
fntype_or_decl,
}
static rtx
-ix86_function_value (const_tree valtype, const_tree fntype_or_decl,
-bool outgoing ATTRIBUTE_UNUSED)
+ix86_function_value (const_tree valtype, const_tree fntype_or_decl, bool)
{
enum machine_mode mode, orig_mode;
@@ -8019,7 +8016,7 @@ ix86_libcall_value (enum machine_mode mode)
/* Return true iff type is returned in memory. */
static bool
-ix86_return_in_memory (const_tree type, const_tree fntype ATTRIBUTE_UNUSED)
+ix86_return_in_memory (const_tree type, const_tree fntype)
{
#ifdef SUBTARGET_RETURN_IN_MEMORY
return SUBTARGET_RETURN_IN_MEMORY (type, fntype);
@@ -8313,8 +8310,7 @@ setup_incoming_varargs_ms_64 (CUMULATIVE_ARGS *cum)
static void
ix86_setup_incoming_varargs (cumulative_args_t cum_v, enum machine_mode mode,
-tree type, int *pretend_size ATTRIBUTE_UNUSED,
-int no_rtl)
+tree type, int *, int no_rtl)
{
CUMULATIVE_ARGS *cum =
Re: More informative ODR warnings
On 07/02/2014 03:34 PM, Trevor Saunders wrote: On Wed, Jul 02, 2014 at 09:28:03PM +0200, Jan Hubicka wrote: it seems to me this doesn't get at the real issue that the type names are the same but the fields are different. maybe "a type of the same name with different fields defined here"? This is what I print when I see name mismatch. It usually means completely different structure/field. It speaks of fields names rather than type names. Better wording is welcome. I was sort of suggesting that part I quoted, but its not great either. maybe note: First differing member of $whatever defined here ? I think that would be better. The problem is that the different BufferedMsg definitions are not equivalent by the ODR rules, so the primary error should be about BufferedMsg, followed by information about the differing data members. Jason
Re: Strenghten assumption about dynamic type changes (placement new)
On 07/02/2014 01:18 PM, Jan Hubicka wrote: We propagate types from places we know instances are created across pointers passed to functions. Once non-POD type is created at a given memory location, one can not change its type by placement_new into something else. Hmm. If the memory location is untyped (i.e. from malloc) or a character array, or a union, you can indeed destroy an object of one type and create an object of a different type in that location. Jason, this assumes that one can not destroy the type and re-construct same type at the same spot. That is an invalid assumption; you can destroy one object and construct a new one in the same location. Doing it within a method would be unusual, but I don't think there's a rule against it. Jason
Re: [GOOGLE] Define libgcov interface for distinguishing -ftest-coverage from -fprofile-generate
New patch below. Retested. Ok for google branches?
Thanks,
Teresa
2014-07-02 Teresa Johnson
Google ref b/15378201.
* gcc/tree-profile.c (gcov_test_coverage_decl): Declare.
(tree_init_instrumentation): Initialize gcov_test_coverage_decl.
* libgcc/libgcov-driver.c (__gcov_dummy_ref7): Define.
* libgcc/libgcov-interface.c (__gcov_profiling_for_test_coverage): New
function.
Index: gcc/tree-profile.c
===
--- gcc/tree-profile.c (revision 212044)
+++ gcc/tree-profile.c (working copy)
@@ -164,6 +164,9 @@ static GTY(()) tree gcov_sample_counter_decl = NUL
/* extern gcov_unsigned_t __gcov_profile_prefix */
static tree GTY(()) gcov_profile_prefix_decl = NULL_TREE;
+/* extern gcov_unsigned_t __gcov_test_coverage */
+static tree GTY(()) gcov_test_coverage_decl = NULL_TREE;
+
/* extern gcov_unsigned_t __gcov_sampling_period */
static GTY(()) tree gcov_sampling_period_decl = NULL_TREE;
@@ -498,6 +501,27 @@ tree_init_instrumentation (void)
DECL_INITIAL (gcov_profile_prefix_decl) = prefix_ptr;
varpool_finalize_decl (gcov_profile_prefix_decl);
}
+
+ if (!gcov_test_coverage_decl)
+{
+ /* Initialize __gcov_test_coverage to 1 if -ftest-coverage
+ specified, 0 otherwise. Used by libgcov to determine whether
+ a binary was instrumented for coverage or profile optimization. */
+ gcov_test_coverage_decl = build_decl (
+ UNKNOWN_LOCATION,
+ VAR_DECL,
+ get_identifier ("__gcov_test_coverage"),
+ get_gcov_unsigned_t ());
+ TREE_PUBLIC (gcov_test_coverage_decl) = 1;
+ DECL_ARTIFICIAL (gcov_test_coverage_decl) = 1;
+ DECL_COMDAT_GROUP (gcov_test_coverage_decl)
+ = DECL_ASSEMBLER_NAME (gcov_test_coverage_decl);
+ TREE_STATIC (gcov_test_coverage_decl) = 1;
+ DECL_INITIAL (gcov_test_coverage_decl) = build_int_cst (
+ get_gcov_unsigned_t (),
+ flag_test_coverage ? 1 : 0);
+ varpool_finalize_decl (gcov_test_coverage_decl);
+}
}
/* Initialization function for FDO sampling. */
Index: libgcc/libgcov-driver.c
===
--- libgcc/libgcov-driver.c (revision 212044)
+++ libgcc/libgcov-driver.c (working copy)
@@ -79,6 +79,8 @@ extern unsigned int __gcov_sampling_enabled (void)
char *(*__gcov_dummy_ref5)(void) = &__gcov_sampling_enabled;
extern void __gcov_flush (void);
char *(*__gcov_dummy_ref6)(void) = &__gcov_flush;
+extern unsigned int __gcov_profiling_for_test_coverage (void);
+char *(*__gcov_dummy_ref7)(void) = &__gcov_profiling_for_test_coverage;
/* Default callback function for profile instrumentation callback. */
extern void __coverage_callback (gcov_type, int);
Index: libgcc/libgcov-interface.c
===
--- libgcc/libgcov-interface.c (revision 212044)
+++ libgcc/libgcov-interface.c (working copy)
@@ -116,6 +116,20 @@ __gcov_dump (void)
set_gcov_dump_complete ();
}
+/* Emitted in coverage.c. */
+extern gcov_unsigned_t __gcov_test_coverage;
+
+unsigned int __gcov_profiling_for_test_coverage (void);
+
+/* Function that can be called from application to distinguish binaries
+ instrumented for coverage from those instrumented for profile
+ optimization (e.g. -fprofile-generate). */
+
+unsigned int __gcov_profiling_for_test_coverage (void)
+{
+ return __gcov_test_coverage;
+}
+
#endif /* L_gcov_dump */
#ifdef L_gcov_sampling
On Wed, Jul 2, 2014 at 1:25 PM, Xinliang David Li wrote:
> The reason is that test coverage is using the same underlying
> profiling. Returning false when calling '__gcov_profiling_enabled())
> in coverage mode is a little misleading.
>
> David
>
> On Wed, Jul 2, 2014 at 1:22 PM, Teresa Johnson wrote:
>> On Wed, Jul 2, 2014 at 1:15 PM, Xinliang David Li wrote:
>>> Should the interface be something like:
>>>
>>> int __gcov_profiling_for_test_coverage(void)?
>>
>> I was equating the term "profiling" with -fprofile-generate, as
>> opposed to -ftest-coverage instrumentation. But I can change it to
>> this if you think that is clearer.
>>
>> Teresa
>>
>>>
>>> David
>>>
>>>
>>> On Wed, Jul 2, 2014 at 12:55 PM, Teresa Johnson
>>> wrote:
The following patch adds support for a new libgcov interface,
__gcov_profiling_enabled, that can be used by applications to
determine whether a binary has been instrumented for test coverage or
profile optimization.
Passes regression tests and manual testing with different options. Ok
for google branches?
Thanks,
Teresa
2014-07-02 Teresa Johnson
Google ref b/15378201.
* gcc/tree-profile.c (gcov_test_coverage_decl): Declare.
(tree_init_instrumentation): Initialize gcov_test_coverage_decl.
* libgcc/libgcov-driver.c (__gcov_dummy_ref7): Define.
Re: [GOMP4, OpenACC] Fixed-form Fortran code failing to parse
On 07/02/2014 12:02 PM, Tobias Burnus wrote:
> Thomas Schwinge wrote:
>> In -fopenmp mode as well as in combined -fopenacc -fopenmp mode as
>> well as in "regular" (no -fopen*) mode, it parses fine.
>
> [Note I am testing with an outdated branch (20140404), but it still
> might be representative.]
>
> I am not sure it parses fine in the combined more. If I use
>
> *$ACC xPARALLEL
>
> i.e. add a typo, it fails to diagnose it in the combined mode. It does
> diagnose it for -fopenacc:
>
> *$ACC xPARALLEL COPYIN(ARGC)
> 1
> Error: Unclassifiable OpenACC directive at (1)
>
> And it does diagnose the problem for $OMP in only OpenMP and in combined
> mode.
Thanks for catching that.
>> I couldn't find anything obvious in gcc/fortran/parse.c; is someone
>> able to have a more in-depth look than I have?
>
> First, some minor point, I think one should crosswise reset the
> {openmp,openacc}_flag, i.e.
>
> --- a/gcc/fortran/scanner.c
> +++ b/gcc/fortran/scanner.c
> @@ -735,2 +735,3 @@ skip_oacc_attribute (locus start, locus old_loc,
> bool continue_flag)
> openacc_flag = 1;
> + openmp_flag = 0;
> openacc_locus = old_loc;
> @@ -775,2 +776,3 @@ skip_omp_attribute (locus start, locus old_loc, bool
> continue_flag)
> openmp_flag = 1;
> + openacc_flag = 0;
> openmp_locus = old_loc;
> @@ -826,3 +828,3 @@ skip_free_comments (void)
> /* Keep the !GCC$ line. */
> - if (at_bol && skip_gcc_attribute (start))
> + if (at_bol && skip_gcc_attribute (start))
> return false;
I think openmp and openacc should be able to coexist, so that should be
left in for now.
> Secondly, the following looks wrong (also scanner.c). I have the feeling
> that this could be behind the issue above. But in any case, it looks
> wrong to me.
>
>
> skip_fixed_comments (void)
> ...
> start = gfc_current_locus;
> c = next_char ();
> if (c == '!' || c == 'c' || c == 'C' || c == '*')
> if (gfc_option.gfc_flag_openmp)
> {
> if (next_char () == '$')
> {
> c = next_char ();
> if (c == 'o' || c == 'O')
> {
> ...
> gfc_current_locus = start;
> ...
> }
>
> if (gfc_option.gfc_flag_openacc)
> {
> if (next_char () == '$')
> {
>
> Namely: If "omp" or the "!$" continuation lines hasn't been matched, one
> returns to "start". However, start is "!" or "*" in this case and not
> "$". In addition, also in this case, one might want to set the other
> flag to 0.
>
>
> My feeling is that the bug is also related to something in scanner.c.
The problem was inside skip_fixed_comments. The openacc_flag wasn't
being set to zero at the end of that function. That's bad, because acc
directive can't continue across multiple comments. E.g.
!$acc
! parallel
is invalid. This patch fixes that.
Thomas, is this patch ok for gomp-4_0-branch? If so, please check it in.
Thanks,
Cesar
> PS: I might try later to find out what goes wrong, but that requires in
> any case a binary which is not older than the source code.
2014-07-02 Cesar Philippidis
gcc/fortran/
* scanner.c (skip_fixed_comments): Reset openacc_flag
if an openacc directive wasn't found.
gcc/testsuite/
* gfortran.dg/goacc/fixed-1.f: New test.
* gfortran.dg/goacc/fixed-2.f: New test.
diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index 6936222..5869d7d 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -1170,6 +1170,7 @@ skip_fixed_comments (void)
}
openmp_flag = 0;
+ openacc_flag = 0;
gcc_attribute_flag = 0;
gfc_current_locus = start;
}
diff --git a/gcc/testsuite/gfortran.dg/goacc/fixed-1.f b/gcc/testsuite/gfortran.dg/goacc/fixed-1.f
new file mode 100644
index 000..6a45419
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/fixed-1.f
@@ -0,0 +1,12 @@
+ INTEGER :: ARGC
+ ARGC = COMMAND_ARGUMENT_COUNT ()
+
+!$OMP PARALLEL
+!$ACC PARALLEL COPYIN(ARGC)
+ IF (ARGC .NE. 0) THEN
+ CALL ABORT
+ END IF
+!$ACC END PARALLEL
+!$OMP END PARALLEL
+
+ END
diff --git a/gcc/testsuite/gfortran.dg/goacc/fixed-2.f b/gcc/testsuite/gfortran.dg/goacc/fixed-2.f
new file mode 100644
index 000..2c2b0a3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/fixed-2.f
@@ -0,0 +1,15 @@
+! { dg-do compile }
+! { dg-additional-options "-fmax-errors=100" }
+
+ INTEGER :: ARGC
+ ARGC = COMMAND_ARGUMENT_COUNT ()
+
+!$OMP xPARALLEL
+!$ACC xPARALLEL COPYIN(ARGC) ! { dg-error "Unclassifiable OpenACC directive" }
+ IF (ARGC .NE. 0) THEN
+ CALL ABORT
+ END IF
+!$ACC END PARALLEL ! { dg-error "Unexpected" }
+!$OMP END PARALLEL
+
+ END
Re: Strenghten assumption about dynamic type changes (placement new)
> On 07/02/2014 01:18 PM, Jan Hubicka wrote: > >We propagate types from places we know instances are created across pointers > >passed to functions. Once non-POD type is created at a given memory > >location, > >one can not change its type by placement_new into something else. > > Hmm. If the memory location is untyped (i.e. from malloc) or a > character array, or a union, you can indeed destroy an object of one > type and create an object of a different type in that location. > > >Jason, this assumes that one can not destroy the type and re-construct same > >type at the same spot. > > That is an invalid assumption; you can destroy one object and > construct a new one in the same location. Doing it within a method > would be unusual, but I don't think there's a rule against it. The types get currently are one comming from declarations of variables and parameters passed by reference. Can I really destroy it/allocate new type/destroy new type/allocate back the original type (or terminate the program) so the destruction at the end of the lifetimate of the variable apsses? I suppose we should keep track if memory location is comming from declaration or it is dynamically typed (i.e. type seen from calling constructor)? Currently we don't deal with dynamic types, but I have patches for that. Honza > > Jason
Re: Strenghten assumption about dynamic type changes (placement new)
> > On 07/02/2014 01:18 PM, Jan Hubicka wrote: > > >We propagate types from places we know instances are created across > > >pointers > > >passed to functions. Once non-POD type is created at a given memory > > >location, > > >one can not change its type by placement_new into something else. > > > > Hmm. If the memory location is untyped (i.e. from malloc) or a > > character array, or a union, you can indeed destroy an object of one > > type and create an object of a different type in that location. > > > > >Jason, this assumes that one can not destroy the type and re-construct same > > >type at the same spot. > > > > That is an invalid assumption; you can destroy one object and > > construct a new one in the same location. Doing it within a method > > would be unusual, but I don't think there's a rule against it. > > The types get currently are one comming from declarations of variables > and parameters passed by reference. > Can I really destroy it/allocate new type/destroy new type/allocate back the > original > type (or terminate the program) so the destruction at the end of the > lifetimate of the > variable apsses? > > I suppose we should keep track if memory location is comming from declaration > or > it is dynamically typed (i.e. type seen from calling constructor)? Currently > we don't > deal with dynamic types, but I have patches for that. But this is one of things that was not quite clear to me. I know that polymorphic type A was created at a give memory location. THis means that accesses to that location in one alias class has been made. Now I destroy A and turn it into B, construct B and make memory accesses in different alias set. I see this has chance to work if one is base of another, but if B is completely different type, I think strick aliasin should just make those accesses to not alias and in turn make whole thing undefined? honza
Re: Strenghten assumption about dynamic type changes (placement new)
On 07/02/2014 06:30 PM, Jan Hubicka wrote: But this is one of things that was not quite clear to me. I know that polymorphic type A was created at a give memory location. THis means that accesses to that location in one alias class has been made. Now I destroy A and turn it into B, construct B and make memory accesses in different alias set. I see this has chance to work if one is base of another, but if B is completely different type, I think strick aliasin should just make those accesses to not alias and in turn make whole thing undefined? Right, if they're unrelated types the accesses don't alias (3.10p10). On the subject of aliasing, there's a proposal to add explicit alias sets to C++: http://open-std.org/jtc1/sc22/wg21/docs/papers/2014/n3988.pdf Any thoughts? Jason
Re: [C/C++ PATCH] Implement -Wsizeof-array-argument (PR c/6940)
On 06/26/2014 03:22 PM, Marek Polacek wrote:
The following is a revamped patch for -Wsizeof-array-argument.
Its purpose is to detect suspicious usage of the sizeof operator on an array
function parameter.
Then the name should be -Wsizeof-array-parm, not -argument.
@@ -9550,6 +9551,8 @@ grokdeclarator (const cp_declarator *declarator,
array. */
returned_attrs = chainon (returned_attrs,
declarator->std_attributes);
+ if (decl_context == PARM)
+ array_parameter_p = true;
break;
Setting this here means that you'll treat a parameter with
pointer-to-array type as an array parm. I think you want to set it
here, instead:
/* A parameter declared as an array of T is really a pointer to T.
One declared as a function is really a pointer to a function.
One declared as a member is really a pointer to member. */
if (TREE_CODE (type) == ARRAY_TYPE)
{
/* Transfer const-ness of array into that of type pointed to. */
type = build_pointer_type (TREE_TYPE (type));
type_quals = TYPE_UNQUALIFIED;
}
Jason
Re: [GOOGLE] Define libgcov interface for distinguishing -ftest-coverage from -fprofile-generate
ok.
thanks,
David
On Wed, Jul 2, 2014 at 5:20 PM, Teresa Johnson wrote:
> New patch below. Retested. Ok for google branches?
>
> Thanks,
> Teresa
>
> 2014-07-02 Teresa Johnson
>
> Google ref b/15378201.
> * gcc/tree-profile.c (gcov_test_coverage_decl): Declare.
> (tree_init_instrumentation): Initialize gcov_test_coverage_decl.
> * libgcc/libgcov-driver.c (__gcov_dummy_ref7): Define.
> * libgcc/libgcov-interface.c (__gcov_profiling_for_test_coverage): New
> function.
>
> Index: gcc/tree-profile.c
> ===
> --- gcc/tree-profile.c (revision 212044)
> +++ gcc/tree-profile.c (working copy)
> @@ -164,6 +164,9 @@ static GTY(()) tree gcov_sample_counter_decl = NUL
> /* extern gcov_unsigned_t __gcov_profile_prefix */
> static tree GTY(()) gcov_profile_prefix_decl = NULL_TREE;
>
> +/* extern gcov_unsigned_t __gcov_test_coverage */
> +static tree GTY(()) gcov_test_coverage_decl = NULL_TREE;
> +
> /* extern gcov_unsigned_t __gcov_sampling_period */
> static GTY(()) tree gcov_sampling_period_decl = NULL_TREE;
>
> @@ -498,6 +501,27 @@ tree_init_instrumentation (void)
>DECL_INITIAL (gcov_profile_prefix_decl) = prefix_ptr;
>varpool_finalize_decl (gcov_profile_prefix_decl);
> }
> +
> + if (!gcov_test_coverage_decl)
> +{
> + /* Initialize __gcov_test_coverage to 1 if -ftest-coverage
> + specified, 0 otherwise. Used by libgcov to determine whether
> + a binary was instrumented for coverage or profile optimization. */
> + gcov_test_coverage_decl = build_decl (
> + UNKNOWN_LOCATION,
> + VAR_DECL,
> + get_identifier ("__gcov_test_coverage"),
> + get_gcov_unsigned_t ());
> + TREE_PUBLIC (gcov_test_coverage_decl) = 1;
> + DECL_ARTIFICIAL (gcov_test_coverage_decl) = 1;
> + DECL_COMDAT_GROUP (gcov_test_coverage_decl)
> + = DECL_ASSEMBLER_NAME (gcov_test_coverage_decl);
> + TREE_STATIC (gcov_test_coverage_decl) = 1;
> + DECL_INITIAL (gcov_test_coverage_decl) = build_int_cst (
> + get_gcov_unsigned_t (),
> + flag_test_coverage ? 1 : 0);
> + varpool_finalize_decl (gcov_test_coverage_decl);
> +}
> }
>
> /* Initialization function for FDO sampling. */
> Index: libgcc/libgcov-driver.c
> ===
> --- libgcc/libgcov-driver.c (revision 212044)
> +++ libgcc/libgcov-driver.c (working copy)
> @@ -79,6 +79,8 @@ extern unsigned int __gcov_sampling_enabled (void)
> char *(*__gcov_dummy_ref5)(void) = &__gcov_sampling_enabled;
> extern void __gcov_flush (void);
> char *(*__gcov_dummy_ref6)(void) = &__gcov_flush;
> +extern unsigned int __gcov_profiling_for_test_coverage (void);
> +char *(*__gcov_dummy_ref7)(void) = &__gcov_profiling_for_test_coverage;
>
> /* Default callback function for profile instrumentation callback. */
> extern void __coverage_callback (gcov_type, int);
> Index: libgcc/libgcov-interface.c
> ===
> --- libgcc/libgcov-interface.c (revision 212044)
> +++ libgcc/libgcov-interface.c (working copy)
> @@ -116,6 +116,20 @@ __gcov_dump (void)
>set_gcov_dump_complete ();
> }
>
> +/* Emitted in coverage.c. */
> +extern gcov_unsigned_t __gcov_test_coverage;
> +
> +unsigned int __gcov_profiling_for_test_coverage (void);
> +
> +/* Function that can be called from application to distinguish binaries
> + instrumented for coverage from those instrumented for profile
> + optimization (e.g. -fprofile-generate). */
> +
> +unsigned int __gcov_profiling_for_test_coverage (void)
> +{
> + return __gcov_test_coverage;
> +}
> +
> #endif /* L_gcov_dump */
>
> #ifdef L_gcov_sampling
>
> On Wed, Jul 2, 2014 at 1:25 PM, Xinliang David Li wrote:
>> The reason is that test coverage is using the same underlying
>> profiling. Returning false when calling '__gcov_profiling_enabled())
>> in coverage mode is a little misleading.
>>
>> David
>>
>> On Wed, Jul 2, 2014 at 1:22 PM, Teresa Johnson wrote:
>>> On Wed, Jul 2, 2014 at 1:15 PM, Xinliang David Li
>>> wrote:
Should the interface be something like:
int __gcov_profiling_for_test_coverage(void)?
>>>
>>> I was equating the term "profiling" with -fprofile-generate, as
>>> opposed to -ftest-coverage instrumentation. But I can change it to
>>> this if you think that is clearer.
>>>
>>> Teresa
>>>
David
On Wed, Jul 2, 2014 at 12:55 PM, Teresa Johnson
wrote:
> The following patch adds support for a new libgcov interface,
> __gcov_profiling_enabled, that can be used by applications to
> determine whether a binary has been instrumented for test coverage or
> profile optimization.
>
> Passes regression tests and manual testing with different options. Ok
> for google branches?
>
> Thanks,
> Teresa
>
[Committed]: [PATCH, loop2_invariant, 2/2] Change heuristics for identical invariants
On 2 July 2014 03:54, Jeff Law wrote:
> On 07/01/14 01:16, Zhenqiang Chen wrote:
>
>>
>> ChangeLog:
>> 2014-07-01 Zhenqiang Chen
>>
>> * loop-invariant.c (struct invariant): Add a new member: eqno;
>> (find_identical_invariants): Update eqno;
>> (create_new_invariant): Init eqno;
>> (get_inv_cost): Compute comp_cost wiht eqno;
>
> s/wiht/with/
>
> Do you have a testcase for this? If at all possible I'd like to see some
> kind of test to verify that this tracking results in better invariant
> selection.
Since the patch bases on the result of rtx_cost, it is hard to design
a common test case. A testcase for ARM is added.
> With some kind of test and the ChangeLog typo fixed, this is OK.
Thanks. Updated and committed @r212256.
ChangeLog:
2014-07-03 Zhenqiang Chen
* loop-invariant.c (struct invariant): Add a new member: eqno;
(find_identical_invariants): Update eqno;
(create_new_invariant): Init eqno;
(get_inv_cost): Compute comp_cost with eqno;
testsuite/ChangeLog:
2014-07-03 Zhenqiang Chen
* gcc.target/arm/identical-invariants.c: New test.
diff --git a/gcc/loop-invariant.c b/gcc/loop-invariant.c
index d47d461..bd67eb9 100644
--- a/gcc/loop-invariant.c
+++ b/gcc/loop-invariant.c
@@ -104,6 +104,9 @@ struct invariant
/* The number of the invariant with the same value. */
unsigned eqto;
+ /* The number of invariants which eqto this. */
+ unsigned eqno;
+
/* If we moved the invariant out of the loop, the register that contains its
value. */
rtx reg;
@@ -498,6 +501,7 @@ find_identical_invariants (invariant_htab_type
*eq, struct invariant *inv)
struct invariant *dep;
rtx expr, set;
enum machine_mode mode;
+ struct invariant *tmp;
if (inv->eqto != ~0u)
return;
@@ -513,7 +517,12 @@ find_identical_invariants (invariant_htab_type
*eq, struct invariant *inv)
mode = GET_MODE (expr);
if (mode == VOIDmode)
mode = GET_MODE (SET_DEST (set));
- inv->eqto = find_or_insert_inv (eq, expr, mode, inv)->invno;
+
+ tmp = find_or_insert_inv (eq, expr, mode, inv);
+ inv->eqto = tmp->invno;
+
+ if (tmp->invno != inv->invno && inv->always_executed)
+tmp->eqno++;
if (dump_file && inv->eqto != inv->invno)
fprintf (dump_file,
@@ -1136,7 +1149,7 @@ get_inv_cost (struct invariant *inv, int
*comp_cost, unsigned *regs_needed,
if (!inv->cheap_address
|| inv->def->n_addr_uses < inv->def->n_uses)
-(*comp_cost) += inv->cost;
+(*comp_cost) += inv->cost * inv->eqno;
#ifdef STACK_REGS
{
diff --git a/gcc/testsuite/gcc.target/arm/identical-invariants.c
b/gcc/testsuite/gcc.target/arm/identical-invariants.c
new file mode 100644
index 000..7762ce6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/identical-invariants.c
@@ -0,0 +1,29 @@
+/* { dg-do compile { target { arm_thumb2_ok } } } */
+/* { dg-options "-O2 -fdump-rtl-loop2_invariant " } */
+
+int t1, t2, t3, t4, t5, t6, t7, t8, t9, t10;
+extern void foo2 (int *, int *, int *, int *, int *, int *);
+extern int foo3 (int, int, int, int, int, int);
+int foo (int a, int b, int c, int d)
+{
+ int i = a;
+
+ for (; i > 0; i += b)
+{
+ if (a > 0x1234567)
+ foo2 (&t1, &t2, &t3, &t4, &t5, &t6);
+ foo2 (&t1, &t2, &t3, &t4, &t5, &t6);
+ if (b > 0x1234567)
+ foo2 (&t7, &t2, &t8, &t4, &t5, &t6);
+ foo2 (&t1, &t2, &t3, &t4, &t5, &t6);
+ if (c > 0x1234567)
+ foo2 (&t1, &t9, &t10, &t4, &t5, &t6);
+ t2 = t5 - d;
+}
+
+ return foo3 (t1, t2, t3, t4, t5, t6);
+}
+
+/* { dg-final { scan-rtl-dump "Decided to move invariant 0"
"loop2_invariant" } } */
+/* { dg-final { cleanup-rtl-dump "loop2_invariant" } } */
+
Re: [GOMP4, OpenACC] Fixed-form Fortran code failing to parse
Cesar Philippidis wrote:
I couldn't find anything obvious in gcc/fortran/parse.c; is someone
able to have a more in-depth look than I have?
First, some minor point, I think one should crosswise reset the
{openmp,openacc}_flag, i.e.
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -735,2 +735,3 @@ skip_oacc_attribute (locus start, locus old_loc,
bool continue_flag)
openacc_flag = 1;
+ openmp_flag = 0;
openacc_locus = old_loc;
@@ -775,2 +776,3 @@ skip_omp_attribute (locus start, locus old_loc, bool
continue_flag)
openmp_flag = 1;
+ openacc_flag = 0;
openmp_locus = old_loc;
I think openmp and openacc should be able to coexist, so that should be
left in for now.
Well, -fopenmp and -fopenacc should coexist, which means that both
gfc_option.gfc_flag_openmp (or gfc_option.gfc_flag_openmp_simd) and
gfc_option.gfc_flag_openacc should coexist. However, openacc_flag and
openmp_flag are used for continuation lines – and the same line cannot
be simultaneously an OpenMP and an OpenACC statement. Thus, openacc_flag
and openmp_flag should be mutually exclusive.
I don't know whether it makes a difference in practice; if it doesn't
one could also change it into a single flag. However, I would simply
ensure that maximally only one is set at a given time.
Secondly, the following looks wrong (also scanner.c). I have the feeling
that this could be behind the issue above. But in any case, it looks
wrong to me.
skip_fixed_comments (void)
...
start = gfc_current_locus;
c = next_char ();
if (c == '!' || c == 'c' || c == 'C' || c == '*')
...
if (gfc_option.gfc_flag_openmp)
{
if (next_char () == '$')
...
gfc_current_locus = start;
...
if (gfc_option.gfc_flag_openacc)
{
if (next_char () == '$')
{
Namely: If "omp" or the "!$" continuation lines hasn't been matched, one
returns to "start". However, start is "!" or "*" in this case and not
"$". In addition, also in this case, one might want to set the other
flag to 0.
I still think that there is something wrong with the gfc_current_local
handling above – even if it is not behind the bug we are currently
seeing. In addition, I wonder whether the inner else branch couldn't be
merged, i.e. the one which takes care of conditional compilation of the form
!$ This line is a comment, unless -fopenmp or -fopenacc is enabled
Those checks appear twice for fixed-form source code and the code is 24 lines
long in either case.
The problem was inside skip_fixed_comments. The openacc_flag wasn't
being set to zero at the end of that function. [...]
Thomas, is this patch ok for gomp-4_0-branch? If so, please check it in.
Looks good to me. Thanks for the patch.
Tobias
Re: [PATCH] Don't run guality.exp tests with LTO_TORTURE_OPTIONS.
On Wed, Jul 02, 2014 at 04:06:30PM -0700, Jason Merrill wrote: > I think that makes sense; I'm not aware of anyone working on improving LTO > debugging. I think at this point all we care about is that with -flto we don't ICE on those, perhaps we should arrange to change all the tests into dg-do compile with -flto and ignore all gdb-test and have some env var override which would force full testing also with -flto? Jakub
Re: [PATCH] Power/GCC: Subword atomic operation endianness check bug fix
On Wed, 2 Jul 2014, David Edelsohn wrote: > * config/rs6000/rs6000.c (rs6000_adjust_atomic_subword): Use > BYTES_BIG_ENDIAN rather than WORDS_BIG_ENDIAN to check for byte > endianness. > > This patch is okay. Thanks for noticing it. Committed, thanks. Maciej
[PATCH] Fix simplify_comparison in the combiner (PR rtl-optimization/61673)
Hi!
The following testcase is miscompiled on s390-linux (31-bit).
r202393 changed:
@@ -11946,11 +11949,11 @@
if (op1 == const0_rtx && (code == LT || code == GE)
&& HWI_COMPUTABLE_MODE_P (mode))
{
+ unsigned HOST_WIDE_INT sign
+ = (unsigned HOST_WIDE_INT) 1 << (GET_MODE_BITSIZE (mode) - 1);
op0 = simplify_gen_binary (AND, tmode,
gen_lowpart (tmode, op0),
-GEN_INT ((unsigned HOST_WIDE_INT) 1
- << (GET_MODE_BITSIZE (mode)
- - 1)));
+gen_int_mode (sign, mode));
code = (code == LT) ? NE : EQ;
break;
}
This code creates AND of a paradoxical subreg where the bits above
mode are undefined, so of course the mask has to check just the single
sign bit rather than that bit + all bits above it.
In this particular testcase, mode is QImode and tmode is SImode,
previously and with my patch we were masking with 128, current 4.9 branch
and trunk masks with -128.
Bootstrapped/regtested on x86_64-linux, i686-linux and s390{,x}-linux. Ok
for trunk/4.9?
2014-07-03 Jakub Jelinek
PR rtl-optimization/61673
* combine.c (simplify_comparison): Test just mode's sign bit
in tmode rather than the sign bit and any bits above it.
* gcc.c-torture/execute/pr61673.c: New test.
--- gcc/combine.c.jj2014-03-28 20:49:52.892077022 +0100
+++ gcc/combine.c 2014-07-02 16:56:02.260456040 +0200
@@ -11987,7 +11987,7 @@ simplify_comparison (enum rtx_code code,
= (unsigned HOST_WIDE_INT) 1 << (GET_MODE_BITSIZE (mode) - 1);
op0 = simplify_gen_binary (AND, tmode,
gen_lowpart (tmode, op0),
-gen_int_mode (sign, mode));
+gen_int_mode (sign, tmode));
code = (code == LT) ? NE : EQ;
break;
}
--- gcc/testsuite/gcc.c-torture/execute/pr61673.c.jj2014-07-02
17:17:01.398908630 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr61673.c 2014-07-02
17:12:36.0 +0200
@@ -0,0 +1,50 @@
+/* PR rtl-optimization/61673 */
+
+char e;
+
+__attribute__((noinline, noclone)) void
+bar (char x)
+{
+ if (x != 0x54 && x != (char) 0x87)
+__builtin_abort ();
+}
+
+__attribute__((noinline, noclone)) void
+foo (const char *x)
+{
+ char d = x[0];
+ int c = d;
+ if ((c >= 0 && c <= 0x7f) == 0)
+e = d;
+ bar (d);
+}
+
+__attribute__((noinline, noclone)) void
+baz (const char *x)
+{
+ char d = x[0];
+ int c = d;
+ if ((c >= 0 && c <= 0x7f) == 0)
+e = d;
+}
+
+int
+main ()
+{
+ const char c[] = { 0x54, 0x87 };
+ e = 0x21;
+ foo (c);
+ if (e != 0x21)
+__builtin_abort ();
+ foo (c + 1);
+ if (e != (char) 0x87)
+__builtin_abort ();
+ e = 0x21;
+ baz (c);
+ if (e != 0x21)
+__builtin_abort ();
+ baz (c + 1);
+ if (e != (char) 0x87)
+__builtin_abort ();
+ return 0;
+}
Jakub
Re: [PATCH] Don't run guality.exp tests with LTO_TORTURE_OPTIONS.
On July 3, 2014 1:06:30 AM CEST, Jason Merrill wrote: >I think that makes sense; I'm not aware of anyone working on improving >LTO debugging. I've done that in the past. So it would be nice to verify we don't regress existing tests. Richard. >Jason
