Re: [PATCH 1/3] optabs: ensure mem_thread_fence is a compiler barrier
Alexander Monakov writes:
> diff --git a/gcc/optabs.c b/gcc/optabs.c
> index a9900657a58..d568ca376fe 100644
> --- a/gcc/optabs.c
> +++ b/gcc/optabs.c
> @@ -6298,7 +6298,12 @@ void
> expand_mem_thread_fence (enum memmodel model)
> {
>if (targetm.have_mem_thread_fence ())
> -emit_insn (targetm.gen_mem_thread_fence (GEN_INT (model)));
> +{
> + rtx_insn *last = get_last_insn ();
> + emit_insn (targetm.gen_mem_thread_fence (GEN_INT (model)));
> + if (last == get_last_insn () && !is_mm_relaxed (model))
> + expand_asm_memory_barrier ();
> +}
>else if (!is_mm_relaxed (model))
> {
>if (targetm.have_memory_barrier ())
It would be simpler to test whether targetm.gen_mem_thread_fence
returns NULL.
This feels a bit hacky though. Checking whether a generator produced no
instructions is usually the test for whether the generator FAILed, which
should normally be treated as though the pattern wasn't defined to start
with. This is useful if the FAIL condition is too complex to put in the
define_expand/insn C condition.
In those contexts, if a generator wants to succeed and generate no
instructions, it should emit a note instead. (Yes, it would be good
to have a nicer interface...)
So IMO it's confusing to overload the empty sequence to mean something
else in this context, and it wouldn't work if a target does emit the kind
of "I didn't FAIL" note just mentioned.
FWIW, I agree with Jeff in the previous thread that (compared to this)
it would be more robust to make optabs.c emit the compile barrier
unconditionally and just leave the target to emit a machine barrier.
Your earlier patch to make targets emit at least a compile barrier
seemed good too. Perhaps that way we can treat an empty sequence as a
FAIL in the normal way, or assert that the pattern doesn't FAIL.
Thanks,
Richard
Re: Add support to trace comparison instructions and switch statements
Hi all Is it worth adding my codes to gcc ? Are there some steps I need to do ? Could somebody tell me the progress ? Maybe there should be a project like libfuzzer to solve bugs in program. Wish Wu -- From:Wish Wu Time:2017 Jul 21 (Fri) 13:38 To:gcc ; gcc-patches ; Jeff Law Cc:wishwu007 Subject:Re: Add support to trace comparison instructions and switch statements Hi Jeff I have signed the copyright assignment, and used the name 'Wish Wu' . Should I send you a copy of my assignment ? The attachment is my new patch with small changes. Codes are checked by ./contrib/check_GNU_style.sh, except some special files. With -- From:Jeff Law Time:2017 Jul 14 (Fri) 15:37 To:Wish Wu ; gcc ; gcc-patches Cc:wishwu007 Subject:Re: Add support to trace comparison instructions and switch statements On 07/10/2017 06:07 AM, 吴潍浠(此彼) wrote: > Hi > > I write some codes to make gcc support comparison-guided fuzzing. > It is very like > http://clang.llvm.org/docs/SanitizerCoverage.html#tracing-data-flow . > With -fsanitize-coverage=trace-cmp the compiler will insert extra > instrumentation around comparison instructions and switch statements. > I think it is useful for fuzzing. :D > > Patch is below, I may supply test cases later. Before anyone can really look at this code you'll need to get a copyright assignment on file with the FSF. See: https://gcc.gnu.org/contribute.html If you've already done this, please let me know and I'll confirm with the FSF copyright clerk. Jeff
Re: [PATCH 0/5] RFC, WIP: RTL cost improvements
Segher Boessenkool writes: > This series creates pattern_cost and insn_cost functions that together > replace the existing insn_rtx_cost function. > > pattern_cost is like the old insn_rtx_cost function; insn_cost takes > an actual rtx_insn * as input, not just a pattern. > > Also a targetm.insn_cost is added, which targets can use to implement > a more exact cost more easily. > > The combine patch is pretty gross (but functional), it needs some > refactoring (to not call recog so often). The rs6000 patch is very > much a work in progress. > > How does this look? Is this the right direction? Seems good to me FWIW. Since this hook is entirely new, would it be worth standardising on attribute names for size and speed costs, a bit like "length" and "enabled"? I think otherwise the target hooks are going to end up with similar boilerplate. Thanks, Richard
Re: [PATCH 0/5] RFC, WIP: RTL cost improvements
On Sat, Aug 05, 2017 at 11:07:20AM +0100, Richard Sandiford wrote: > Segher Boessenkool writes: > > This series creates pattern_cost and insn_cost functions that together > > replace the existing insn_rtx_cost function. > > > > pattern_cost is like the old insn_rtx_cost function; insn_cost takes > > an actual rtx_insn * as input, not just a pattern. > > > > Also a targetm.insn_cost is added, which targets can use to implement > > a more exact cost more easily. > > > > The combine patch is pretty gross (but functional), it needs some > > refactoring (to not call recog so often). The rs6000 patch is very > > much a work in progress. > > > > How does this look? Is this the right direction? > > Seems good to me FWIW. Since this hook is entirely new, would it > be worth standardising on attribute names for size and speed costs, > a bit like "length" and "enabled"? I think otherwise the target hooks > are going to end up with similar boilerplate. For size cost I currently use just "length", but I haven't looked at size cost much at all yet. For speed cost I primarily use "type", modified by the number of machine insns a pattern generates (quite a few are split); and I get the number of machine insns just from "length" again, which for rs6000 is easy and correct in most cases. Some other targets may need something else. I also have an attribute "cost" that can be used to override the default calculation; that seems useful to standardise on. I've pondered a "cost_adjust" that will be added to the calculated cost instead, but it hasn't been useful so far. There are two types of insns that happen in the insn stream but which aren't recognised: asm, and no-op moves. I cost both as cost 0 currently. "length" and "enabled" are special, they are used by non-target code. So far I don't think it is useful to handle "cost" specially, but it is such a nice short name that I expect many targets will want it, whether it is handled generically or not :-) Segher
Re: [PATCH 0/2] add unique_ptr class
On 08/04/2017 07:52 PM, Jonathan Wakely wrote: > On 31/07/17 19:46 -0400, tbsaunde+...@tbsaunde.org wrote: >> I've been saying I'd do this for a long time, but I'm finally getting to >> importing the C++98 compatable unique_ptr class Pedro wrote for gdb a >> while >> back. Thanks a lot for doing this! > I believe the gtl namespace also comes from Pedro, but GNU template > library seems as reasonable as any other name I can come up with. Yes, I had suggested it here: https://sourceware.org/ml/gdb-patches/2017-02/msg00197.html > > If it's inspired by "STL" then can we call it something else? > > std::unique_ptr is not part of the STL, because the STL is a library > of containers and algorithms from the 1990s. std::unique_ptr is > something much newer that doesn't originate in the STL. > > STL != C++ Standard Library That I knew, but gtl was not really a reference to the C++ Standard Library, so I don't see the problem. It was supposed to be the name of a library which would contain other C++ utilities that would be shared by different GNU toolchain components. Some of those bits would be inspired by, or be straight backports of more-recent standards, but it'd be more than that. An option would be to keep "gtl" as acronym, but expand it to "GNU Toolchain Library" instead. For example, gdb has been growing C++ utilities under gdb/common/ that might be handy for gcc and other projects too, for example: - enum_flags (type-safe enum bit flags) - function_view (non-owning reference to callables) - offset_type (type safe / distinct integer types to represent offsets into anything addressable) - optional (C++11 backport of libstdc++'s std::optional) - refcounted_object.h (intrusively-refcounted types) - scoped_restore (RAII save/restore of globals) - an allocator that default-constructs using default-initialization instead of value-initialization. and more. GCC OTOH has code that might be handy for GDB as well, like for example the open addressing hash tables (hash_map/hash_table/hash_set etc.). Maybe Gold could make use of some of this code too. There are some bits in Gold that might be useful for (at least) GDB too. For example, its Stringpool class. I think there's a lot of scope for sharing more code between the different components of the GNU toolchain, even beyond general random utilities and data structures, and I'd love to see us move more in that direction. > If we want a namespace for GNU utilities, what's wrong with "gnu"? That'd be an "obvious" choice, and I'm not terribly against it, though I wonder whether it'd be taking over a name that has a wider scope than intended? I.e., GNU is a larger set of projects than the GNU toolchain. For example, there's Gnulib, which already compiles as libgnu.a / -lgnu, which might be confusing. GCC doesn't currently use Gnulib, but GDB does, and, there was work going on a while ago to make GCC use gnulib as well. Thanks, Pedro Alves
Re: Restore proper operation of -fdump-ada-spec in C++
> It was broken by the recent removal of TYPE_METHODS.
It turns out that the support for constructors/destructors also needs to be
adjusted after the recent changes.
Tested on x86_64-suse-linux, applied on the mainline.
2017-08-05 Eric Botcazou
c-family/
* c-ada-spec.c (has_static_fields): Look only into variables.
(print_constructor): Add TYPE parameter and use it for the name.
(print_destructor): Likewise.
(print_ada_declaration): Adjust to new constructor/destructor names.
Adjust calls to print_constructor and print_destructor.
(print_ada_struct_decl): Do not test TREE_STATIC on FIELD_DECL.
Look only into variables in the final loop.
--
Eric BotcazouIndex: c-ada-spec.c
===
--- c-ada-spec.c (revision 250802)
+++ c-ada-spec.c (working copy)
@@ -1056,7 +1056,7 @@ has_static_fields (const_tree type)
return false;
for (tree fld = TYPE_FIELDS (type); fld; fld = TREE_CHAIN (fld))
-if (TREE_CODE (fld) == FIELD_DECL && DECL_NAME (fld) && TREE_STATIC (fld))
+if (TREE_CODE (fld) == VAR_DECL && DECL_NAME (fld))
return true;
return false;
@@ -2635,12 +2634,12 @@ dump_nested_type (pretty_printer *buffer, tree fie
}
}
-/* Dump in BUFFER constructor spec corresponding to T. */
+/* Dump in BUFFER constructor spec corresponding to T for TYPE. */
static void
-print_constructor (pretty_printer *buffer, tree t)
+print_constructor (pretty_printer *buffer, tree t, tree type)
{
- tree decl_name = DECL_NAME (DECL_ORIGIN (t));
+ tree decl_name = DECL_NAME (TYPE_NAME (type));
pp_string (buffer, "New_");
pp_ada_tree_identifier (buffer, decl_name, t, false);
@@ -2649,9 +2648,9 @@ static void
/* Dump in BUFFER destructor spec corresponding to T. */
static void
-print_destructor (pretty_printer *buffer, tree t)
+print_destructor (pretty_printer *buffer, tree t, tree type)
{
- tree decl_name = DECL_NAME (DECL_ORIGIN (t));
+ tree decl_name = DECL_NAME (TYPE_NAME (type));
pp_string (buffer, "Delete_");
pp_ada_tree_identifier (buffer, decl_name, t, false);
@@ -2907,7 +2906,8 @@ print_ada_declaration (pretty_printer *buffer, tre
return 0;
/* Only consider constructors/destructors for complete objects. */
- if (strncmp (IDENTIFIER_POINTER (decl_name), "__comp", 6) != 0)
+ if (strncmp (IDENTIFIER_POINTER (decl_name), "__ct_comp", 9) != 0
+ && strncmp (IDENTIFIER_POINTER (decl_name), "__dt_comp", 9) != 0)
return 0;
}
@@ -2935,9 +2935,9 @@ print_ada_declaration (pretty_printer *buffer, tre
}
if (is_constructor)
- print_constructor (buffer, t);
+ print_constructor (buffer, t, type);
else if (is_destructor)
- print_destructor (buffer, t);
+ print_destructor (buffer, t, type);
else
dump_ada_decl_name (buffer, t, false);
@@ -2976,7 +2976,7 @@ print_ada_declaration (pretty_printer *buffer, tre
if (is_constructor)
{
pp_string (buffer, "pragma CPP_Constructor (");
- print_constructor (buffer, t);
+ print_constructor (buffer, t, type);
pp_string (buffer, ", \"");
pp_asm_name (buffer, t);
pp_string (buffer, "\");");
@@ -2984,7 +2984,7 @@ print_ada_declaration (pretty_printer *buffer, tre
else if (is_destructor)
{
pp_string (buffer, "pragma Import (CPP, ");
- print_destructor (buffer, t);
+ print_destructor (buffer, t, type);
pp_string (buffer, ", \"");
pp_asm_name (buffer, t);
pp_string (buffer, "\");");
@@ -3214,7 +3208,7 @@ print_ada_struct_decl (pretty_printer *buffer, tre
field_num++;
}
}
- else if (TREE_CODE (tmp) == FIELD_DECL && !TREE_STATIC (tmp))
+ else if (TREE_CODE (tmp) == FIELD_DECL)
{
/* Skip internal virtual table field. */
if (!DECL_VIRTUAL_P (tmp))
@@ -3308,9 +3302,7 @@ print_ada_struct_decl (pretty_printer *buffer, tre
/* Print the static fields of the structure, if any. */
for (tmp = TYPE_FIELDS (node); tmp; tmp = TREE_CHAIN (tmp))
{
- if (TREE_CODE (tmp) == FIELD_DECL
- && DECL_NAME (tmp)
- && TREE_STATIC (tmp))
+ if (TREE_CODE (tmp) == VAR_DECL && DECL_NAME (tmp))
{
if (need_semicolon)
{
[PATCH][Arm] Test suite failures resulting from deprecation of -mstructure-size-boundary
This patch fixes test case failures on arm targets due to '-mstructure-size-boundary' being deprecated. The test cases were failing because a warning was being issued and due to the fact that the size of packed and unpacked structures is the same after deprecating '-mstructure-size-boundary' Okay for trunk? 2017-08-04 Michael Collison * testsuite/g++.dg/ext/packed8.C: Skip test for arm_eabi. * testsuite/g++.dg/init/array16.C: Skip test for arm_eabi. * testsuite/g++.dg/other/crash-4.C: Skip test for arm_eabi. * testsuite/gcc.dg/builtin-stringop-chk-1.c: Skip test for arm_eabi. pr7519v1.patch Description: pr7519v1.patch
Re: [PATCH] Fold (int *)&a + 4 to a[1] using offset_int (PR middle-end/81695)
On Fri, Aug 4, 2017 at 2:45 AM, Marek Polacek wrote:
> On Fri, Aug 04, 2017 at 11:08:49AM +0200, Richard Biener wrote:
>> On Fri, Aug 4, 2017 at 10:38 AM, Marek Polacek wrote:
>> > We were crashing because size_binop_loc got operands of different types:
>> > sizetype and ssizetype. Richi suggested performing the computation in
>> > offset_int which my patch tries to do. Unsure about the sdiv_trunc part,
>> > what do I use instead of EXACT_DIV_EXPR?
>>
>> Use wi::divmod_trunc and check for a zero remainder. If the remainder is not
>> zero we may not fold.
>>
>> Ok with that change.
>
> Thanks, will commit this after the usual testing:
>
> 2017-08-04 Marek Polacek
>
> PR middle-end/81695
> * fold-const.c (fold_indirect_ref_1): For ((int *)&a + 4 -> a[1],
> perform the computation in offset_int.
>
> * gcc.dg/pr81695.c: New test.
>
> diff --git gcc/fold-const.c gcc/fold-const.c
> index ed6c289a64b..1f55bee8fc0 100644
> --- gcc/fold-const.c
> +++ gcc/fold-const.c
> @@ -14106,14 +14106,21 @@ fold_indirect_ref_1 (location_t loc, tree type,
> tree op0)
>&& type == TREE_TYPE (op00type))
> {
> tree type_domain = TYPE_DOMAIN (op00type);
> - tree min_val = size_zero_node;
> - if (type_domain && TYPE_MIN_VALUE (type_domain))
> - min_val = TYPE_MIN_VALUE (type_domain);
> - op01 = size_binop_loc (loc, EXACT_DIV_EXPR, op01,
> -TYPE_SIZE_UNIT (type));
> - op01 = size_binop_loc (loc, PLUS_EXPR, op01, min_val);
> - return build4_loc (loc, ARRAY_REF, type, op00, op01,
> -NULL_TREE, NULL_TREE);
> + tree min = TYPE_MIN_VALUE (type_domain);
> + if (min && TREE_CODE (min) == INTEGER_CST)
> + {
> + offset_int off = wi::to_offset (op01);
> + offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (type));
> + offset_int remainder;
> + off = wi::divmod_trunc (off, el_sz, SIGNED, &remainder);
> + if (remainder == 0)
> + {
> + off = off + wi::to_offset (min);
> + op01 = wide_int_to_tree (sizetype, off);
> + return build4_loc (loc, ARRAY_REF, type, op00, op01,
> +NULL_TREE, NULL_TREE);
> + }
> + }
> }
> }
> }
This caused:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81737
--
H.J.
