[PATCH] Update to clang-format script
GNU style brace wrapping rules have been added to clang-format quite a while ago. http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?revision=197138&view=markup This simple patch proposes to use the built-in rules instead of the custom brace rules. Ray Kim 2019-06-27 Ray Kim * contrib/clang-format: Changed the custom brace wrapping rules to built-in GNU style. diff --git a/contrib/clang-format b/contrib/clang-format index d734001c06f..3ebb85fff77 100644 --- a/contrib/clang-format +++ b/contrib/clang-format @@ -25,20 +25,8 @@ AccessModifierOffset: -2 AlwaysBreakAfterDefinitionReturnType: All BinPackArguments: true BinPackParameters: true -BraceWrapping: - AfterClass: true - AfterControlStatement: true - AfterEnum: true - AfterFunction: true - AfterNamespace: false - AfterObjCDeclaration: true - AfterStruct: true - AfterUnion: true - BeforeCatch: true - BeforeElse: true - IndentBraces: true BreakBeforeBinaryOperators: All -BreakBeforeBraces: Custom +BreakBeforeBraces: GNU BreakBeforeTernaryOperators: true ColumnLimit: 80 ConstructorInitializerIndentWidth: 2
Re: [PATCH] i386: Separate costs of RTL expressions from costs of moves
> I opened: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90952 > > We shouldn't use costs for moves for costs of RTL expressions. We can > experiment different RTL expression cost formulas. But we need to separate > costs of RTL expressions from costs for moves first. What is the best way > to partition processor_costs to avoid confusion between costs of moves vs. > costs of RTL expressions? I am still worried that splitting the cost and experimentally finding value which works well for SPEC2017 is not very reliable solution here, since the problematic decisions is not only about store cost but also about other factors. What benchmarks besides x264 are sensitive to this? Looking at x264 the problem is really simple SLP vectorization of 8 integer stores into one AVX256 store which is not a win on Core. I wrote simple microbenchmark that tests SLP vectorized versus normal store (attached). Results at skylake are: 64bit float 2 SLP:1.54 float 2 no-SLP: 1.52 float 2 def:1.55 char 8 SLP:3.35 char 8 no-SLP: 3.34 char 8 def:3.32 short 4 SLP:1.51 short 4 no-SLP: 1.51 short 4 def:1.52 int 2 SLP:1.22 int 2 no-SLP: 1.24 int 2 def:1.25 AVX126 float 4 SLP:1.51 float 4 no-SLP: 1.81 float 4 def:1.54 double 2 SLP:1.51 double 2 no-SLP: 1.53 double 2 def:1.55 char 16 SLP:6.31 char 16 no-SLP: 8.31 char 16 def:6.33 short 8 SLP:3.91 short 8 no-SLP: 3.33 short 8 def:3.92 int 4 SLP:2.12 int 4 no-SLP: 1.51 int 4 def:1.56 long long 2 SLP:1.50 long long 2 no-SLP: 1.21 long long 2 def:1.26 AVX256 float 8 SLP:2.11 float 8 no-SLP: 2.70 float 8 def:2.13 double 4 SLP:1.83 double 4 no-SLP: 1.80 double 4 def:1.82 char 32 SLP:12.72 char 32 no-SLP: 17.28 char 32 def:12.71 short 16 SLP:6.32 short 16 no-SLP: 8.77 short 16 def:6.20 int 8 SLP:3.93 int 8 no-SLP: 3.31 int 8 def:3.33 long long 4 SLP:2.13 long long 4 no-SLP: 1.52 long long 4 def:1.51 def is with cost model based decision. SLP seems bad idea for - 256 long long and int vectors (which I see are cured by your change in cost table. - doubles (little bit) - shorts for 128bit vectors (I guess that would be cured if 16bit store cost was decreased a bit like you did for int) For zen we get: 64bit float 2 SLP:2.22 float 2 no-SLP: 2.23 float 2 def:2.23 char 8 SLP:4.08 char 8 no-SLP: 4.08 char 8 def:4.08 short 4 SLP:2.22 short 4 no-SLP: 2.23 short 4 def:2.23 int 2 SLP:1.86 int 2 no-SLP: 1.87 int 2 def:1.86 AVX126 float 4 SLP:2.23 float 4 no-SLP: 2.60 float 4 def:2.23 double 2 SLP:2.23 double 2 no-SLP: 2.23 double 2 def:2.23 char 16 SLP:4.79 char 16 no-SLP: 10.03 char 16 def:4.85 short 8 SLP:3.20 short 8 no-SLP: 4.08 short 8 def:3.22 int 4 SLP:2.23 int 4 no-SLP: 2.23 int 4 def:2.23 long long 2 SLP:1.86 long long 2 no-SLP: 1.86 long long 2 def:1.87 So SLP is win in general and for buldozer 64bit float 2 SLP:2.76 float 2 no-SLP: 2.77 float 2 def:2.77 char 8 SLP:4.48 char 8 no-SLP: 4.49 char 8 def:4.48 short 4 SLP:2.84 short 4 no-SLP: 2.84 short 4 def:2.83 int 2 SLP:2.14 int 2 no-SLP: 2.13 int 2 def:2.15 AVX126 float 4 SLP:2.59 float 4 no-SLP: 3.07 float 4 def:2.59 double 2 SLP:2.48 double 2 no-SLP: 2.49 double 2 def:2.48 char 16 SLP:30.33 char 16 no-SLP: 11.72 char 16 def:30.30 short 8 SLP:21.04 short 8 no-SLP: 4.62 short 8 def:21.06 int 4 SLP:4.29 int 4 no-SLP: 2
Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
... hi again ;)
The other day I was having a look at using declarations for this issue
and noticed that only a few lines below the de-virtualization check we
have to handle functions found by a using declaration, for various
reasons. In particular, we know whether we found a function fn where has
been declared or in a derived class. Thus the idea: for the purpose of
making some progress, in particular all the cases in c++/67184 & co,
would it make sense for the time being to simply add a check to the
de-virtualization condition restricting it to non-using declarations?
See the below (it also moves the conditional a few lines below only for
clarity and consistency with the code handling using declarations, no
functional impact) What do you think?
Thanks, Paolo.
///
Index: cp/call.c
===
--- cp/call.c (revision 272583)
+++ cp/call.c (working copy)
@@ -8241,12 +8241,6 @@ build_over_call (struct z_candidate *cand, int fla
return error_mark_node;
}
- /* See if the function member or the whole class type is declared
-final and the call can be devirtualized. */
- if (DECL_FINAL_P (fn)
- || CLASSTYPE_FINAL (TYPE_METHOD_BASETYPE (TREE_TYPE (fn
- flags |= LOOKUP_NONVIRTUAL;
-
/* [class.mfct.nonstatic]: If a nonstatic member function of a class
X is called for an object that is not of type X, or of a type
derived from X, the behavior is undefined.
@@ -8271,6 +8265,17 @@ build_over_call (struct z_candidate *cand, int fla
else
return error_mark_node;
}
+
+ /* See if the function member or the whole class type is declared
+final and the call can be devirtualized. */
+ if (DECL_FINAL_P (fn)
+ || (CLASSTYPE_FINAL (TREE_TYPE (argtype))
+ /* Give up for now if fn was found by a using declaration,
+the complex case, see c++/90909. */
+ && (TREE_TYPE (TREE_TYPE (converted_arg))
+ == TREE_TYPE (parmtype
+ flags |= LOOKUP_NONVIRTUAL;
+
/* If fn was found by a using declaration, the conversion path
will be to the derived class, not the base declaring fn. We
must convert from derived to base. */
Index: testsuite/g++.dg/other/final3.C
===
--- testsuite/g++.dg/other/final3.C (nonexistent)
+++ testsuite/g++.dg/other/final3.C (working copy)
@@ -0,0 +1,28 @@
+// PR c++/67184
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+
+struct V {
+ virtual void foo();
+};
+
+struct wV final : V {
+};
+
+struct oV final : V {
+ void foo();
+};
+
+void call(wV& x)
+{
+ x.foo();
+ x.V::foo();
+}
+
+void call(oV& x)
+{
+ x.foo();
+ x.V::foo();
+}
+
+// { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "original" } }
Index: testsuite/g++.dg/other/final4.C
===
--- testsuite/g++.dg/other/final4.C (nonexistent)
+++ testsuite/g++.dg/other/final4.C (working copy)
@@ -0,0 +1,16 @@
+// PR c++/67184
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+
+struct B
+{
+ virtual void operator()();
+ virtual operator int();
+ virtual int operator++();
+};
+
+struct D final : B { };
+
+void foo(D& d) { d(); int t = d; ++d; }
+
+// { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "original" } }
Index: testsuite/g++.dg/other/final5.C
===
--- testsuite/g++.dg/other/final5.C (nonexistent)
+++ testsuite/g++.dg/other/final5.C (working copy)
@@ -0,0 +1,19 @@
+// PR c++/69445
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+
+struct Base {
+ virtual void foo() const = 0;
+ virtual void bar() const {}
+};
+
+struct C final : Base {
+ void foo() const { }
+};
+
+void func(const C & c) {
+ c.bar();
+ c.foo();
+}
+
+// { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "original" } }
[C++ Patch] A few additional location improvements to grokdeclarator and check_tag_decl
Hi,
here there are a couple of rather straightforward improvements in the
second half of grokdeclarator plus a check_tag_decl change consistent
with the other existing case of multiple_types_p diagnostic. Tested
x86_64-linux.
Thanks, Paolo.
/
/cp
2019-06-23 Paolo Carlini
* decl.c (get_type_quals): New.
(check_tag_decl): Use smallest_type_location and the latter in
error_at about multiple types in one declaration.
(grokdeclarator): Use locations[ds_storage_class] in error_at
about static cdtor; use id_loc in error_at about flexible
array member in union; use get_type_quals.
/testsuite
2019-06-23 Paolo Carlini
* g++.dg/diagnostic/static-cdtor-1.C: New.
* g++.dg/cpp1z/has-unique-obj-representations2.C: Test location
too.
* g++.dg/other/anon-union3.C: Adjust expected location.
* g++.dg/parse/error8.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 272584)
+++ cp/decl.c (working copy)
@@ -100,6 +100,7 @@ static tree build_cp_library_fn (tree, enum tree_c
static void store_parm_decls (tree);
static void initialize_local_var (tree, tree);
static void expand_static_init (tree, tree);
+static location_t smallest_type_location (int, const location_t*);
/* The following symbols are subsumed in the cp_global_trees array, and
listed here individually for documentation purposes.
@@ -4802,6 +4803,24 @@ warn_misplaced_attr_for_class_type (location_t loc
class_type, class_key_or_enum_as_string (class_type));
}
+/* Returns the cv-qualifiers that apply to the type specified
+ by the DECLSPECS. */
+
+static int
+get_type_quals (const cp_decl_specifier_seq *declspecs)
+{
+ int type_quals = TYPE_UNQUALIFIED;
+
+ if (decl_spec_seq_has_spec_p (declspecs, ds_const))
+type_quals |= TYPE_QUAL_CONST;
+ if (decl_spec_seq_has_spec_p (declspecs, ds_volatile))
+type_quals |= TYPE_QUAL_VOLATILE;
+ if (decl_spec_seq_has_spec_p (declspecs, ds_restrict))
+type_quals |= TYPE_QUAL_RESTRICT;
+
+ return type_quals;
+}
+
/* Make sure that a declaration with no declarator is well-formed, i.e.
just declares a tagged type or anonymous union.
@@ -4821,7 +4840,9 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
bool error_p = false;
if (declspecs->multiple_types_p)
-error ("multiple types in one declaration");
+error_at (smallest_type_location (get_type_quals (declspecs),
+ declspecs->locations),
+ "multiple types in one declaration");
else if (declspecs->redefined_builtin_type)
{
if (!in_system_header_at (input_location))
@@ -10403,7 +10424,7 @@ grokdeclarator (const cp_declarator *declarator,
a member function. */
cp_ref_qualifier rqual = REF_QUAL_NONE;
/* cv-qualifiers that apply to the type specified by the DECLSPECS. */
- int type_quals = TYPE_UNQUALIFIED;
+ int type_quals = get_type_quals (declspecs);
tree raises = NULL_TREE;
int template_count = 0;
tree returned_attrs = NULL_TREE;
@@ -10449,13 +10470,6 @@ grokdeclarator (const cp_declarator *declarator,
if (concept_p)
constexpr_p = true;
- if (decl_spec_seq_has_spec_p (declspecs, ds_const))
-type_quals |= TYPE_QUAL_CONST;
- if (decl_spec_seq_has_spec_p (declspecs, ds_volatile))
-type_quals |= TYPE_QUAL_VOLATILE;
- if (decl_spec_seq_has_spec_p (declspecs, ds_restrict))
-type_quals |= TYPE_QUAL_RESTRICT;
-
if (decl_context == FUNCDEF)
funcdef_flag = true, decl_context = NORMAL;
else if (decl_context == MEMFUNCDEF)
@@ -11571,9 +11585,12 @@ grokdeclarator (const cp_declarator *declarator,
virtual. A constructor may not be static.
A constructor may not be declared with ref-qualifier. */
if (staticp == 2)
- error ((flags == DTOR_FLAG)
-? G_("destructor cannot be static member function")
-: G_("constructor cannot be static member function"));
+ error_at (declspecs->locations[ds_storage_class],
+ (flags == DTOR_FLAG)
+ ? G_("destructor cannot be static member "
+"function")
+ : G_("constructor cannot be static member "
+"function"));
if (memfn_quals)
{
error ((flags == DTOR_FLAG)
@@ -12431,7 +12448,7 @@ grokdeclarator (const cp_declarator *declarator,
&& (TREE_CODE (ctype) == UNION_TYPE
|| TREE_CODE (ctype) == QUAL_UNION_TYPE))
{
- error ("flexible array member in union");
+ error_at (id_loc, "flexible array member in union");
type = error_mark_node;
}
else
Index: testsu
[PATCH 0/3] RFC: Let debug stmts influence codegen at -Og
-Og is documented as:
@option{-Og} should be the optimization
level of choice for the standard edit-compile-debug cycle, offering
a reasonable level of optimization while maintaining fast compilation
and a good debugging experience. It is a better choice than @option{-O0}
for producing debuggable code because some compiler passes
that collect debug information are disabled at @option{-O0}.
One of the things hampering that is that, as for the "normal" -O* flags,
the code produced by -Og -g must be the same as the code produced
without any debug IL at all. There are many cases in which that makes
it impossible to stop useful values from being optimised out of the
debug info, either because the value simply isn't available at runtime
at the point that the debugger needs it, or because of limitations in
the debug representation. (E.g. pointers to external data are dropped
from debug info because the relocations wouldn't be honoured.)
I think it would be better to flip things around so that the debug IL
is always present when optimising at -Og, and then allow the debug IL
to influence codegen at -Og. This still honours the -fcompare-debug
principle, and the compile speed of -Og without -g doesn't seem very
important.
This series therefore adds a mode in which debug stmts and debug insns
are present even without -g and are explicitly allowed to affect codegen.
In particular, when this mode is active:
- uses in debug binds become first-class uses, acting like uses in
executable code
- the use of DEBUG_EXPR_DECLs is banned. If we want to refer to
a temporary value in debug binds, we need to calculate the value
with executable code instead
This needs a new term to distinguish stmts/insns that affect codegen
from those that don't. I couldn't think of one that I was really
happy with, but possibilities included:
tangible/shadow
manifest/hidden
foreground/background
reactive/inert
active/dormant (but "active insn" already means something else)
peppy/sullen
The series uses tangible/shadow. There's a new global flag_tangible_debug
that controls whether debug insns are "tangible" insns (for the new mode)
or "shadow" insns (for normal optimisation). -Og enables the new mode
while the other optimisation levels leave it off. (Despite the name,
the new variable is just an internal flag, there's no -ftangible-debug
option.)
The first patch adds the infrastructure but doesn't improve the debug
experience much on its own.
As an example of one thing we can do with the new mode, the second patch
ensures that the gimple IL has debug info for each is_gimple_reg variable
throughout the variable's lifetime. This fixes a couple of the PRs in
the -Og meta-bug and from spot-testing seems to ensure that far fewer
values are optimised out.
Also, the new mode is mostly orthogonal to the optimisation level
(although it would in effect disable optimisations like loop
vectorisation, until we have a way of representing debug info for
vectorised loops). The third patch therefore adds an -O1g option
that optimises more heavily than -Og but provides a better debug
experience than -O1.
I think -O2g would make sense too, and would be a viable option
for people who want to deploy relatively heavily optimised binaries
without compromising the debug experience too much.
Other possible follow-ons for the new mode include:
- Make sure that tangible debug stmts never read memory or take
an address. (This is so that addressability and vops depend
only on non-debug insns.)
- Fall back on expanding real code if expand_debug_expr fails.
- Force debug insns to be simple enough for dwarf2out (e.g. no external
or TLS symbols). This could be done by having a validation step for
debug insns, like we already do for normal insns.
- Prevent the removal of dead stores if it would lead to wrong debug info.
(Maybe under control of an option?)
To get an idea of the runtime cost, I tried compiling tree-into-ssa.ii
at -O2 -g with various --enable-checking=yes builds of cc1plus:
time taken
cc1plus compiled with -O0: 100.00% (baseline)
cc1plus compiled with old -Og: 30.94%
cc1plus compiled with new -Og: 31.82%
cc1plus compiled with -O1g: 28.22%
cc1plus compiled with -O1: 26.72%
cc1plus compiled with -O2: 25.15%
So there is a noticeable but small performance cost to the new mode.
To get an idea of the compile-time impact, I tried compiling
tree-into-ssa.ii at various optimisation levels, all using the
same --enable-checking=release bootstrap build:
time taken
tree-into-ssa.ii with -O0 -g: 100.0% (baseline)
tree-into-ssa.ii with old -Og -g: 180.6%
tree-into-ssa.ii with new -Og -g: 198.2%
tree-into-ssa.ii with -O1g -g:237.1%
tree-into-ssa.ii with -O1 -g: 211.8%
tree-into-ssa.ii with -O2 -g: 331.5%
So there's definitely a bit of a compile-time hit. I haven't yet looked
at how easy it would be
[PATCH 1/3] RFC: Add a mode in which debug stmts participate in codegen
This patch adds a mode in which debug stmts and debug insns are explicitly allowed to affect codegen. In particular: - uses in debug binds become first-class uses, acting like uses in executable code - the use of DEBUG_EXPR_DECLs is banned. If we want to refer to a temporary value in debug binds, we need to calculate the value with executable code instead The idea is that this mode can then take the quality of the debug experience into account when optimising. The flipside is that it needs to generate debug insns even if -g isn't passed, so that -g continues to have no effect on codegen. (That shouldn't be a significant problem though. There's not much point using this mode unless you intend to use -g most/all of the time, so the compile-time impact without -g shouldn't be important.) The patch calls stmts and insns "tangible" stmts/insns if they can effect codegen and "shadow" stmts/insns otherwise. A new global, flag_tangible_debug, controls which category debug binds fall into. They're tangible stmts/insns for the new mode and shadow stmts/insns otherwise. (Despite the name, flag_tangible_debug is just an internal flag, there's no -ftangible-debug option.) The main part of the patch involves replacing tests for debug/nondebug with tests for shadow/tangible where appropriate. The patch is far from complete; it just handles the cases needed by -Og and -O1g bootstraps (see later patch for -O1g). On its own the mode doesn't help many cases, although the patch includes one token test. The patch is really just adding infrastructure for later patches. I added FIXMEs to rtlanal.c with the intention of renaming the functions in a separate patch. 2019-06-23 Richard Sandiford config/ * bootstrap-Og.mk: New file. gcc/ * common.opt (flag_tangible_debug): New variable. * opts.c (default_options_optimization): Initialize it as appropriate for each -O option. * toplev.c (process_options): Don't test for -g when choosing the default flag_var_tracking setting for flag_tangible_debug. * tree.h (MAY_HAVE_SHADOW_DEBUG_BIND_STMTS): New macro. * gimple.h (tangible_stmt_p): New function. * tree-ssa-scopedtables.c: Include options.h. * rtl.h (MAY_HAVE_SHADOW_DEBUG_BIND_INSNS): New macro. (tangible_insn_p, shadow_insn_p): New functions. * tree.c (make_node): Forbid the use of DEBUG_DECL_EXPR when flag_tangible_debug. * tree-ssa.c (insert_debug_temp_for_var_def): Don't handle debug uses of SSA_NAMEs here if flag_tangible_debug. * tree-ssanames.c (release_ssa_name_fn): Likewise. * ssa-iterators.h (has_zero_uses): Don't skip tangible debug insns. (has_single_use, single_imm_use): Likewise. (num_imm_uses): Test MAY_HAVE_SHADOW_DEBUG_BIND_STMTS instead of MAY_HAVE_DEBUG_BIND_STMTS when deciding whether to ignore uses in debug bind statements. * tree-ssa-operands.c (single_imm_use_1): Don't skip tangible debug stmts. * emit-rtl.c (next_nondebug_insn, prev_nondebug_insn) (next_nonnote_nondebug_insn, next_nonnote_nondebug_insn_bb) (prev_nonnote_nondebug_insn, prev_nonnote_nondebug_insn_bb) (next_real_nondebug_insn, prev_real_nondebug_insn): Don't skip over tangible debug insns. Add FIXME note that the functions should be renamed accordingly. (emit_pattern_after, emit_pattern_before): Replace "skip_debug_insns" parameter with "skip_shadow_insns". * cfgexpand.c (avoid_deep_ter_for_debug): For flag_tangible_debug, use the same depth limit as avoid_complex_debug_insns and undo the replacement when the depth is reached. (expand_debug_locations): Don't call avoid_complex_debug_insns. (expand_gimple_basic_block): Change the condition under which debug uses can occur after the last "real" use of a TER variable from MAY_HAVE_DEBUG_BIND_STMTS to MAY_HAVE_SHADOW_DEBUG_BIND_STMTS. Don't delink uses of tangible debug stmts here. * combine.c (find_single_use, create_log_links, combine_instructions): (try_combine, distribute_notes, distribute_links): Handle tangible debug insns like normal insns. Make special handling of debug uses conditional on MAY_HAVE_SHADOW_DEBUG_BIND_INSNS rather than MAY_HAVE_DEBUG_BIND_INSNS. (make_more_copies): Test NONJUMP_INSN_P rather than NONDEBUG_INSN_P. (cant_combine_insn_p): Explain why we still test NONDEBUG_INSN_P. * cse.c (try_back_substitute_reg, cse_extended_basic_block) (count_reg_usage, delete_trivially_dead_insns): Handle tangible debug insns like normal insns. Make special handling of debug uses conditional on MAY_HAVE_SHADOW_DEBUG_BIND_INSNS rather than MAY_HAVE_DEBUG_BIND_INSN. * dce.c (reset_unmarked_insns_debug_uses, prescan_insns_for_dce) (mark_reg_
Re: [PATCH 0/3] RFC: Keep debug values live for -Og
This patch tries to make sure that, when optimising at -Og, the gimple
IL has appropriate debug information for the whole of a variable's scope.
It does the same for parameters in functions that always return (but still
misses cases in functions that don't return -- a TODO for later).
The idea is to emit a debug stmt that uses each gimple register "var"
when it goes out of scope. With the new "tangible" debug stmts, this
forces SSA renaming of "var" for its entire lifetime, even after the
last real use. This in turn means that we should enter rtl in a state
where, at any given point in a variable's scope, there is a single
dominating debug bind insn for it.
A side-effect of this is that a variable that is in-scope but
uninitialised and unused will give "0" rather than ""
(where the zero is guaranteed by init-regs). E.g. for functions like:
static inline void
get_conflict_and_start_profitable_regs (ira_allocno_t a, bool retry_p,
HARD_REG_SET *conflict_regs,
HARD_REG_SET *start_profitable_regs)
{
int i, nwords;
ira_object_t obj;
nwords = ALLOCNO_NUM_OBJECTS (a);
for (i = 0; i < nwords; i++)
{
obj = ALLOCNO_OBJECT (a, i);
COPY_HARD_REG_SET (conflict_regs[i],
OBJECT_TOTAL_CONFLICT_HARD_REGS (obj));
}
if (retry_p)
{
COPY_HARD_REG_SET (*start_profitable_regs,
reg_class_contents[ALLOCNO_CLASS (a)]);
AND_COMPL_HARD_REG_SET (*start_profitable_regs,
ira_prohibited_class_mode_regs
[ALLOCNO_CLASS (a)][ALLOCNO_MODE (a)]);
}
else
COPY_HARD_REG_SET (*start_profitable_regs,
ALLOCNO_COLOR_DATA (a)->profitable_hard_regs);
}
it is still possible to print "obj" after the loop. If nwords is <= 0
(which it never is in practice), "obj" would print as zero.
IMO this isn't wrong debug info, because the value that we print for
"obj" is the value that a printf at that location would print, even in
the uninitialised/zero case. And this kind of for loop is very common:
the program in practice guarantees that the loop executes at least once,
and thus guarantees that "obj" has a useful value, but the compiler
doesn't know that.
The patch uses debug insns of the form:
# DEBUG NULL => var
to keep "var" live. Binding to null seemed better than binding to "var",
which is about to go out of scope and doesn't actually change value here.
Binding to null also means that we can delete the stmt if no longer has
any SSA uses (e.g. due to fwprop). Most of the patch is just adding
support for null debug bind vars.
Although this should ensure we have good-quality debug info for gimple
registers when entering rtl, it doesn't guarantee that the value bound
in a debug bind insn remains live for as long as the debug bind is in effect.
That's a separate patch.
It's also possible that rtl cfg manipulations could break the original
property of having one dominating debug bind at each point. That too
is a separate patch. It probably makes sense to fix both of these
problems immediately before IRA.
The patch fixes the referenced PRs. In particular, the backtrace for
PR78685 is now:
#0 call_debugger (x=x@entry=3) at pr78685.c:6
#1 ... in apply_lambda (fun=fun@entry=1, args=args@entry=2,
count=count@entry=3) at pr78685.c:14
#2 ... in main (argc=1, argv=0x) at pr78685.c:22
instead of:
#0 call_debugger (x=3) at pr78685.c:6
#1 ... in apply_lambda (fun=1, args=2, count=) at pr78685.c:14
#2 ... in main (argc=, argv=) at pr78685.c:22
2019-06-23 Richard Sandiford
gcc/
PR debug/78685
PR debug/88730
* gimplify.c (gimplify_keep_decl_live): New function.
(gimplify_bind_expr, gimplify_return_expr): Use it.
* function.c (gimplify_parameters): Add debug stmts for parameters
if flag_tangible_debug.
* var-tracking.c (use_type): Drop debug binds with null variables.
(delete_vta_debug_insn): Likewise.
* gimple-low.c (lower_stmt): Allow debug binds too.
* cfgexpand.c (expand_debug_locations): Handle null debug bind vars.
(expand_gimple_basic_block): Likewise.
* cfgrtl.c (duplicate_insn_chain): Likewise.
* cse.c (insn_live_p): Likewise.
* omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
* print-rtl.c (rtx_writer::print_rtx, print_insn): Likewise.
* tree-cfg.c (make_blocks_1, stmt_starts_bb_p): Handle tangible
debug stmts like normal stmts.
(make_blocks): Check for marker stmts rather than asserting for them.
(gimple_duplicate_bb): Handle null debug bind vars.
* tree-inline.c (maybe_move_debug_stmts_to_successors): Likewise.
(copy_debug_stmt): Likewise.
* tree-parloops.c (separate_decls_in_region_debug): Likewise.
* tree-ssa-threadedge.c (propagate_threaded_block_debug_into
[PATCH 3/3] RFC: Add -O1g
This patch adds an option called -O1g, a half-way house between
-Og and -O1 in terms of both performance and debuggability.
2019-06-23 Richard Sandiford
config/
* bootstrap-O1g.mk: New file.
gcc/
* common.opt (O1g): New option.
* doc/invoke.texi: Document it.
* opts.c (default_options_optimization): Handle it.
(common_handle_option): Likewise.
gcc/testsuite/
* g++.dg/guality/guality.exp: Add "-O1g -g" to the list of options
to try for Og-* tests.
* gcc.dg/guality/guality.exp: Likewise.
* gcc.dg/sso/sso.exp (SSO_TORTURE_OPTIONS): Add "-O1g -g".
* gnat.dg/sso/sso.exp (SSO_TORTURE_OPTIONS): Likewise.
* lib/c-torture.exp (TORTURE_OPTIONS): Likewise.
Index: config/bootstrap-O1g.mk
===
--- /dev/null 2019-06-14 15:59:19.298479944 +0100
+++ config/bootstrap-O1g.mk 2019-06-23 14:48:59.302941670 +0100
@@ -0,0 +1,1 @@
+BOOT_CFLAGS := -O1g $(filter-out -O%, $(BOOT_CFLAGS))
Index: gcc/common.opt
===
--- gcc/common.opt 2019-06-23 14:48:44.495065059 +0100
+++ gcc/common.opt 2019-06-23 14:48:59.302941670 +0100
@@ -487,6 +487,9 @@ Og
Common Optimization
Optimize for debugging experience rather than speed or size.
+O1g
+Common Optimization
+
Q
Driver
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi 2019-06-18 09:35:52.089892867 +0100
+++ gcc/doc/invoke.texi 2019-06-23 14:48:59.314941570 +0100
@@ -8403,6 +8403,19 @@ optimization flags except for those that
-fmove-loop-invariants -fssa-phiopt @gol
-ftree-bit-ccp -ftree-pta -ftree-sra}
+@item -O1g
+@opindex O1g
+Perform the same optimizations as @option{-O1}, but take the debug
+experience into consideration when optimizing. In particular, try to
+make sure that the values of simple variables are printable at any
+point during their scope.
+
+This option is a compromise between @option{-Og} and @option{-O1}.
+The code it produces is generally faster than the code that
+@option{-Og} produces, but not as fast as the code that @option{-O1}
+produces. In contrast, the code that @option{-O1g} produces should be
+easier to debug than code that @option{-O1} produces, but might not be
+as easy to debug as the code that @option{-Og} produces.
@end table
If you use multiple @option{-O} options, with or without level numbers,
Index: gcc/opts.c
===
--- gcc/opts.c 2019-06-23 14:48:44.507064959 +0100
+++ gcc/opts.c 2019-06-23 14:48:59.314941570 +0100
@@ -639,6 +639,14 @@ default_options_optimization (struct gcc
opts->x_flag_tangible_debug = 1;
break;
+ case OPT_O1g:
+ opts->x_optimize_size = 0;
+ opts->x_optimize = 1;
+ opts->x_optimize_fast = 0;
+ opts->x_optimize_debug = 0;
+ opts->x_flag_tangible_debug = 1;
+ break;
+
case OPT_fopenacc:
if (opt->value)
openacc_mode = true;
@@ -2323,6 +2331,7 @@ common_handle_option (struct gcc_options
case OPT_Os:
case OPT_Ofast:
case OPT_Og:
+case OPT_O1g:
/* Currently handled in a prescan. */
break;
Index: gcc/testsuite/g++.dg/guality/guality.exp
===
--- gcc/testsuite/g++.dg/guality/guality.exp2019-06-23 14:48:44.511064925
+0100
+++ gcc/testsuite/g++.dg/guality/guality.exp2019-06-23 14:48:59.314941570
+0100
@@ -78,9 +78,9 @@ if {[check_guality "
gcc-dg-runtest [lsort [glob $srcdir/$subdir/*.C]] "" ""
gcc-dg-runtest $general "" ""
set-torture-options \
- [list "-O0" "-Og"] \
+ [list "-O0" "-Og" "-O1g"] \
[list {}] \
- [list "-Og -flto"]
+ [list "-Og -flto" "-O1g -flto"]
gcc-dg-runtest $Og "" ""
}
Index: gcc/testsuite/gcc.dg/guality/guality.exp
===
--- gcc/testsuite/gcc.dg/guality/guality.exp2019-06-23 14:48:44.511064925
+0100
+++ gcc/testsuite/gcc.dg/guality/guality.exp2019-06-23 14:48:59.314941570
+0100
@@ -93,9 +93,9 @@ if {[check_guality "
gcc-dg-runtest [lsort [glob $srcdir/$subdir/*.c]] "" ""
gcc-dg-runtest $general "" "-Wc++-compat"
set-torture-options \
- [list "-O0" "-Og"] \
+ [list "-O0" "-Og" "-O1g"] \
[list {}] \
- [list "-Og -flto"]
+ [list "-Og -flto" "-O1g -flto"]
gcc-dg-runtest $Og "" "-Wc++-compat"
}
Index: gcc/testsuite/gcc.dg/sso/sso.exp
===
--- gcc/testsuite/gcc.dg/sso/sso.exp2019-03-08 18:15:04.412863081 +
+++ gcc/testsuite/gcc.dg/sso/sso.exp2019-06-23 14:48:59.314941570 +0100
@@ -32,7 +32,8 @@ set SSO_TORTURE_OPTIONS [list \
{ -O2 } \
{ -O3 -finline-functions } \
Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
On 6/23/19 7:53 AM, Paolo Carlini wrote: ... hi again ;) The other day I was having a look at using declarations for this issue and noticed that only a few lines below the de-virtualization check we have to handle functions found by a using declaration, for various reasons. In particular, we know whether we found a function fn where has been declared or in a derived class. Thus the idea: for the purpose of making some progress, in particular all the cases in c++/67184 & co, would it make sense for the time being to simply add a check to the de-virtualization condition restricting it to non-using declarations? See the below (it also moves the conditional a few lines below only for clarity and consistency with the code handling using declarations, no functional impact) What do you think? Hmm, perhaps we should check CLASSTYPE_FINAL in resolves_to_fixed_type_p rather than in build_over_call at all; then the code in build_new_method_call ought to set LOOKUP_NONVIRTUAL when appropriate. Jason
Re: [PATCH] [ARC] Fix PR89838
* Claudiu Zissulescu [2019-06-06 10:32:11 +0300]:
> Hi Andrew,
>
> This is a proposed fix for bugzilla PR89838 issue. It also needs to be
> backported to gcc9 and, eventually, gcc8 branches.
>
> Ok to apply?
> Claudiu
I had a look through the patch and found just one small nit detailed
below. Otherwise this looks fine.
Thanks,
Andrew
>
> gcc/
> -xx-xx Claudiu Zissulescu
>
> * config/arc/arc.c (arc_symbol_binds_local_p): New function.
> (arc_legitimize_pic_address): Simplify and cleanup the function.
> (SYMBOLIC_CONST): Remove.
> (prepare_pic_move): Likewise.
> (prepare_move_operands): Handle complex mov cases here.
> (arc_legitimize_address_0): Remove call to
> arc_legitimize_pic_address.
> (arc_legitimize_address): Remove call to
> arc_legitimize_tls_address.
> * config/arc/arc.md (movqi_insn): Allow Cm3 match.
> (movhi_insn): Likewise.
>
> /gcc/testsuite
> -xx-xx Claudiu Zissulescu
>
> * gcc.target/arc/pr89838.c: New file.
> ---
> gcc/config/arc/arc.c | 215 +++--
> gcc/config/arc/arc.md | 8 +-
> gcc/testsuite/gcc.target/arc/pr89838.c | 16 ++
> 3 files changed, 77 insertions(+), 162 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/arc/pr89838.c
>
> diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> index 20dfae66370..a5ee5c49a8f 100644
> --- a/gcc/config/arc/arc.c
> +++ b/gcc/config/arc/arc.c
> @@ -5986,130 +5986,47 @@ arc_legitimize_tls_address (rtx addr, enum tls_model
> model)
> }
> }
>
> -/* Legitimize a pic address reference in ORIG.
> - The return value is the legitimated address.
> - If OLDX is non-zero, it is the target to assign the address to first. */
> +/* Return true if SYMBOL_REF X binds locally. */
>
> -static rtx
> -arc_legitimize_pic_address (rtx orig, rtx oldx)
> +static bool
> +arc_symbol_binds_local_p (const_rtx x)
> {
> - rtx addr = orig;
> - rtx pat = orig;
> - rtx base;
> + return (SYMBOL_REF_DECL (x)
> + ? targetm.binds_local_p (SYMBOL_REF_DECL (x))
> + : SYMBOL_REF_LOCAL_P (x));
> +}
> +
> +/* Legitimize a pic address reference in ORIG. The return value is
> + the legitimated address. */
This comment needs updating I think, it references ORIG, but there is
no ORIG in arc_legitimize_pic.
>
> - if (oldx == orig)
> -oldx = NULL;
> +static rtx
> +arc_legitimize_pic_address (rtx addr)
> +{
> + if (!flag_pic)
> +return addr;
>
> - if (GET_CODE (addr) == LABEL_REF)
> -; /* Do nothing. */
> - else if (GET_CODE (addr) == SYMBOL_REF)
> + switch (GET_CODE (addr))
> {
> - enum tls_model model = SYMBOL_REF_TLS_MODEL (addr);
> - if (model != 0)
> - return arc_legitimize_tls_address (addr, model);
> - else if (!flag_pic)
> - return orig;
> - else if (CONSTANT_POOL_ADDRESS_P (addr) || SYMBOL_REF_LOCAL_P (addr))
> - return arc_unspec_offset (addr, ARC_UNSPEC_GOTOFFPC);
> +case SYMBOL_REF:
> + /* TLS symbols are handled in different place. */
> + if (SYMBOL_REF_TLS_MODEL (addr))
> + return addr;
>
>/* This symbol must be referenced via a load from the Global
>Offset Table (@GOTPC). */
> - pat = arc_unspec_offset (addr, ARC_UNSPEC_GOT);
> - pat = gen_const_mem (Pmode, pat);
> -
> - if (oldx == NULL)
> - oldx = gen_reg_rtx (Pmode);
> -
> - emit_move_insn (oldx, pat);
> - pat = oldx;
> -}
> - else
> -{
> - if (GET_CODE (addr) == CONST)
> - {
> - addr = XEXP (addr, 0);
> - if (GET_CODE (addr) == UNSPEC)
> - {
> - /* Check that the unspec is one of the ones we generate? */
> - return orig;
> - }
> - /* fwprop is placing in the REG_EQUIV notes constant pic
> - unspecs expressions. Then, loop may use these notes for
> - optimizations resulting in complex patterns that are not
> - supported by the current implementation. The following
> - two if-cases are simplifying the complex patters to
> - simpler ones. */
> - else if (GET_CODE (addr) == MINUS)
> - {
> - rtx op0 = XEXP (addr, 0);
> - rtx op1 = XEXP (addr, 1);
> - gcc_assert (oldx);
> - gcc_assert (GET_CODE (op1) == UNSPEC);
> -
> - emit_move_insn (oldx,
> - gen_rtx_CONST (SImode,
> - arc_legitimize_pic_address (op1,
> -
> NULL_RTX)));
> - emit_insn (gen_rtx_SET (oldx, gen_rtx_MINUS (SImode, op0, oldx)));
> - return oldx;
> -
> - }
> - else if (GET_CODE (addr) != PLUS)
> - {
> - rtx tmp = XEXP (addr, 0);
> - enum rtx_code code = GET_CODE (addr);
> -
> - /* It only works for UNARY operations. */
> - gcc_assert (UNARY_
[Darwin, PPC, testsuite, committed] Fix pr71785 testcase for Darwin.
The test was failing with false positive for the scan-assembler-not.
Firstly, we adjust the test conditions to use non-PIC code for Darwin.
Secondly, we have to account for out-of-line GPR restores which gives
a false positive on the scan-assembler-not. Lastly, we make the test a
bit more specific for Darwin - that it looks for absence of branches to
local labels.
Tested on powerpc-darwin9, powerpc-linux-gnu
applied to mainline
thanks,
Iain
2019-06-23 Iain Sandoe
* gcc.target/powerpc/pr71785.c: For Darwin, make test non-PIC,
expect the out-of-line GPR restore, and test specifically for
absence of branches to local labels.
diff --git a/gcc/testsuite/gcc.target/powerpc/pr71785.c
b/gcc/testsuite/gcc.target/powerpc/pr71785.c
index 613dcd1..c667ad8 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr71785.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr71785.c
@@ -1,6 +1,11 @@
/* { dg-do compile { target { powerpc*-*-* } } } */
/* { dg-options "-O2" } */
-/* { dg-final { scan-assembler-not {\mb\M} } } */
+/* We have to lose the default pic codegen on Darwin. */
+/* { dg-additional-options "-mdynamic-no-pic" { target powerpc*-*-darwin* } }
*/
+/* ... and account for the out-of-line GPR restore. */
+/* { dg-final { scan-assembler-times {\mb[ \t]*restGPR} 1 { target
powerpc*-*-darwin* } } } */
+/* { dg-final { scan-assembler-not {\mb[ \t]L} { target powerpc*-*-darwin* } }
} */
+/* { dg-final { scan-assembler-not {\mb\M} { target { ! powerpc*-*-darwin* } }
} } */
/* Check that all computed gotos in this testcase end up unfactored completely.
If some is not there will be a unconditional jump left; if all works fine,
[Darwin, PPC, committed] Emit uppercase versions of ppc defines.
Emit __PPC__ (32b) and __PPC64__ (64bit) as per the other members
of the PowerPC port. Darwin has always emitted __ppc__ and __ppc64__
but some testcases rely on the upper case variants.
tested on powerpc-darwin9, applied to mainline.
thanks
Iain
2019-06-23 Iain Sandoe
* config/rs6000/darwin.h: (__PPC__, __PPC64__): New.
diff --git a/gcc/config/rs6000/darwin.h b/gcc/config/rs6000/darwin.h
index ee949f2..2df617d 100644
--- a/gcc/config/rs6000/darwin.h
+++ b/gcc/config/rs6000/darwin.h
@@ -54,7 +54,9 @@
do \
{ \
if (!TARGET_64BIT) builtin_define ("__ppc__"); \
+ if (!TARGET_64BIT) builtin_define ("__PPC__"); \
if (TARGET_64BIT) builtin_define ("__ppc64__"); \
+ if (TARGET_64BIT) builtin_define ("__PPC64__"); \
builtin_define ("__POWERPC__"); \
builtin_define ("__NATURAL_ALIGNMENT__");\
darwin_cpp_builtins (pfile); \
[Darwin, PPC, committed] Handle GCC target pragma.
Primarily, for compatibility with other members of the port.
Note, that we do not handle the longcall attribute, since longcall is not
required/used on current Darwin (supported only for legacy cases).
2019-06-23 Iain Sandoe
* config/rs6000/darwin.h: Handle GCC target pragma.
diff --git a/gcc/config/rs6000/darwin.h b/gcc/config/rs6000/darwin.h
index 2df617d..705dd7f 100644
--- a/gcc/config/rs6000/darwin.h
+++ b/gcc/config/rs6000/darwin.h
@@ -400,6 +400,7 @@ extern int darwin_emit_picsym_stub;
do \
{ \
DARWIN_REGISTER_TARGET_PRAGMAS(); \
+ targetm.target_option.pragma_parse = rs6000_pragma_target_parse; \
targetm.resolve_overloaded_builtin = altivec_resolve_overloaded_builtin;
\
} \
while (0)
[Darwin, PPC, testsuite, committed] Fix builtins-1 testcase for Darwin.
The test needs to account for Darwin's __USER_LABEL_PREFIX__.
tested on powerpc-darwin9, powerpc-linux-gnu
applied to mainline
thanks
Iain
2019-06-23 Iain Sandoe
* gcc.target/powerpc/builtins-1.c: Account for Darwin's use of
__USER_LABEL_PREFIX__.
diff --git a/gcc/testsuite/gcc.target/powerpc/builtins-1.c
b/gcc/testsuite/gcc.target/powerpc/builtins-1.c
index 8263e28..73f8fb5 100644
--- a/gcc/testsuite/gcc.target/powerpc/builtins-1.c
+++ b/gcc/testsuite/gcc.target/powerpc/builtins-1.c
@@ -330,10 +330,10 @@ int main ()
/* { dg-final { scan-assembler-times "divd" 8 { target lp64 } } } */
/* { dg-final { scan-assembler-times "divdu" 2 { target lp64 } } } */
/* { dg-final { scan-assembler-times "mulld" 4 { target lp64 } } } */
-/* check for both .__divdi3 (AIX) and __divdi3 (Linux) */
-/* { dg-final { scan-assembler-times {\mbl \.?__divdi3\M} 2 { target { ilp32
} } } } */
-/* check for both .__udivdi3 (AIX) and __udivdi3 (Linux) */
-/* { dg-final { scan-assembler-times {\mbl \.?__udivdi3\M} 2 { target { ilp32
} } } } */
+/* check for .__divdi3 (AIX), __divdi3 (Linux) and ___divdi3 (Darwin) */
+/* { dg-final { scan-assembler-times {\mbl \.?_?__divdi3\M} 2 { target {
ilp32 } } } } */
+/* check for both .__udivdi3 (AIX), __udivdi3 (Linux) and ___udivdi3 (Darwin)
*/
+/* { dg-final { scan-assembler-times {\mbl \.?_?__udivdi3\M} 2 { target {
ilp32 } } } } */
/* { dg-final { scan-assembler-times "mullw" 12 { target ilp32 } } } */
/* { dg-final { scan-assembler-times "mulhwu" 4 { target ilp32 } } } */
/* { dg-final { scan-assembler-times "xxmrgld" 0 } } */
[Darwin, PPC, testsuite, committed] Fix pr80125 testcase for Darwin.
Darwin (unlike most of the members of the PowerPC port family) defaults to signed chars, so the test was failing to compile with a "mismatched parameters" error. tested on powerpc-darwin9, powerpc-linux-gnu applied to mainline thanks Iain 2019-06-23 Iain Sandoe * gcc.target/powerpc/pr80125.c (foo): Use an unsigned char vector explicitly for the vec_perm. diff --git a/gcc/testsuite/gcc.target/powerpc/pr80125.c b/gcc/testsuite/gcc.target/powerpc/pr80125.c index 494a6e6..366602d 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr80125.c +++ b/gcc/testsuite/gcc.target/powerpc/pr80125.c @@ -16,7 +16,7 @@ foo () vector int k = vec_mergel (i, j); vector int l = vec_sl (k, c); vector int m = vec_sl (l, d); - vector char o; + vector unsigned char o; vector int p = vec_perm (m, n, o); e = vec_sra (p, c); vec_st (e, 0, a);
[Darwin, PPC, testsuite, committed] Fix builtins-2 Darwin.
This cannot pass for current Darwin, since it requires VSX and we don't have
any hardware supporting that. The “fix” is to add a dg-requires clause for
this.
2019-06-23 Iain Sandoe
* gcc.target/powerpc/builtins-2.c: Require VSX hardware support.
diff --git a/gcc/testsuite/gcc.target/powerpc/builtins-2.c
b/gcc/testsuite/gcc.target/powerpc/builtins-2.c
index bf3347a..0fa60b2 100644
--- a/gcc/testsuite/gcc.target/powerpc/builtins-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/builtins-2.c
@@ -1,4 +1,5 @@
/* { dg-do run { target { powerpc*-*-* } } } */
+/* { dg-require-effective-target vsx_hw } */
/* { dg-options "-mdejagnu-cpu=power8 " } */
#include
[Patch, fortran] PR90903 - Implement runtime checks for bit manipulation intrinsics
Dear all,
the attached patch provides run-time checks for the bit manipulation
intrinsic functions (IBSET/IBCLR/BTEST/SHIFT[RLA]/ISHFT/ISHFTC).
I am using only one testcase whose purpose is mainly to verify that
there are no false positives, which I consider essential, and one
"failing" test at the end.
What is still missing are run-time checks for the subroutine MVBITS.
I am not sure yet how to handle that case (frontend or library?),
and I am open to suggestions. For this purpose I intend to leave
the PR open until a good solution is found.
Regtested on x86_64-pc-linux-gnu. OK for trunk?
Harald
2019-06-23 Harald Anlauf
PR fortran/90903
* libgfortran.h: Add mask for -fcheck=bits option.
* options.c (gfc_handle_runtime_check_option): Add option "bits"
to run-time checks selectable via -fcheck.
* trans-intrinsic.c (gfc_conv_intrinsic_btest)
(gfc_conv_intrinsic_singlebitop, gfc_conv_intrinsic_ibits)
(gfc_conv_intrinsic_shift, gfc_conv_intrinsic_ishft)
(gfc_conv_intrinsic_ishftc): Implement run-time checks for the
POS, LEN, SHIFT, and SIZE arguments.
* gfortran.texi: Document run-time checks for bit manipulation
intrinsics.
* invoke.texi: Document new -fcheck=bits option.
2019-06-23 Harald Anlauf
PR fortran/90903
* gfortran.dg/check_bits_1.f90: New testcase.
Index: gcc/fortran/gfortran.texi
===
--- gcc/fortran/gfortran.texi (revision 272560)
+++ gcc/fortran/gfortran.texi (working copy)
@@ -3790,7 +3790,8 @@
Default: enabled.
@item @var{option}[6] @tab Enables run-time checking. Possible values
are (bitwise or-ed): GFC_RTCHECK_BOUNDS (1), GFC_RTCHECK_ARRAY_TEMPS (2),
-GFC_RTCHECK_RECURSION (4), GFC_RTCHECK_DO (16), GFC_RTCHECK_POINTER (32).
+GFC_RTCHECK_RECURSION (4), GFC_RTCHECK_DO (16), GFC_RTCHECK_POINTER (32),
+GFC_RTCHECK_BITS (64).
Default: disabled.
@item @var{option}[7] @tab Unused.
@item @var{option}[8] @tab Show a warning when invoking @code{STOP} and
Index: gcc/fortran/invoke.texi
===
--- gcc/fortran/invoke.texi (revision 272560)
+++ gcc/fortran/invoke.texi (working copy)
@@ -183,7 +183,7 @@
@gccoptlist{-faggressive-function-elimination -fblas-matmul-limit=@var{n} @gol
-fbounds-check -ftail-call-workaround -ftail-call-workaround=@var{n} @gol
-fcheck-array-temporaries @gol
--fcheck=@var{} @gol
+-fcheck=@var{} @gol
-fcoarray=@var{} -fexternal-blas -ff2c
-ffrontend-loop-interchange @gol
-ffrontend-optimize @gol
@@ -1558,6 +1558,7 @@
@item -fcheck=@var{}
@opindex @code{fcheck}
@cindex array, bounds checking
+@cindex bit intrinsics checking
@cindex bounds checking
@cindex pointer checking
@cindex memory checking
@@ -1582,6 +1583,10 @@
Note: The warning is only printed once per location.
+@item @samp{bits}
+Enable generation of run-time checks for invalid arguments to the bit
+manipulation intrinsics.
+
@item @samp{bounds}
Enable generation of run-time checks for array subscripts
and against the declared minimum and maximum values. It also
Index: gcc/fortran/libgfortran.h
===
--- gcc/fortran/libgfortran.h (revision 272560)
+++ gcc/fortran/libgfortran.h (working copy)
@@ -73,9 +73,11 @@
#define GFC_RTCHECK_DO (1<<3)
#define GFC_RTCHECK_POINTER (1<<4)
#define GFC_RTCHECK_MEM (1<<5)
+#define GFC_RTCHECK_BITS(1<<6)
#define GFC_RTCHECK_ALL(GFC_RTCHECK_BOUNDS | GFC_RTCHECK_ARRAY_TEMPS \
| GFC_RTCHECK_RECURSION | GFC_RTCHECK_DO \
- | GFC_RTCHECK_POINTER | GFC_RTCHECK_MEM)
+ | GFC_RTCHECK_POINTER | GFC_RTCHECK_MEM \
+ | GFC_RTCHECK_BITS)
/* Special unit numbers used to convey certain conditions. Numbers -4
thru -9 available. NEWUNIT values start at -10. */
Index: gcc/fortran/options.c
===
--- gcc/fortran/options.c (revision 272560)
+++ gcc/fortran/options.c (working copy)
@@ -580,12 +580,12 @@
int result, pos = 0, n;
static const char * const optname[] = { "all", "bounds", "array-temps",
"recursion", "do", "pointer",
- "mem", NULL };
+ "mem", "bits", NULL };
static const int optmask[] = { GFC_RTCHECK_ALL, GFC_RTCHECK_BOUNDS,
GFC_RTCHECK_ARRAY_TEMPS,
GFC_RTCHECK_RECURSION, GFC_RTCHECK_DO,
GFC_RTCHECK_POINTER, GFC_RTCHECK_MEM,
-0 };
+GFC_RTCHECK_BITS, 0 };
while (*arg)
{
Index: gcc/fortran/trans-intrinsic.c
===
Re: [PATCH] don't trim empty string initializers for pointers (PR 90947)
On 6/22/19 9:37 PM, Jason Merrill wrote:
On 6/21/19 8:05 PM, Martin Sebor wrote:
The solution we implemented in GCC 9 to get the mangling of
non-type template arguments of class types containing array
members consistent regardless of the form of their
initialization introduced a couple of bugs. One of these
is the subject of this patch. The bug results in stripping
trailing initializers for array elements that involve the empty
string such as in here:
void f (void)
{
const char* a[1][1] = { "" };
if (!a[0][0])
__builtin_abort ();
}
The problem is caused by relying on initializer_zerop() that
returns true for the empty string regardless of whether it's
used to initialize an array or a pointer (the function doesn't
know what the initializer is being used for).
Why doesn't the existing POINTER_TYPE_P check handle this? It's clearly
intended to.
It does but only only for initializers of "leaf" elements/members.
The function processes initializers of enclosing elements or members
recursively. When initializer_zerop returns true for one of those,
it doesn't differentiate between an empty string that's being used
to initialize a leaf array or a leaf pointer.
For the example above, the first time through, elt_type is char*
and elt_init is the empty string, or the array char[1], and so
the initializer is retained.
The second time through (after the leaf recursive call returns),
elt_init is { "" } (i.e., an array of one empty string), elt_type
is also an array, and initializer_zerop(elt_init) returns true, so
the initializer is dropped.
I'm actually surprised this initializer_zerop ambiguity between
arrays and strings doesn't come up elsewhere. That's also why
I added the new function to tree.c.
Btw., it belatedly occurred to me that the new function doesn't
correctly handled unnamed bit-fields so I've corrected it to
handle those as well. Attached is the revised patch with this
change and a test to exercise it.
Martin
PS I found DECL_UNNAMED_BIT_FIELD in c-family/c-common.h.
I used (DECL_BIT_FIELD (fld) && !DECL_NAME (fld)) because
the former macro isn't available in tree.c. Does that mean
that that would make the new function not completely correct
in some other languages?
PR c++/90947 - Simple lookup table of array of strings is miscompiled
gcc/cp/ChangeLog:
PR c++/90947
* decl.c (reshape_init_array_1): Avoid truncating initializer
lists containing string literals.
gcc/testsuite/ChangeLog:
PR c++/90947
* c-c++-common/array-1.c: New test.
* g++.dg/abi/mangle73.C: New test.
* g++.dg/cpp2a/nontype-class18.C: New test.
* g++.dg/init/array53.C: New test.
gcc/ChangeLog:
PR c++/90947
* tree.c (type_initializer_zero_p): Define.
* tree.h (type_initializer_zero_p): New function.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 98b54d542a0..95893631992 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -5863,8 +5863,9 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d,
/* Pointers initialized to strings must be treated as non-zero
even if the string is empty. */
tree init_type = TREE_TYPE (elt_init);
- if ((POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type))
- || !initializer_zerop (elt_init))
+ if ((POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type)))
+ last_nonzero = index;
+ else if (!type_initializer_zero_p (elt_type, elt_init))
last_nonzero = index;
/* This can happen with an invalid initializer (c++/54501). */
diff --git a/gcc/testsuite/c-c++-common/array-1.c b/gcc/testsuite/c-c++-common/array-1.c
new file mode 100644
index 000..5de9ade4d43
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/array-1.c
@@ -0,0 +1,247 @@
+// PR c++/90947 - Simple lookup table of array of strings is miscompiled
+// { dg-do compile }
+// { dg-options "-O1 -fdump-tree-optimized" }
+
+#define assert(expr) ((expr) ? (void)0 : __builtin_abort ())
+
+void pr90947 (void)
+{
+ int vecsize = 4;
+ int index = 0;
+ static const char *a[4][4] =
+{
+ { ".x", ".y", ".z", ".w" },
+ { ".xy", ".yz", ".zw", 0 },
+ { ".xyz", ".yzw", 0, 0 },
+ { "", 0, 0, 0 },
+};
+
+ assert (vecsize >= 1 && vecsize <= 4);
+ assert (index >= 0 && index < 4);
+ assert (a[vecsize - 1][index]);
+}
+
+void f_a1_1 (void)
+{
+ {
+const char* a[1][1] = { { 0 } };
+assert (0 == a[0][0]);
+ }
+ {
+const char* a[1][1] = { { "" } };
+assert ('\0' == *a[0][0]);
+ }
+}
+
+void f_a2_1 (void)
+{
+ {
+const char* a[2][1] = { { "" }, { "" } };
+assert ('\0' == *a[0][0] && '\0' == *a[1][0]);
+ }
+ {
+const char* a[2][1] = { { 0 }, { "" } };
+assert (0 == a[0][0] && '\0' == *a[1][0]);
+ }
+ {
+const char* a[2][1] = { { }, { "" } };
+assert (0 == a[0][0] && '\0' == *a[1][0]);
+ }
+}
+
+void f_a2_2 (void)
+{
+ {
+const char* a[2][2] = { { "", "" }, { "", "" } };
+assert ('\0' == *a[0][0] && '\0' == *a[0][1]);
+assert ('\0' == *a[1][0] && '\0' == *a[1][1]);
+
Go patch committed: Stop using gcc diagnostic formatting for info messages
This patch to the Go frontend adds a new function go_debug and uses it
for debug messages.
GCC recently added a new warning -Wformat-diag which does a lot of
rigorous checks on GCC diagnostic messages. This produces a number of
unnecessary diagnostics on gofrontend diagnostic output, such as:
../../trunk/gcc/go/gofrontend/escape.cc: In member function ‘virtual
int Escape_analysis_assign::statement(Block*, size_t*, Statement*)’:
../../trunk/gcc/go/gofrontend/escape.cc:1336:33: warning: spurious
leading punctuation sequence ‘[’ in format [-Wformat-diag]
1336 | go_inform(s->location(), "[%d] %s esc: %s",
| ^
../../trunk/gcc/go/gofrontend/escape.cc: In member function ‘void
Escape_analysis_assign::call(Call_expression*)’:
../../trunk/gcc/go/gofrontend/escape.cc:1964:17: warning: unquoted
operator ‘::’ in format [-Wformat-diag]
1964 | "esccall:: indirect call <- %s, untracked",
| ^~
../../trunk/gcc/go/gofrontend/escape.cc:1964:34: warning: unbalanced
punctuation character ‘<’ in format [-Wformat-diag]
1964 | "esccall:: indirect call <- %s, untracked",
| ^
Avoid these messages by adding a new function go_debug that uses only
printf formatting, not GCC diagnostic formatting, and change all the
optimization debugging messages to use it. None of the debugging
messages used the GCC diagnostic formatting specifiers anyhow.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed
to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 272579)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-9b5a43baaf391005989d140109261e5a8e1b1b63
+6bb63a21434b3360dbe7e4bd34889734f361d434
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/escape.cc
===
--- gcc/go/gofrontend/escape.cc (revision 272577)
+++ gcc/go/gofrontend/escape.cc (working copy)
@@ -964,9 +964,9 @@ Gogo::analyze_escape()
{
done = false;
if (this->debug_escape_level() > 2)
-go_inform((*n)->location(), "Reflooding %s %s",
- debug_function_name((*n)->state(context,
NULL)->fn).c_str(),
- (*n)->ast_format(this).c_str());
+go_debug((*n)->location(), "Reflooding %s %s",
+debug_function_name((*n)->state(context,
NULL)->fn).c_str(),
+(*n)->ast_format(this).c_str());
escapes[*n] = (*n)->encoding();
this->propagate_escape(context, *n);
}
@@ -990,9 +990,9 @@ Gogo::analyze_escape()
{
Node::Escape_state* state = (*n)->state(context, NULL);
if ((*n)->encoding() == Node::ESCAPE_NONE)
- go_inform((*n)->location(), "%s %s does not escape",
- strip_packed_prefix(this,
debug_function_name(state->fn)).c_str(),
- (*n)->ast_format(this).c_str());
+ go_debug((*n)->location(), "%s %s does not escape",
+strip_packed_prefix(this,
debug_function_name(state->fn)).c_str(),
+(*n)->ast_format(this).c_str());
}
}
delete context;
@@ -1333,9 +1333,9 @@ Escape_analysis_assign::statement(Block*
{
Node* n = Node::make_node(s);
std::string fn_name = this->context_->current_function_name();
- go_inform(s->location(), "[%d] %s esc: %s",
- this->context_->loop_depth(), fn_name.c_str(),
- n->ast_format(gogo).c_str());
+ go_debug(s->location(), "[%d] %s esc: %s",
+ this->context_->loop_depth(), fn_name.c_str(),
+ n->ast_format(gogo).c_str());
}
switch (s->classification())
@@ -1495,9 +1495,9 @@ move_to_heap(Gogo* gogo, Expression *exp
{
Node* n = Node::make_node(expr);
if (gogo->debug_escape_level() != 0)
-go_inform(n->definition_location(),
- "moved to heap: %s",
- n->ast_format(gogo).c_str());
+go_debug(n->definition_location(),
+"moved to heap: %s",
+n->ast_format(gogo).c_str());
if (gogo->compiling_runtime() && gogo->package_name() == "runtime")
go_error_at(expr->location(),
"%s escapes to heap, not allowed in runtime",
@@ -1519,8 +1519,8 @@ Escape_analysis_assign::expression(Expre
&& n->is_big(this->context_))
{
if (debug_level > 1)
- go_inform((*pexpr)->location(), "%s too large for stack",
- n->ast_format(gogo).c_str());
+ go_debug((*pexpr)->location(), "%s too large fo
Go patch committed: edit error messages to avoid -Wformat-diag warnings
This patch to the Go frontend edits various error messages to avoid
-Wformat-diag warnings.
GCC recently introduced -Wformat-diag to scrutinize GCC error
messages. It reports a number of warnings about gofrontend code, such
as:
../../trunk/gcc/go/gofrontend/import.cc: In member function ‘Type*
Import::type_for_index(int, const string&, size_t, bool*)’:
../../trunk/gcc/go/gofrontend/import.cc:1129:48: warning: unquoted
operator ‘>=’ in format [-Wformat-diag]
1129 | "error in %s at %lu: bad type index %d >= %d",
|^~
../../trunk/gcc/go/gofrontend/ast-dump.cc: In member function ‘void
Ast_dump_context::dump(Gogo*, const char*)’:
../../trunk/gcc/go/gofrontend/ast-dump.cc:203:25: warning: unquoted
option name ‘-fgo-dump-ast’ in format [-Wformat-diag]
203 | "cannot open %s:%m, -fgo-dump-ast ignored", dumpname.c_str());
| ^
../../trunk/gcc/go/gofrontend/expressions.cc: In static member
function ‘static Bexpression* Func_expression::get_code_pointer(Gogo*,
Named_object*, Location)’:
../../trunk/gcc/go/gofrontend/expressions.cc:1350:29: warning:
misspelled term ‘builtin function’ in format; use ‘built-in function’
instead [-Wformat-diag]
1350 | "invalid use of special builtin function %qs; must be called",
| ^~~~
../../trunk/gcc/go/gofrontend/gogo.cc: In member function ‘void
Gogo::add_linkname(const string&, bool, const string&, Location)’:
../../trunk/gcc/go/gofrontend/gogo.cc:2527:4: warning: unquoted
sequence of 2 consecutive punctuation characters ‘//’ in format
[-Wformat-diag]
2527 | ("%s is not a function; "
| ~^~~~
2528 |"//go:linkname is only supported for functions"),
|
This patch edits error messages to avoid these warnings. I don't
think they are all improvements, but some are, and none seem
significantly worse.
This required changing one Go test to accept quote characters we were
not previously generating.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed
to mainline.
Ian
2019-06-23 Ian Lance Taylor
* go.test/test/blank1.go: Update for diagnostic message changes.
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 272607)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-6bb63a21434b3360dbe7e4bd34889734f361d434
+1232eef628227ef855c5fa6d94b31778b2e74a85
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/ast-dump.cc
===
--- gcc/go/gofrontend/ast-dump.cc (revision 272577)
+++ gcc/go/gofrontend/ast-dump.cc (working copy)
@@ -200,7 +200,8 @@ Ast_dump_context::dump(Gogo* gogo, const
if (out.fail())
{
go_error_at(Linemap::unknown_location(),
- "cannot open %s:%m, -fgo-dump-ast ignored", dumpname.c_str());
+ "cannot open %s:%m; %<-fgo-dump-ast%> ignored",
+ dumpname.c_str());
return;
}
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 272607)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -1347,7 +1347,8 @@ Func_expression::get_code_pointer(Gogo*
if (fntype->is_builtin())
{
go_error_at(loc,
- "invalid use of special builtin function %qs; must be called",
+ ("invalid use of special built-in function %qs; "
+ "must be called"),
no->message_name().c_str());
return gogo->backend()->error_expression();
}
@@ -1386,7 +1387,7 @@ Func_expression::do_get_backend(Translat
if (no->func_declaration_value()->type()->is_builtin())
{
go_error_at(this->location(),
- ("invalid use of special builtin function %qs; "
+ ("invalid use of special built-in function %qs; "
"must be called"),
no->message_name().c_str());
return gogo->backend()->error_expression();
@@ -8425,7 +8426,7 @@ Builtin_call_expression::lower_make(Stat
if (!type->in_heap())
go_error_at(first_arg->location(),
- "can't make slice of go:notinheap type");
+ "cannot make slice of go:notinheap type");
bool is_slice = false;
bool is_map = false;
@@ -9804,7 +9805,7 @@ Builtin_call_expression::do_check_types(
{
if (this->code_ == BUILTIN_PRINT)
go_warning_at(this->location(), 0,
-"no arguments for builtin function %<%s%>",
+"no arguments for built-in function %<
Re: [PATCH][RFC] Sanitize equals and hash functions in hash-tables.
On Fri, Jun 7, 2019 at 5:04 AM Martin Liška wrote:
>
> On 6/7/19 10:57 AM, Richard Biener wrote:
> > On Mon, Jun 3, 2019 at 3:35 PM Martin Liška wrote:
> >>
> >> On 6/1/19 12:06 AM, Jeff Law wrote:
> >>> On 5/22/19 3:13 AM, Martin Liška wrote:
> On 5/21/19 1:51 PM, Richard Biener wrote:
> > On Tue, May 21, 2019 at 1:02 PM Martin Liška wrote:
> >>
> >> On 5/21/19 11:38 AM, Richard Biener wrote:
> >>> On Tue, May 21, 2019 at 12:07 AM Jeff Law wrote:
>
> On 5/13/19 1:41 AM, Martin Liška wrote:
> > On 11/8/18 9:56 AM, Martin Liška wrote:
> >> On 11/7/18 11:23 PM, Jeff Law wrote:
> >>> On 10/30/18 6:28 AM, Martin Liška wrote:
> On 10/30/18 11:03 AM, Jakub Jelinek wrote:
> > On Mon, Oct 29, 2018 at 04:14:21PM +0100, Martin Liška wrote:
> >> +hashtab_chk_error ()
> >> +{
> >> + fprintf (stderr, "hash table checking failed: "
> >> + "equal operator returns true for a pair "
> >> + "of values with a different hash value");
> > BTW, either use internal_error here, or at least if using
> > fprintf
> > terminate with \n, in your recent mail I saw:
> > ...different hash valueduring RTL pass: vartrack
> > ^^
> Sure, fixed in attached patch.
>
> Martin
>
> >> + gcc_unreachable ();
> >> +}
> > Jakub
> >
> 0001-Sanitize-equals-and-hash-functions-in-hash-tables.patch
>
> From 0d9c979c845580a98767b83c099053d36eb49bb9 Mon Sep 17
> 00:00:00 2001
> From: marxin
> Date: Mon, 29 Oct 2018 09:38:21 +0100
> Subject: [PATCH] Sanitize equals and hash functions in
> hash-tables.
>
> ---
> gcc/hash-table.h | 40 +++-
> 1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/hash-table.h b/gcc/hash-table.h
> index bd83345c7b8..694eedfc4be 100644
> --- a/gcc/hash-table.h
> +++ b/gcc/hash-table.h
> @@ -503,6 +503,7 @@ private:
>
> value_type *alloc_entries (size_t n CXX_MEM_STAT_INFO) const;
> value_type *find_empty_slot_for_expand (hashval_t);
> + void verify (const compare_type &comparable, hashval_t hash);
> bool too_empty_p (unsigned int);
> void expand ();
> static bool is_deleted (value_type &v)
> @@ -882,8 +883,12 @@ hash_table
> if (insert == INSERT && m_size * 3 <= m_n_elements * 4)
> expand ();
>
> - m_searches++;
> +#if ENABLE_EXTRA_CHECKING
> +if (insert == INSERT)
> + verify (comparable, hash);
> +#endif
>
> + m_searches++;
> value_type *first_deleted_slot = NULL;
> hashval_t index = hash_table_mod1 (hash, m_size_prime_index);
> hashval_t hash2 = hash_table_mod2 (hash, m_size_prime_index);
> @@ -930,6 +935,39 @@ hash_table
> return &m_entries[index];
> }
>
> +#if ENABLE_EXTRA_CHECKING
> +
> +/* Report a hash table checking error. */
> +
> +ATTRIBUTE_NORETURN ATTRIBUTE_COLD
> +static void
> +hashtab_chk_error ()
> +{
> + fprintf (stderr, "hash table checking failed: "
> + "equal operator returns true for a pair "
> + "of values with a different hash value\n");
> + gcc_unreachable ();
> +}
> >>> I think an internal_error here is probably still better than a
> >>> simple
> >>> fprintf, even if the fprintf is terminated with a \n :-)
> >> Fully agree with that, but I see a lot of build errors when using
> >> internal_error.
> >>
> >>> The question then becomes can we bootstrap with this stuff
> >>> enabled and
> >>> if not, are we likely to soon? It'd be a shame to put it into
> >>> EXTRA_CHECKING, but then not be able to really use EXTRA_CHECKING
> >>> because we've got too many bugs to fix.
> >> Unfortunately it's blocked with these 2 PRs:
> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87845
> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87847
> > Hi.
> >
> > I've just added one more PR:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90450
> >
> > I'm sending updated version of the
[PATCH] some more -Wformat-diag cleanup
The attached patch cleans up a number of outstanding -Wformat-diag
instances. I plan to commit it tomorrow.
With it applied, an x86_64-linux bootstrap shows just 79 unique
instances of the warning originating in 17 files. 49 of those are
in the Go front-end that Ian is already dealing with. I will work
on the rest.
Martin
gcc/ada/ChangeLog:
* gcc-interface/utils.c (handle_nonnull_attribute): Quote attribute
name.
gcc/c/ChangeLog:
* c-typeck.c (build_binary_op): Hyphenate floating-point.
gcc/testsuite/ChangeLog:
* gcc.dg/Wfloat-equal-1.c: Adjust text of expected diagnostic.
* gcc.dg/misc-column.c: Ditto.
gcc/ChangeLog:
* tree-pretty-print.h: Remove unnecessary punctuation characters
from a diagnostic.
* tree-ssa.c (release_defs_bitset): Correct preprocessor conditional.
Index: gcc/ada/gcc-interface/utils.c
===
--- gcc/ada/gcc-interface/utils.c (revision 272575)
+++ gcc/ada/gcc-interface/utils.c (working copy)
@@ -6234,7 +6234,8 @@ handle_nonnull_attribute (tree *node, tree ARG_UNU
&& (!TYPE_ATTRIBUTES (type)
|| !lookup_attribute ("type generic", TYPE_ATTRIBUTES (type
{
- error ("nonnull attribute without arguments on a non-prototype");
+ error ("%qs attribute without arguments on a non-prototype",
+ "nonnull");
*no_add_attrs = true;
}
return NULL_TREE;
@@ -6248,8 +6249,8 @@ handle_nonnull_attribute (tree *node, tree ARG_UNU
if (!get_nonnull_operand (TREE_VALUE (args), &arg_num))
{
- error ("nonnull argument has invalid operand number (argument %lu)",
- (unsigned long) attr_arg_num);
+ error ("%qs argument has invalid operand number (argument %lu)",
+ "nonnull", (unsigned long) attr_arg_num);
*no_add_attrs = true;
return NULL_TREE;
}
@@ -6270,8 +6271,8 @@ handle_nonnull_attribute (tree *node, tree ARG_UNU
if (!argument
|| TREE_CODE (argument) == VOID_TYPE)
{
- error ("nonnull argument with out-of-range operand number "
- "(argument %lu, operand %lu)",
+ error ("%qs argument with out-of-range operand number "
+ "(argument %lu, operand %lu)", "nonnull",
(unsigned long) attr_arg_num, (unsigned long) arg_num);
*no_add_attrs = true;
return NULL_TREE;
@@ -6279,8 +6280,8 @@ handle_nonnull_attribute (tree *node, tree ARG_UNU
if (TREE_CODE (argument) != POINTER_TYPE)
{
- error ("nonnull argument references non-pointer operand "
- "(argument %lu, operand %lu)",
+ error ("%qs argument references non-pointer operand "
+ "(argument %lu, operand %lu)", "nonnull",
(unsigned long) attr_arg_num, (unsigned long) arg_num);
*no_add_attrs = true;
return NULL_TREE;
Index: gcc/c/c-typeck.c
===
--- gcc/c/c-typeck.c (revision 272575)
+++ gcc/c/c-typeck.c (working copy)
@@ -11939,7 +11939,7 @@ build_binary_op (location_t location, enum tree_co
if (FLOAT_TYPE_P (type0) || FLOAT_TYPE_P (type1))
warning_at (location,
OPT_Wfloat_equal,
- "comparing floating point with %<==%> or % is unsafe");
+ "comparing floating-point with %<==%> or % is unsafe");
/* Result of comparison is always int,
but don't convert the args to int! */
build_type = integer_type_node;
Index: gcc/testsuite/gcc.dg/Wfloat-equal-1.c
===
--- gcc/testsuite/gcc.dg/Wfloat-equal-1.c (revision 272575)
+++ gcc/testsuite/gcc.dg/Wfloat-equal-1.c (working copy)
@@ -4,7 +4,7 @@
double a, b;
_Complex double c, d;
-int f(void) { return a == b; } /* { dg-warning "comparing floating point" } */
-int g(void) { return c == d; } /* { dg-warning "comparing floating point" } */
-int h(void) { return a != b; } /* { dg-warning "comparing floating point" } */
-int i(void) { return c != d; } /* { dg-warning "comparing floating point" } */
+int f(void) { return a == b; } /* { dg-warning "comparing floating-point" } */
+int g(void) { return c == d; } /* { dg-warning "comparing floating-point" } */
+int h(void) { return a != b; } /* { dg-warning "comparing floating-point" } */
+int i(void) { return c != d; } /* { dg-warning "comparing floating-point" } */
Index: gcc/testsuite/gcc.dg/misc-column.c
===
--- gcc/testsuite/gcc.dg/misc-column.c (revision 272575)
+++ gcc/testsuite/gcc.dg/misc-column.c (working copy)
@@ -13,7 +13,7 @@ extern void bar();
void foo (void)
{
- if (a == b) /* { dg-warning "9:comparing floating point with" } */
+ if (a == b) /* { dg-warning "9:comparing floating-point with" } */
bar ();
if (p < q) /* { dg-warning "9:comparison of distinct pointer types" } */
Index: gcc/tree-pretty-print.h
===
--- gcc/tree-pretty-print.h (revision 272575)
+++ gcc/tree-pretty-print.h (working copy)
@@ -25,7 +2
Re: [PATCH] [RFC, PGO+LTO] Missed function specialization + partial devirtualization
Hi Honza,
Thanks very much to get so many useful comments from you.
As a newbie to GCC, not sure whether my questions are described clearly
enough. Thanks for your patience in advance. :)
On 2019/6/20 21:47, Jan Hubicka wrote:
Hi,
some comments on the ipa part of the patch
(and thanks for working on it - this was on my TODO list for years)
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index de82316d4b1..0d373a67d1b 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -553,6 +553,7 @@ cgraph_node::get_create (tree decl)
fprintf (dump_file, "Introduced new external node "
"(%s) and turned into root of the clone tree.\n",
node->dump_name ());
+ node->profile_id = first_clone->profile_id;
}
else if (dump_file)
fprintf (dump_file, "Introduced new external node "
This is independent of the rest of changes. Do you have example where
this matters? The inline clones are created in ipa-inline while
ipa-profile is run before it, so I can not think of such a scenario.
I see you also copy profile_id from function to clone. I would like to
know why you needed that.
Also you mention that you hit some ICEs. If fixes are independent of
rest of your changes, send them separately.
I copy the profile_id for cloned node as when in LTO ltrans, there is no
references or referrings info for the specialized node/cloned node, so it
is difficult to track the node's reference in
cgraph_edge::speculative_call_info. I use it mainly for debug purpose now.
Will remove it and split the patches in later version to include ICE fixes.
@@ -1110,6 +,7 @@ cgraph_edge::speculative_call_info (cgraph_edge *&direct,
int i;
cgraph_edge *e2;
cgraph_edge *e = this;
+ cgraph_node *referred_node;
if (!e->indirect_unknown_callee)
for (e2 = e->caller->indirect_calls;
@@ -1142,8 +1144,20 @@ cgraph_edge::speculative_call_info (cgraph_edge *&direct,
&& ((ref->stmt && ref->stmt == e->call_stmt)
|| (!ref->stmt && ref->lto_stmt_uid == e->lto_stmt_uid)))
{
- reference = ref;
- break;
+ if (e2->indirect_info && e2->indirect_info->num_of_ics)
+ {
+ referred_node = dyn_cast (ref->referred);
+ if (strstr (e->callee->name (), referred_node->name ()))
+ {
+ reference = ref;
+ break;
+ }
+ }
+ else
+ {
+ reference = ref;
+ break;
+ }
}
This function is intended to return everything related to the
speculative call, so if you add multiple direct targets, i would expect
it to tage auto_vec of cgraph_nodes for direct and auto_vec of
references.
So will the signature becomes
cgraph_edge::speculative_call_info (auto_vec *direct,
cgraph_edge *&indirect,
auto_vec *reference)
Seems a lot of code related to it, maybe should split to another patch.
And will the sequence of direct and reference in each auto_vec be strictly
mapped for iteration convenience?
Second question is "this" is a direct edge will be pushed to auto_vec
"direct", how can it get its next direct edge here? From e->caller->callees?
/* Speculative edge always consist of all three components - direct edge,
@@ -1199,7 +1213,14 @@ cgraph_edge::resolve_speculation (tree callee_decl)
in the functions inlined through it. */
}
edge->count += e2->count;
- edge->speculative = false;
+ if (edge->indirect_info && edge->indirect_info->num_of_ics)
+{
+ edge->indirect_info->num_of_ics--;
+ if (edge->indirect_info->num_of_ics == 0)
+ edge->speculative = false;
+}
+ else
+edge->speculative = false;
e2->speculative = false;
ref->remove_reference ();
if (e2->indirect_unknown_callee || e2->inline_failed)
This function should turn speculative call into direct call to DECL, so
I think it should remove all the other direct calls associated with stmt
and the indirect one.
There are now two cases - in first case you want to turn speculative
call into direct call or give up on especulation completely, while in
other case you want to only remove one of speculations.
I guess we want to have resolve_speculation(decl) for first and
remove_one_speculation(edge) for the second case?
The second case would be useful for the code below handling type
mismatches and also for inline when one of speculative targets seems not
useful to bother with.
So the logic will be:
if (edge->indirect_info->num_of_ics > 1)
cgraph_edge::resolve_speculation (tree callee_decl);
else
remove_one_speculation(edge);
cgraph_edge::resolve_speculation will call edge->speculative_call_info (e2,
edge, ref) internally, at this time, e2 and ref will only contains one
direct target?
@@ -1333,7 +1354,14 @@ cgraph_edge::redirect_call_stmt_to_callee (void)
e->caller->set_call_stmt_including_clones (e->cal
Re: [PATCH] Update to clang-format script
On 6/23/19 11:39 AM, 김규래 wrote: GNU style brace wrapping rules have been added to clang-format quite a while ago. http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?revision=197138&view=markup This simple patch proposes to use the built-in rules instead of the custom brace rules. I like the changes, please install the patch. Martin Ray Kim 2019-06-27 Ray Kim * contrib/clang-format: Changed the custom brace wrapping rules to built-in GNU style. diff --git a/contrib/clang-format b/contrib/clang-format index d734001c06f..3ebb85fff77 100644 --- a/contrib/clang-format +++ b/contrib/clang-format @@ -25,20 +25,8 @@ AccessModifierOffset: -2 AlwaysBreakAfterDefinitionReturnType: All BinPackArguments: true BinPackParameters: true -BraceWrapping: - AfterClass: true - AfterControlStatement: true - AfterEnum: true - AfterFunction: true - AfterNamespace: false - AfterObjCDeclaration: true - AfterStruct: true - AfterUnion: true - BeforeCatch: true - BeforeElse: true - IndentBraces: true BreakBeforeBinaryOperators: All -BreakBeforeBraces: Custom +BreakBeforeBraces: GNU BreakBeforeTernaryOperators: true ColumnLimit: 80 ConstructorInitializerIndentWidth: 2
[PATCH] [RS6000] Change maddld match_operand from DI to GPR
Hi,
>From PowerPC ISA3.0, the description of `maddld RT, RA.RB, RC` is as follows:
64-bit RA and RB are multiplied and then the RC is signed extend to 128 bits,
and add them together.
We only apply it to 64-bit mode (DI) when implementing maddld. However, if we
can guarantee that the result of the maddld operation will be limited to 32-bit
mode (SI), we can still apply it to 32-bit mode (SI).
The regression testing for the patch was done on GCC mainline on
powerpc64le-unknown-linux-gnu (Power 9 LE)
with no regressions. Is it OK for trunk ?
Thanks,
Lijia He
gcc/ChangeLog
2019-06-24 Li Jia He
* config/rs6000/rs6000.h (TARGET_MADDLD): Remove the restriction of
TARGET_POWERPC64.
* config/rs6000/rs6000.md (maddld): Change maddld match_operand from DI
to GPR.
gcc/testsuite/ChangeLog
2019-06-24 Li Jia He
* gcc.target/powerpc/maddld-1.c: New testcase.
---
gcc/config/rs6000/rs6000.h | 2 +-
gcc/config/rs6000/rs6000.md | 10 +-
gcc/testsuite/gcc.target/powerpc/maddld-1.c | 19 +++
3 files changed, 25 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/powerpc/maddld-1.c
diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
index 34fa36b6ed9..f83f19afbba 100644
--- a/gcc/config/rs6000/rs6000.h
+++ b/gcc/config/rs6000/rs6000.h
@@ -453,7 +453,7 @@ extern int rs6000_vector_align[];
#define TARGET_FCTIWUZ TARGET_POPCNTD
#define TARGET_CTZ TARGET_MODULO
#define TARGET_EXTSWSLI(TARGET_MODULO && TARGET_POWERPC64)
-#define TARGET_MADDLD (TARGET_MODULO && TARGET_POWERPC64)
+#define TARGET_MADDLD TARGET_MODULO
#define TARGET_XSCVDPSPN (TARGET_DIRECT_MOVE || TARGET_P8_VECTOR)
#define TARGET_XSCVSPDPN (TARGET_DIRECT_MOVE || TARGET_P8_VECTOR)
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 47cbba89443..9122b29e99b 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -3057,11 +3057,11 @@
DONE;
})
-(define_insn "*maddld4"
- [(set (match_operand:DI 0 "gpc_reg_operand" "=r")
- (plus:DI (mult:DI (match_operand:DI 1 "gpc_reg_operand" "r")
- (match_operand:DI 2 "gpc_reg_operand" "r"))
-(match_operand:DI 3 "gpc_reg_operand" "r")))]
+(define_insn "*maddld4"
+ [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
+ (plus:GPR (mult:GPR (match_operand:GPR 1 "gpc_reg_operand" "r")
+ (match_operand:GPR 2 "gpc_reg_operand" "r"))
+ (match_operand:GPR 3 "gpc_reg_operand" "r")))]
"TARGET_MADDLD"
"maddld %0,%1,%2,%3"
[(set_attr "type" "mul")])
diff --git a/gcc/testsuite/gcc.target/powerpc/maddld-1.c
b/gcc/testsuite/gcc.target/powerpc/maddld-1.c
new file mode 100644
index 000..06f5f5774d4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/maddld-1.c
@@ -0,0 +1,19 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-require-effective-target powerpc_p9modulo_ok } */
+/* { dg-options "-mdejagnu-cpu=power9 -O2" } */
+
+int
+s_madd (int a, int b, int c)
+{
+ return (a * b) + c;
+}
+
+unsigned int
+u_madd (unsigned int a, unsigned int b, unsigned int c)
+{
+ return (a * b) + c;
+}
+
+/* { dg-final { scan-assembler-times "maddld " 2 } } */
+/* { dg-final { scan-assembler-not "mulld "} } */
+/* { dg-final { scan-assembler-not "add " } } */
--
2.17.1
Re: [PATCH] [RS6000] Change maddld match_operand from DI to GPR
Hi Lijia,
on 2019/6/24 下午2:00, Li Jia He wrote:
> Hi,
>
> From PowerPC ISA3.0, the description of `maddld RT, RA.RB, RC` is as follows:
> 64-bit RA and RB are multiplied and then the RC is signed extend to 128 bits,
> and add them together.
>
> We only apply it to 64-bit mode (DI) when implementing maddld. However, if we
> can guarantee that the result of the maddld operation will be limited to
> 32-bit
> mode (SI), we can still apply it to 32-bit mode (SI).
>
> The regression testing for the patch was done on GCC mainline on
>
> powerpc64le-unknown-linux-gnu (Power 9 LE)
>
> with no regressions. Is it OK for trunk ?
>
> Thanks,
> Lijia He
>
> gcc/ChangeLog
> 2019-06-24 Li Jia He
>
> * config/rs6000/rs6000.h (TARGET_MADDLD): Remove the restriction of
> TARGET_POWERPC64.
> * config/rs6000/rs6000.md (maddld): Change maddld match_operand from DI
> to GPR.
>
> gcc/testsuite/ChangeLog
>
> 2019-06-24 Li Jia He
>
> * gcc.target/powerpc/maddld-1.c: New testcase.
> ---
> gcc/config/rs6000/rs6000.h | 2 +-
> gcc/config/rs6000/rs6000.md | 10 +-
> gcc/testsuite/gcc.target/powerpc/maddld-1.c | 19 +++
> 3 files changed, 25 insertions(+), 6 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/powerpc/maddld-1.c
>
> diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> index 34fa36b6ed9..f83f19afbba 100644
> --- a/gcc/config/rs6000/rs6000.h
> +++ b/gcc/config/rs6000/rs6000.h
> @@ -453,7 +453,7 @@ extern int rs6000_vector_align[];
> #define TARGET_FCTIWUZ TARGET_POPCNTD
> #define TARGET_CTZ TARGET_MODULO
> #define TARGET_EXTSWSLI (TARGET_MODULO && TARGET_POWERPC64)
> -#define TARGET_MADDLD(TARGET_MODULO && TARGET_POWERPC64)
> +#define TARGET_MADDLDTARGET_MODULO
>
IMHO, I don't think this removal of TARGET_POWERPC64 is reasonable.
As ISA V3.0, the description of this insn maddld is:
GPR[RT].dword[0] ← Chop(result, 64)
It assumes the GPR has dword, it's a 64-bit specific insn, right?
Your change relaxes it to be adopted on 32-bit.
Although it's fine for powerpc LE since it's always 64-bit, it will
have problems for power9 32bit like AIX?
> #define TARGET_XSCVDPSPN (TARGET_DIRECT_MOVE || TARGET_P8_VECTOR)
> #define TARGET_XSCVSPDPN (TARGET_DIRECT_MOVE || TARGET_P8_VECTOR)
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 47cbba89443..9122b29e99b 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -3057,11 +3057,11 @@
>DONE;
> })
>
> -(define_insn "*maddld4"
> - [(set (match_operand:DI 0 "gpc_reg_operand" "=r")
> - (plus:DI (mult:DI (match_operand:DI 1 "gpc_reg_operand" "r")
> - (match_operand:DI 2 "gpc_reg_operand" "r"))
> - (match_operand:DI 3 "gpc_reg_operand" "r")))]
> +(define_insn "*maddld4"
> + [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
> + (plus:GPR (mult:GPR (match_operand:GPR 1 "gpc_reg_operand" "r")
> + (match_operand:GPR 2 "gpc_reg_operand" "r"))
> + (match_operand:GPR 3 "gpc_reg_operand" "r")))]
>"TARGET_MADDLD"
>"maddld %0,%1,%2,%3"
>[(set_attr "type" "mul")])
> diff --git a/gcc/testsuite/gcc.target/powerpc/maddld-1.c
> b/gcc/testsuite/gcc.target/powerpc/maddld-1.c
> new file mode 100644
> index 000..06f5f5774d4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/maddld-1.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */
The above target requirement seems useless? since you have
below one which is more specific.
Thanks,
Kewen
> +/* { dg-require-effective-target powerpc_p9modulo_ok } */
> +/* { dg-options "-mdejagnu-cpu=power9 -O2" } */
> +
>
