Re: [PATCH] Fix dump message issue
On 2019/10/14 00:32, Jeff Law wrote: > On 10/8/19 4:45 AM, Martin Jambor wrote: >> Hi, >> >> On Tue, Oct 08 2019, luoxhu wrote: >>> '}' is missed at the end. >> >> heh, yeah, I wonder for how long. >> >> If it irritates you, I'd say the patch is obvious (though note that I >> cannot approve a patch in this area). > Looks obvious to me. And while you may not be an official reviewer > Martin, if you say someone's code looks good to you, I'm just going to > rubber stamp it. > > Which in turn means you ought to be a reviewer. Thanks, Martin and Jeff: It was introduced since the initial commit in 2009: 8d53b873fdce (jamborm 2009-05-29 16:47:31 + 277) fprintf (f, ", write = %d, grp_partial_lhs = %d\n", access->write, and: c79d6ecf5563 (jamborm 2009-09-02 17:52:18 + 271) "grp_to_be_replaced = %d\n", Commited in r276948. Xiong Hu BR > > Jeff >
[Darwin, machopic 8/n, committed] Back out part of PR71767 fix.
We applied a conservative, but fairly large, hammer to fix PR71767.
However, ideally, we want minimise the number of symbols visible to
ld64 and to match the cases emitted by clang (since that's what ld64
is expecting). Now we've improved the handling of indirections, we
can make the indirection symbols local when they are in the regular
non-lazy symbol pointers section. We will continue to make any
indirections in the data section visible (since right now we have no
way to track if a given symbol follows a weak global).
This change makes no difference to handling of labels for constants
(to be revised in a future patch).
There's a mechanical change to a number of tests (allowing 'l' or 'L'
as the indirection symbol prefix).
tested on x86-64-darwin16, i686-darwin9,
applied to mainline
Iain.
gcc/ChangeLog:
2019-10-13 Iain Sandoe
* config/darwin.c (machopic_indirection_name): Rework the
function to emit linker-visible symbols only for indirections
in the data section. Clean up the code and update comments.
gcc/testsuite/ChangeLog:
2019-10-13 Iain Sandoe
* gcc.target/i386/indirect-thunk-1.c: Allow 'l' or 'L' in
indirection label prefix, for Darwin.
* gcc.target/i386/indirect-thunk-2.c: Likewise.
* gcc.target/i386/indirect-thunk-3.c: Likewise.
* gcc.target/i386/indirect-thunk-4.c: Likewise.
* gcc.target/i386/indirect-thunk-attr-1.c: Likewise.
* gcc.target/i386/indirect-thunk-attr-2.c: Likewise.
* gcc.target/i386/indirect-thunk-attr-3.c: Likewise.
* gcc.target/i386/indirect-thunk-attr-4.c: Likewise.
* gcc.target/i386/indirect-thunk-attr-5.c: Likewise.
* gcc.target/i386/indirect-thunk-attr-6.c: Likewise.
* gcc.target/i386/indirect-thunk-extern-1.c: Likewise.
* gcc.target/i386/indirect-thunk-extern-2.c: Likewise.
* gcc.target/i386/indirect-thunk-extern-3.c: Likewise.
* gcc.target/i386/indirect-thunk-extern-4.c: Likewise.
* gcc.target/i386/indirect-thunk-inline-1.c: Likewise.
* gcc.target/i386/indirect-thunk-inline-2.c: Likewise.
* gcc.target/i386/indirect-thunk-inline-3.c: Likewise.
* gcc.target/i386/indirect-thunk-inline-4.c: Likewise.
* gcc.target/i386/pr32219-2.c: Likewise.
* gcc.target/i386/pr32219-3.c: Likewise.
* gcc.target/i386/pr32219-4.c: Likewise.
* gcc.target/i386/pr32219-7.c: Likewise.
* gcc.target/i386/pr32219-8.c: Likewise.
* gcc.target/i386/ret-thunk-14.c: Likewise.
* gcc.target/i386/ret-thunk-15.c: Likewise.
* gcc.target/i386/ret-thunk-9.c: Likewise.
diff --git a/gcc/config/darwin.c b/gcc/config/darwin.c
index eefffee55f..8635fc2b44 100644
--- a/gcc/config/darwin.c
+++ b/gcc/config/darwin.c
@@ -495,7 +495,7 @@ indirection_hasher::equal (machopic_indirection *s, const
char *k)
/* Return the name of the non-lazy pointer (if STUB_P is false) or
stub (if STUB_B is true) corresponding to the given name.
- If we have a situation like:
+ PR71767 - If we have a situation like:
global_weak_symbol:
@@ -504,36 +504,22 @@ Lnon_weak_local:
ld64 will be unable to split this into two atoms (because the "L" makes
the second symbol 'invisible'). This means that legitimate direct accesses
- to the second symbol will appear to be non-allowed direct accesses to an
- atom of type weak, global which are not allowed.
+ to the second symbol will appear to be direct accesses to an atom of type
+ weak, global which are not allowed.
- To avoid this, we make the indirections have a leading 'l' (lower-case L)
- which has a special meaning: linker can see this and use it to determine
- atoms, but it is not placed into the final symbol table.
-
- The implementation here is somewhat heavy-handed in that it will also mark
- indirections to the __IMPORT,__pointers section the same way which is
- really unnecessary, since ld64 _can_ split those into atoms as they are
- fixed size. FIXME: determine if this is a penalty worth extra code to
- fix.
+ To avoid this, we make any data-section indirections have a leading 'l'
+ (lower-case L) which has a special meaning: linker can see this and use
+ it to determine atoms, but it is not placed into the final symbol table.
+ Symbols in the non-lazy symbol pointers section (or stubs) do not have this
+ problem because ld64 already knows the size of each entry.
*/
const char *
machopic_indirection_name (rtx sym_ref, bool stub_p)
{
- char *buffer;
const char *name = XSTR (sym_ref, 0);
- size_t namelen = strlen (name);
- machopic_indirection *p;
- bool needs_quotes;
- const char *suffix;
- char L_or_l = 'L';
- const char *prefix = user_label_prefix;
- const char *quote = "";
- tree id;
-
- id = maybe_get_identifier (name);
+ tree id = maybe_get_identifier (name);
if (id)
{
tree id_orig = id;
@@ -541,43 +527,47 @@ machopic_indirection_name (rtx sym_ref,
[Darwin, machopic 7/n, committed] Remove code that should be dead.
This code fragment was imported from the Apple branch (it was never
applied to mainline). It is stated to fix up a problem sometimes
created by reload (before that had been extended to have greater
flexibility in assigning the pic registers). In any event, reload
is no longer in use for the port.
tested on i686-darwin9, x86_64-darwin16 (m32/m64),
applied to mainline,
Iain
gcc/ChangeLog:
2019-10-13 Iain Sandoe
* config/darwin.c (machopic_indirect_data_reference): Remove
redundant code.
diff --git a/gcc/config/darwin.c b/gcc/config/darwin.c
index f6543fc997..eefffee55f 100644
--- a/gcc/config/darwin.c
+++ b/gcc/config/darwin.c
@@ -760,21 +760,6 @@ machopic_indirect_data_reference (rtx orig, rtx reg)
else if (GET_CODE (orig) == PLUS)
{
rtx base, result;
- /* When the target is i386, this code prevents crashes due to the
- compiler's ignorance on how to move the PIC base register to
- other registers. (The reload phase sometimes introduces such
- insns.) */
- if (GET_CODE (XEXP (orig, 0)) == REG
- && REGNO (XEXP (orig, 0)) == PIC_OFFSET_TABLE_REGNUM
- /* Prevent the same register from being erroneously used
- as both the base and index registers. */
- && (DARWIN_X86 && (GET_CODE (XEXP (orig, 1)) == CONST))
- && reg)
- {
- emit_move_insn (reg, XEXP (orig, 0));
- XEXP (ptr_ref, 0) = reg;
- return ptr_ref;
- }
/* Legitimize both operands of the PLUS. */
base = machopic_indirect_data_reference (XEXP (orig, 0), reg);
Make ipa-reference bitmaps dense
Hi,
this patch makes ipa-reference to assign sequential IDS to the static
variables to make bitmaps more dense. DECL_UID is not very good choice
since at WPA time just very small percentage of DECLs are static vars.
The memory use for ipa-reference bitmaps after patch is 83MB compared to
197MB before the patch.
More savings are posible by inveting the bitmaps - the pass collect
static not written/read so there is no need to check what decls are
actually tracked by the pass or not. Since we need hash map lookup
anyway it is eas yto save statics read/written instead which is smaller
overall.
I will do this incrementally.
Honza
Index: ChangeLog
===
--- ChangeLog (revision 276941)
+++ ChangeLog (working copy)
@@ -1,3 +1,17 @@
+2019-10-13 Jan Hubicka
+
+ * ipa-reference.h (ipa_reference_var_uid): Move offline.
+ * ipa-reference.c (reference_vars_map_t): new type.
+ (ipa_reference_vars_map, ipa_reference_vars_uids): New static vars.
+ (ipa_reference_var_uid): Implement.
+ (is_improper): Do not check bitmap for id==-1
+ (get_static_name): Update.
+ (ipa_init): Initialize new datastructures.
+ (analyze_function): Do not recompute ids.
+ (propagate): Free reference_vars_to_consider.
+ (stream_out_bitmap): Update.
+ (ipa_reference_read_optimization_summary): Update.
+
2019-10-13 Nathan Sidwell
* gengtype-lex.l (CXX_KEYWORD): Add 'mutable'.
Index: ipa-reference.c
===
--- ipa-reference.c (revision 276935)
+++ ipa-reference.c (working copy)
@@ -93,9 +93,10 @@ typedef struct ipa_reference_vars_info_d
/* This map contains all of the static variables that are
being considered by the compilation level alias analysis. */
-typedef hash_map, tree>
-reference_vars_to_consider_t;
-static reference_vars_to_consider_t *reference_vars_to_consider;
+typedef hash_map reference_vars_map_t;
+static reference_vars_map_t *ipa_reference_vars_map;
+static int ipa_reference_vars_uids;
+static vec *reference_vars_to_consider;
/* Set of all interesting module statics. A bit is set for every module
static we are considering. This is added to the local info when asm
@@ -137,6 +138,31 @@ public:
static ipa_ref_opt_summary_t *ipa_ref_opt_sum_summaries = NULL;
+/* Return ID used by ipa-reference bitmaps. -1 if failed. */
+int
+ipa_reference_var_uid (tree t)
+{
+ if (!ipa_reference_vars_map)
+return -1;
+ int *id = ipa_reference_vars_map->get
+(symtab_node::get (t)->ultimate_alias_target (NULL)->decl);
+ if (!id)
+return -1;
+ return *id;
+}
+
+/* Return ID used by ipa-reference bitmaps. Create new entry if
+ T is not in map. Set EXISTED accordinly */
+int
+ipa_reference_var_get_or_insert_uid (tree t, bool *existed)
+{
+ int = ipa_reference_vars_map->get_or_insert
+(symtab_node::get (t)->ultimate_alias_target (NULL)->decl, existed);
+ if (!*existed)
+id = ipa_reference_vars_uids++;
+ return id;
+}
+
/* Return the ipa_reference_vars structure starting from the cgraph NODE. */
static inline ipa_reference_vars_info_t
get_reference_vars_info (struct cgraph_node *node)
@@ -257,7 +283,9 @@ is_improper (symtab_node *n, void *v ATT
static inline bool
is_proper_for_analysis (tree t)
{
- if (bitmap_bit_p (ignore_module_statics, ipa_reference_var_uid (t)))
+ int id = ipa_reference_var_uid (t);
+
+ if (id != -1 && bitmap_bit_p (ignore_module_statics, id))
return false;
if (symtab_node::get (t)
@@ -273,7 +301,7 @@ is_proper_for_analysis (tree t)
static const char *
get_static_name (int index)
{
- return fndecl_name (*reference_vars_to_consider->get (index));
+ return fndecl_name ((*reference_vars_to_consider)[index]);
}
/* Dump a set of static vars to FILE. */
@@ -414,8 +442,17 @@ ipa_init (void)
ipa_init_p = true;
- if (dump_file)
-reference_vars_to_consider = new reference_vars_to_consider_t(251);
+ vec_alloc (reference_vars_to_consider, 10);
+
+
+ if (ipa_ref_opt_sum_summaries != NULL)
+{
+ delete ipa_ref_opt_sum_summaries;
+ ipa_ref_opt_sum_summaries = NULL;
+ delete ipa_reference_vars_map;
+}
+ ipa_reference_vars_map = new reference_vars_map_t(257);
+ ipa_reference_vars_uids = 0;
bitmap_obstack_initialize (_info_obstack);
bitmap_obstack_initialize (_summary_obstack);
@@ -424,12 +461,6 @@ ipa_init (void)
if (ipa_ref_var_info_summaries == NULL)
ipa_ref_var_info_summaries = new ipa_ref_var_info_summary_t (symtab);
-
- if (ipa_ref_opt_sum_summaries != NULL)
-{
- delete ipa_ref_opt_sum_summaries;
- ipa_ref_opt_sum_summaries = NULL;
-}
}
@@ -464,6 +495,8 @@ analyze_function (struct cgraph_node *fn
local = init_function_info (fn);
for (i = 0; fn->iterate_reference (i, ref); i++)
{
+ int id;
+ bool existed;
if (!is_a (ref->referred))
Re: [PATCH] Fix dump message issue
On 10/8/19 4:45 AM, Martin Jambor wrote: > Hi, > > On Tue, Oct 08 2019, luoxhu wrote: >> '}' is missed at the end. > > heh, yeah, I wonder for how long. > > If it irritates you, I'd say the patch is obvious (though note that I > cannot approve a patch in this area). Looks obvious to me. And while you may not be an official reviewer Martin, if you say someone's code looks good to you, I'm just going to rubber stamp it. Which in turn means you ought to be a reviewer. Jeff
Re: [patch] canonicalize unsigned [1,MAX] ranges into ~[0,0]
On 10/7/19 6:28 AM, Aldy Hernandez wrote:
>
> Fair enough. I guess I don't care what we settle on, inasmuch as we
> canonicalize into one true value. For some reason, I thought the above
> nonzero would cause you to cringe, I guess not :).
:-) Takes more than that these days.. And just to reiterate for the
larger audience what we discussed in IRC -- I'm absolutely for
canonicalization. We're just debating what the canonical form should be
and how to get there.
>
> Happily, normalizing into ~0 for signed and [1,MAX] for unsigned,
> simplified the patch because it on longer needs tweaks to
> ranges_from_anti_range.
Looks like it was ultimately far smaller than I expected. Unless we
haven't audited existing code looking for open-coded ~[0,0] tests.
>
> OK for trunk?
With a ChangeLog, yes.
>
> Aldy
>
> p.s. This still leaves open the issue with ipa_vr's handling of
> nonzero_p. As per my last message, I've adjusted it to check for either
> ~[0,0] or the [1,MAX] variant for unsigned, since before this patch
> there were two ways of representing the same thing. However, ipa-prop
> has no API (it open codes everything), as it has rolled its own version
> of "value ranges" with wide-ints.
>
> class GTY(()) ipa_vr
> {
> public:
> /* The data fields below are valid only if known is true. */
> bool known;
> enum value_range_kind type;
> wide_int min;
> wide_int max;
> bool nonzero_p (tree) const;
> };
>
> I'm tempted to leave the nonzero_p which checks for both ~0 and [1,MAX]:
>
> bool
> ipa_vr::nonzero_p (tree expr_type) const
> {
> if (type == VR_ANTI_RANGE && wi::eq_p (min, 0) && wi::eq_p (max, 0))
> return true;
>
> unsigned prec = TYPE_PRECISION (expr_type);
> return (type == VR_RANGE
> && TYPE_UNSIGNED (expr_type)
> && wi::eq_p (min, wi::one (prec))
> && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN (expr_type;
> }
>
> I don't care either way.
Sigh. I can live with either here as well. I'm hesitant to start
mucking around with it simply because it's well outside of where we've
got expertise.
jeff
Re: [PATCH][MSP430] Add support for post increment addressing
On 10/7/19 2:28 PM, Jozef Lawrynowicz wrote:
> MSP430 supports post increment addressing for the source operand only. The
> attached patch enables post increment addressing for MSP430 in GCC.
>
> Regtested on trunk for the small and large memory models.
>
> Ok for trunk?
>
>
> 0001-MSP430-Implement-post-increment-addressing.patch
>
> From d75e48ba434d7bab141c09d442793b2cb26afa69 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz
> Date: Mon, 16 Sep 2019 16:34:51 +0100
> Subject: [PATCH] MSP430: Implement post increment addressing
>
> gcc/ChangeLog:
>
> 2019-10-07 Jozef Lawrynowicz
>
> * config/msp430/constraints.md: Allow post_inc operand for "Ya"
> constraint.
> * config/msp430/msp430.c (msp430_legitimate_address_p): Handle
> POST_INC.
> (msp430_subreg): Likewise.
> (msp430_split_addsi): Likewise.
> (msp430_print_operand_addr): Likewise.
> * config/msp430/msp430.h (HAVE_POST_INCREMENT): Define.
> (USE_STORE_POST_INCREMENT): Define.
> * config/msp430/msp430.md: Use the msp430_general_dst_operand or
> msp430_general_dst_nonv_operand predicates for the lvalues insns.
> * config/msp430/predicates.md (msp430_nonpostinc_operand): New.
> (msp430_general_dst_operand): New.
> (msp430_general_dst_nonv_operand): New.
> (msp430_nonsubreg_operand): Remove.
> (msp430_nonsubreg_dst_operand): New.
> (msp430_nonsubreg_or_imm_operand): Allow reg or mem operands in place
> of defunct msp430_nonsubreg_operand.
> (msp430_nonsubregnonpostinc_or_imm_operand): New.
So a high level question. The
USE_STORE_{PRE,POST}_{INCREMENT,DECREMENT} are primarily used within the
ivopts code to tune the addressing modes generated in there.
To the best of my knowledge, they do not totally prevent using those
addressing modes elsewhere (auto-inc-dec.c) which instead rely strictly
on the HAVE_ macros and recognizing the result against the MD file.
So will these changes handle auto-increment addressing modes in
destination operands if they are generated by auto-inc-dec.c? Or have
you fixed all the predicates so that auto-inc-dec.c won't create them in
the first place?
Based on a comment in msp430_split_addsi you have to handle stuff like
> + (set (reg:SI)
> + (plus:SI (reg:SI)
> +(mem:SI (post_inc:PSI (reg:PSI).
Do you need to check for and do anything special if the destination
operand is the same as the post-inc operand. Note RTX equality test is
probably not sufficient since you've got differing modes. Note this may
be affected by the prior question...
Are there any places where you could potentially have two source memory
operands? If so, do you need additional checks in those patterns?
I've got no conceptual problem with the patch, I just want to make sure
you've thought about those issues.
jeff
Re: Add a constant_range_value_p function (PR 92033)
On 10/11/19 10:42 AM, Richard Sandiford wrote:
The range-tracking code has a pretty hard-coded assumption that
is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
ADDR_EXPR". It seems better to add a predicate specifically for
that rather than contiually fight cases in which it can't handle
other invariants.
I was going to suggest we normalize ranges to numerics completely before
folding. That is, replacing normalize_addresses() here:
*vr = op->fold_range (expr_type,
vr0.normalize_addresses (),
vr1.normalize_addresses ());
...into normalize_symbolics(). But I suppose getting the gate correct
is even better. Thanks for taking the care of this extensive and manual
change.
The patch looks good to me. However, I do wonder if VRP and
subsidiaries can't handle non-integer invariants, if we shouldn't
disallow them from the setters as well:
void
value_range_base::set (tree val)
{
gcc_assert (TREE_CODE (val) == SSA_NAME || is_gimple_min_invariant
(val));
if (TREE_OVERFLOW_P (val))
val = drop_tree_overflow (val);
set (VR_RANGE, val, val);
}
void
value_range::set (tree val)
{
gcc_assert (TREE_CODE (val) == SSA_NAME || is_gimple_min_invariant
(val));
if (TREE_OVERFLOW_P (val))
val = drop_tree_overflow (val);
set (VR_RANGE, val, val, NULL);
}
This would still allow setting of VARYING and UNDEFINED, but disallow
poly-ints, etc from a range.
Was this a small oversight, or was there a reason you left those in?
Aldy
Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
Richard
2019-10-11 Richard Sandiford
gcc/
PR tree-optimization/92033
* tree-vrp.h (constant_range_value_p): Declare.
* tree-vrp.c (constant_range_value_p): New function.
(value_range_base::symbolic_p, value_range_base::singleton_p)
(get_single_symbol, compare_values_warnv, intersect_ranges)
(value_range_base::normalize_symbolics): Use it instead of
is_gimple_min_invariant.
(simplify_stmt_for_jump_threading): Likewise.
* vr-values.c (symbolic_range_based_on_p, valid_value_p): Likewise.
(vr_values::op_with_constant_singleton_value_range): Likewise.
(vr_values::extract_range_from_binary_expr): Likewise.
(vr_values::extract_range_from_unary_expr): Likewise.
(vr_values::extract_range_from_cond_expr): Likewise.
(vr_values::extract_range_from_comparison): Likewise.
(vr_values::extract_range_from_assignment): Likewise.
(vr_values::adjust_range_with_scev, vrp_valueize): Likewise.
(vr_values::vrp_visit_assignment_or_call): Likewise.
(vr_values::vrp_evaluate_conditional): Likewise.
(vr_values::simplify_bit_ops_using_ranges): Likewise.
(test_for_singularity): Likewise.
(vr_values::simplify_cond_using_ranges_1): Likewise.
Index: gcc/tree-vrp.h
===
--- gcc/tree-vrp.h 2019-10-08 09:23:31.282533990 +0100
+++ gcc/tree-vrp.h 2019-10-11 15:41:20.380576059 +0100
@@ -284,6 +284,7 @@ value_range_base::supports_type_p (tree
return false;
}
+extern bool constant_range_value_p (const_tree);
extern void register_edge_assert_for (tree, edge, enum tree_code,
tree, tree, vec &);
extern bool stmt_interesting_for_vrp (gimple *);
Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c 2019-10-08 09:23:31.282533990 +0100
+++ gcc/tree-vrp.c 2019-10-11 15:41:20.380576059 +0100
@@ -78,6 +78,18 @@ ranges_from_anti_range (const value_rang
for still active basic-blocks. */
static sbitmap *live;
+/* Return true if VALUE is considered constant for range tracking.
+ This is stricter than is_gimple_min_invariant and should be
+ used instead of it in range-related code. */
+
+bool
+constant_range_value_p (const_tree value)
+{
+ return (TREE_CODE (value) == INTEGER_CST
+ || (TREE_CODE (value) == ADDR_EXPR
+ && is_gimple_invariant_address (value)));
+}
+
void
value_range::set_equiv (bitmap equiv)
{
@@ -273,8 +285,8 @@ value_range_base::symbolic_p () const
{
return (!varying_p ()
&& !undefined_p ()
- && (!is_gimple_min_invariant (m_min)
- || !is_gimple_min_invariant (m_max)));
+ && (!constant_range_value_p (m_min)
+ || !constant_range_value_p (m_max)));
}
/* NOTE: This is not the inverse of symbolic_p because the range
@@ -388,7 +400,7 @@ value_range_base::singleton_p (tree *res
}
if (m_kind == VR_RANGE
&& vrp_operand_equal_p (min (), max ())
- && is_gimple_min_invariant (min ()))
+ && constant_range_value_p (min ()))
{
if (result)
*result = min ();
@@ -953,13 +965,13 @@ get_single_symbol (tree t, bool *neg, tr
|| TREE_CODE (t) == POINTER_PLUS_EXPR
Re: [patch, fortran] Fix PR 92004, restore Lapack compilation
OK, so here's the update. There was a problem with uninitialized
variables, which for some reason was not detected on compilation.
OK for trunk?
2019-10-13 Thomas Koenig
PR fortran/92004
* array.c (expand_constructor): Set from_constructor on
expression.
* gfortran.h (gfc_symbol): Add maybe_array.
(gfc_expr): Add from_constructor.
* interface.c (maybe_dummy_array_arg): New function.
(compare_parameter): If the formal argument is generated from a
call, check the conditions where an array element could be
passed to an array. Adjust error message for assumed-shape
or pointer array. Use correct language for assumed shaped arrays.
(gfc_get_formal_from_actual_arglist): Set maybe_array on the
symbol if the actual argument is an array element fulfilling
the conditions of 15.5.2.4.
2019-10-13 Thomas Koenig
PR fortran/92004
* gfortran.dg/argument_checking_24.f90: New test.
* gfortran.dg/abstract_type_6.f90: Add error message.
* gfortran.dg/argument_checking_11.f90: Correct wording
in error message.
* gfortran.dg/argumeent_checking_13.f90: Likewise.
* gfortran.dg/interface_40.f90: Add error message.
Index: fortran/array.c
===
--- fortran/array.c (Revision 276506)
+++ fortran/array.c (Arbeitskopie)
@@ -1763,6 +1763,7 @@ expand_constructor (gfc_constructor_base base)
gfc_free_expr (e);
return false;
}
+ e->from_constructor = 1;
current_expand.offset = >offset;
current_expand.repeat = >repeat;
current_expand.component = c->n.component;
Index: fortran/gfortran.h
===
--- fortran/gfortran.h (Revision 276506)
+++ fortran/gfortran.h (Arbeitskopie)
@@ -1614,6 +1614,9 @@ typedef struct gfc_symbol
/* Set if a previous error or warning has occurred and no other
should be reported. */
unsigned error:1;
+ /* Set if the dummy argument of a procedure could be an array despite
+ being called with a scalar actual argument. */
+ unsigned maybe_array:1;
int refs;
struct gfc_namespace *ns; /* namespace containing this symbol */
@@ -2194,6 +2197,11 @@ typedef struct gfc_expr
/* Set this if no warning should be given somewhere in a lower level. */
unsigned int do_not_warn : 1;
+
+ /* Set this if the expression came from expanding an array constructor. */
+
+ unsigned int from_constructor : 1;
+
/* If an expression comes from a Hollerith constant or compile-time
evaluation of a transfer statement, it may have a prescribed target-
memory representation, and these cannot always be backformed from
Index: fortran/interface.c
===
--- fortran/interface.c (Revision 276506)
+++ fortran/interface.c (Arbeitskopie)
@@ -2229,6 +2229,67 @@ argument_rank_mismatch (const char *name, locus *w
}
+/* Under certain conditions, a scalar actual argument can be passed
+ to an array dummy argument - see F2018, 15.5.2.4, paragraph 14.
+ This function returns true for these conditions so that an error
+ or warning for this can be suppressed later. Always return false
+ for expressions with rank > 0. */
+
+bool
+maybe_dummy_array_arg (gfc_expr *e)
+{
+ gfc_symbol *s;
+ gfc_ref *ref;
+ bool array_pointer = false;
+ bool assumed_shape = false;
+ bool scalar_ref = true;
+
+ if (e->rank > 0)
+return false;
+
+ if (e->ts.type == BT_CHARACTER && e->ts.kind == 1)
+return true;
+
+ /* If this comes from a constructor, it has been an array element
+ originally. */
+
+ if (e->expr_type == EXPR_CONSTANT)
+return e->from_constructor;
+
+ if (e->expr_type != EXPR_VARIABLE)
+return false;
+
+ s = e->symtree->n.sym;
+
+ if (s->attr.dimension)
+{
+ scalar_ref = false;
+ array_pointer = s->attr.pointer;
+}
+
+ if (s->as && s->as->type == AS_ASSUMED_SHAPE)
+assumed_shape = true;
+
+ for (ref=e->ref; ref; ref=ref->next)
+{
+ if (ref->type == REF_COMPONENT)
+ {
+ symbol_attribute *attr;
+ attr = >u.c.component->attr;
+ if (attr->dimension)
+ {
+ array_pointer = attr->pointer;
+ assumed_shape = false;
+ scalar_ref = false;
+ }
+ else
+ scalar_ref = true;
+ }
+}
+
+ return !(scalar_ref || array_pointer || assumed_shape);
+}
+
/* Given a symbol of a formal argument list and an expression, see if
the two are compatible as arguments. Returns true if
compatible, false if not compatible. */
@@ -2544,7 +2605,9 @@ compare_parameter (gfc_symbol *formal, gfc_expr *a
|| (actual->rank == 0 && formal->attr.dimension
&& gfc_is_coindexed (actual)))
{
- if (where)
+ if (where
+ && (!formal->attr.artificial || (!formal->maybe_array
+ && !maybe_dummy_array_arg (actual
{
locus
Re: Add expr_callee_abi
On 10/11/19 8:39 AM, Richard Sandiford wrote: > This turned out to be useful for the SVE PCS support, and is a natural > tree-level analogue of insn_callee_abi. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? > > Richard > > > 2019-10-11 Richard Sandiford > > gcc/ > * function-abi.h (expr_callee_abi): Declare. > * function-abi.cc (expr_callee_abi): New function. OK. Yes I realize you're not using it on the trunk yet :-) jeff
Re: [PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)
>
> Patch bootstrapped and regression tested on arm-none-linux-gnueabihf,
> however, on my native Aarch32 setup the test times out when run as part
> of a big "make check-gcc" regression, but not when run individually.
>
> 2019-10-11 Stamatis Markianos-Wright
>
> * config/arm/arm.md: Update b for Thumb2 range checks.
> * config/arm/arm.c: New function arm_gen_far_branch.
> * config/arm/arm-protos.h: New function arm_gen_far_branch
> prototype.
>
> gcc/testsuite/ChangeLog:
>
> 2019-10-11 Stamatis Markianos-Wright
>
> * testsuite/gcc.target/arm/pr91816.c: New test.
> diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
> index f995974f9bb..1dce333d1c3 100644
> --- a/gcc/config/arm/arm-protos.h
> +++ b/gcc/config/arm/arm-protos.h
> @@ -570,4 +570,7 @@ void arm_parse_option_features (sbitmap, const
> cpu_arch_option *,
>
> void arm_initialize_isa (sbitmap, const enum isa_feature *);
>
> +const char * arm_gen_far_branch (rtx *, int,const char * , const char *);
> +
> +
Lets get the nits out of the way.
Unnecessary extra new line, need a space between int and const above.
> #endif /* ! GCC_ARM_PROTOS_H */
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 39e1a1ef9a2..1a693d2ddca 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -32139,6 +32139,31 @@ arm_run_selftests (void)
> }
> } /* Namespace selftest. */
>
> +
> +/* Generate code to enable conditional branches in functions over 1 MiB. */
> +const char *
> +arm_gen_far_branch (rtx * operands, int pos_label, const char * dest,
> + const char * branch_format)
Not sure if this is some munging from the attachment but check
vertical alignment of parameters.
> +{
> + rtx_code_label * tmp_label = gen_label_rtx ();
> + char label_buf[256];
> + char buffer[128];
> + ASM_GENERATE_INTERNAL_LABEL (label_buf, dest , \
> + CODE_LABEL_NUMBER (tmp_label));
> + const char *label_ptr = arm_strip_name_encoding (label_buf);
> + rtx dest_label = operands[pos_label];
> + operands[pos_label] = tmp_label;
> +
> + snprintf (buffer, sizeof (buffer), "%s%s", branch_format , label_ptr);
> + output_asm_insn (buffer, operands);
> +
> + snprintf (buffer, sizeof (buffer), "b\t%%l0%d\n%s:", pos_label, label_ptr);
> + operands[pos_label] = dest_label;
> + output_asm_insn (buffer, operands);
> + return "";
> +}
> +
> +
Unnecessary extra newline.
> #undef TARGET_RUN_TARGET_SELFTESTS
> #define TARGET_RUN_TARGET_SELFTESTS selftest::arm_run_selftests
> #endif /* CHECKING_P */
> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> index f861c72ccfc..634fd0a59da 100644
> --- a/gcc/config/arm/arm.md
> +++ b/gcc/config/arm/arm.md
> @@ -6686,9 +6686,16 @@
> ;; And for backward branches we have
> ;; (neg_range - neg_base_offs + pc_offs) = (neg_range - (-2 or -4) + 4).
> ;;
> +;; In 16-bit Thumb these ranges are:
> ;; For a 'b' pos_range = 2046, neg_range = -2048 giving (-2040->2048).
> ;; For a 'b' pos_range = 254, neg_range = -256 giving (-250 ->256).
>
> +;; In 32-bit Thumb these ranges are:
> +;; For a 'b' +/- 16MB is not checked for.
> +;; For a 'b' pos_range = 1048574, neg_range = -1048576 giving
> +;; (-1048568 -> 1048576).
> +
> +
Unnecessary extra newline.
> (define_expand "cbranchsi4"
>[(set (pc) (if_then_else
> (match_operator 0 "expandable_comparison_operator"
> @@ -6947,22 +6954,42 @@
> (pc)))]
>"TARGET_32BIT"
>"*
> - if (arm_ccfsm_state == 1 || arm_ccfsm_state == 2)
> -{
> - arm_ccfsm_state += 2;
> - return \"\";
> -}
> - return \"b%d1\\t%l0\";
> + if (arm_ccfsm_state == 1 || arm_ccfsm_state == 2)
> + {
> + arm_ccfsm_state += 2;
> + return \"\";
> + }
> + switch (get_attr_length (insn))
> + {
> + // Thumb2 16-bit b{cond}
> + case 2:
> +
> + // Thumb2 32-bit b{cond}
> + case 4: return \"b%d1\\t%l0\";break;
> +
> + // Thumb2 b{cond} out of range. Use unconditional branch.
> + case 8: return arm_gen_far_branch \
> + (operands, 0, \"Lbcond\", \"b%D1\t\");
> + break;
> +
> + // A32 b{cond}
> + default: return \"b%d1\\t%l0\";
> + }
Please fix indentation here.
>"
>[(set_attr "conds" "use")
> (set_attr "type" "branch")
> (set (attr "length")
> - (if_then_else
> -(and (match_test "TARGET_THUMB2")
> - (and (ge (minus (match_dup 0) (pc)) (const_int -250))
> - (le (minus (match_dup 0) (pc)) (const_int 256
> -(const_int 2)
> -(const_int 4)))]
> + (if_then_else (match_test "TARGET_THUMB2")
> + (if_then_else (and (ge (minus (match_dup 0) (pc)) (const_int -250))
> + (le (minus (match_dup 0) (pc)) (const_int 256)))
> + (const_int 2)
> + (if_then_else (and (ge (minus (match_dup 0) (pc))
> + (const_int -1048568))
Re: [PATCH] Fix -Wshadow=local warnings in elfos.h
On 10/5/19 12:16 AM, Bernd Edlinger wrote: > > > On 10/4/19 10:26 PM, Jeff Law wrote: >> On 10/4/19 12:24 PM, Bernd Edlinger wrote: >>> Hi, >>> >>> these macros don't use reserved names for local variables. >>> This caused -Wshadow=local warnings in varasm.c IIRC. >>> >>> Fixed by using _lowercase reserved names for macro parameters. >>> >>> >>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu. >>> Is it OK for trunk? >>> >>> >>> Thanks >>> Bernd. >>> >>> >>> >>> patch-wshadow-elfos.diff >>> >>> 2019-10-04 Bernd Edlinger >>> >>> * config/elfos.h (ASM_DECLARE_OBJECT_NAME, >>> ASM_FINISH_DECLARE_OBJECT): Rename local vars in macro. >> Same objections as before. As long as we're using macros like this, >> we're going to have increased potential for shadowing problems and >> macros which touch implementation details that just happen to be >> available in the context where the macro is used. >> >> Convert to real functions. It avoids the shadowing problem and avoids >> macros touching/referencing things they shouldn't. Code in macros may >> have been reasonable in the 80s/90s, but we should know better by now. >> >> I'm not ranting against you Bernd, it's more a rant against the original >> coding style for GCC. Your changes just highlight how bad of an idea >> this kind of macro usage really is. We should take the opportunity to >> fix this stuff for real. >> > > I understand that, and would not propose to fix it this way if this > was the *only* place that breaks with -Wshadow=local. > > I will continue to send more patches over the weekend, formally obvious > ones, but the amount of changes is just huge. > > I would suggest I send them here, and wait for at least 24H before > committing, so anybody can suggest better names, for instance, > or other (small) improvements, as you like. I've been away for the last week. I hope you've seen that the concerns folks are raising are a good indicator that the current approach isn't how we want to fix these issues. Factoring/refactoring, hooks, and _sensible_ renaming of variables are the way forward. Underscores and numeric prefixes/suffixes are just making things worse. My recommendation would be to look at an opt-out style. ie, add the new warning, but explicitly disable it (in Makefile.in) for every file were we're not clean yet. There's already some mechanisms to do this in Makefile.in where we disable specific warnings for specific files. Then address one file at a time in the right way then remove that file from the opt-out list. Continue until done :-) I realize this will be a lot of work. jeff
[PATCH] teach gengtype about 'mutable'
In constifying some more of line-map I discovered gengtype didn't know mutable.
Added thusly.
nathan
--
Nathan Sidwell
2019-10-13 Nathan Sidwell
* gengtype-lex.l (CXX_KEYWORD): Add 'mutable'.
Index: gcc/gengtype-lex.l
===
--- gcc/gengtype-lex.l (revision 275726)
+++ gcc/gengtype-lex.l (working copy)
@@ -58,7 +58,7 @@ ITYPE {IWORD}({WS}{IWORD})*
/* Include '::' in identifiers to capture C++ scope qualifiers. */
ID {CID}({HWS}::{HWS}{CID})*
EOID [^[:alnum:]_]
-CXX_KEYWORD inline|public:|private:|protected:|template|operator|friend|static
+CXX_KEYWORD inline|public:|private:|protected:|template|operator|friend|static|mutable
%x in_struct in_struct_comment in_comment
%option warn noyywrap nounput nodefault perf-report
[patch, fortran, committed] Fix PR 92017
Hello world,
I have committed the attached patch as obvious and simple after
regression-testing.
It fixes an ICE on valid for a corner case, so I don't really
feel that it needs to be backported. If anybody disagrees,
please speak up (or do it yourself :-)
Regards
Thomas
2019-10-13 Thomas Koenig
PR fortran/92017
* expr.c (simplify_parameter_variable): Set the character length
of the result expression from the original expression if
necessary.
2019-10-13 Thomas Koenig
PR fortran/92017
* gfortran.dg/minmaxloc_14.f90: New test.
Index: expr.c
===
--- expr.c (Revision 276937)
+++ expr.c (Arbeitskopie)
@@ -2066,6 +2066,9 @@ simplify_parameter_variable (gfc_expr *p, int type
e->rank = p->rank;
+ if (e->ts.type == BT_CHARACTER && e->ts.u.cl == NULL)
+e->ts.u.cl = gfc_new_charlen (gfc_current_ns, p->ts.u.cl);
+
/* Do not copy subobject refs for constant. */
if (e->expr_type != EXPR_CONSTANT && p->ref != NULL)
e->ref = gfc_copy_ref (p->ref);
! { dg-do compile }
! PR 92017 - this used to cause an ICE due do a missing charlen.
! Original test case by Gerhard Steinmetz.
program p
character(3), parameter :: a(4) = 'abc'
integer, parameter :: b(1) = minloc(a)
integer, parameter :: c = minloc(a, dim=1)
integer, parameter :: bb(1) = maxloc(a)
integer, parameter :: c2 = maxloc(a,dim=1)
end program p
Re: [patch, fortran] Fix PR 92004, restore Lapack compilation
Hm, my trunk is doing strange things (debugging not working), and I think I have found an additional problem. I'll need some time to work this out, and will resubmit. Regards Thomas
Re: Make C2X imply -fno-fp-int-builtin-inexact
Rainer Orth writes: I’m not quite sure what you’re proposing here (probably missing something obvious). >>> >>> At the moment, gcc/testsuite/lib/target-supports.exp >>> (add_options_for_c99_runtime) adds -mmacosx-version-min=10.3 to the >>> testcase flags on powerpc-*-darwin*. Since, as Joseph mentioned, gcc >>> now defaults to -std=gnu11 (which implies a C99 runtime), this (or >>> something similar) would always be needed now (unless someone forces, >>> say, -std=c90) and should be handled in the Darwin/PowerPC driver code, >>> not just the testsuite. >> >> The driver has the following rules: >> >> * if the user puts -mmacosx-version-min= on the command line that trumps >> all >> >> * else we pick a default using the following priority. >> 1. MACOSX_DEPLOYMENT_TARGET env var. >> 2. (native) >> - what the kernel returns for the system version. >> (cross) >> - what was set at configure time for DEF_MIN_OSX_VERSION (which will >>be >= 10.3.9 as things stand) >> >> So, of course, if we were hosted on 10.2.8 - that would create a problem >> (2, native) >> but if the system doesn’t have c99 support, that’s a problem anyway. >> >> Otherwise, deliberate mis-configuration or passing -mmacosx-version-min= in >> RUNTESTFLAGS … but we don’t need to support that. > > Right: users should be allowed to shoot themselves ;-) > >> Therefore, I suspect that the addition of the "-mmacosx-version-min=10.3” >> is not >> necessary and is a hang-over from older toolchains. > > I know now what's going on... > >> Perhaps, we could have a target_supports_c99 (maybe we already do), since >> it’s >> feasible that some embedded platforms might also want that exclusion - that >> would >> cover earlier Darwin “automagically” assuming folks remember to apply it. > > We do, and it's unsurprisingly called c99_runtime, too: in this as in a > few other cases where a specific feature may need additional options to > enable it, we have dg-add-options corresponding to the > . > > What's happening for c99_runtime, as can be seen in > lib/target-supports.exp (check_effective_target_c99_runtime) is that it > checks gcc.dg/builtins-config.h for a definition of HAVE_C99_RUNTIME. > > In the Darwin/PowerPC case, that isn't defined before 10.3, skipping the > affected tests. However, if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ > isn't defined, builtins-config.h #error's out. Fortunately, gcc on > Darwin always passes -mmacosx-version-min now, so there's no need to > explicitly pass it in add_options_for_c99_runtime and that proc together > with all calls to dg-add-options c99_runtime can go unless something > really unexpected comes up during Solaris testing. Here's what I installed after successful i386-pc-solaris2.11, sparc-sun-solaris2.11, and x86_64-pc-linux-gnu testing. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University 2019-10-10 Rainer Orth gcc: * doc/sourcebuild.texi (Test Directives, Add Options): Remove c99_runtime. gcc/testsuite: * lib/target-supports.exp (add_options_for_c99_runtime): Remove. (check_effective_target_c99_runtime): Remove call to add_options_for_c99_runtime. * gcc.dg/builtins-18.c: Remove dg-add-options c99_runtime. * gcc.dg/builtins-20.c: Likewise. * gcc.dg/builtins-53.c: Likewise. * gcc.dg/builtins-55.c: Likewise. * gcc.dg/builtins-67.c: Likewise. * gcc.dg/c99-tgmath-1.c: Likewise. * gcc.dg/c99-tgmath-2.c: Likewise. * gcc.dg/c99-tgmath-3.c: Likewise. * gcc.dg/c99-tgmath-4.c: Likewise. * gcc.dg/ipa/inline-8.c: Likewise. * gcc.dg/ipa/ipa-icf-5.c: Likewise. * gcc.dg/ipa/ipa-icf-7.c: Likewise. * gcc.dg/nextafter-2.c: Likewise. * gcc.dg/pr42427.c: Likewise. * gcc.dg/pr78965.c: Likewise. * gcc.dg/single-precision-constant.c: Likewise. * gcc.dg/torture/builtin-convert-1.c: Likewise. * gcc.dg/torture/builtin-convert-2.c: Likewise. * gcc.dg/torture/builtin-convert-3.c: Likewise. * gcc.dg/torture/builtin-convert-4.c: Likewise. * gcc.dg/torture/builtin-fp-int-inexact.c: Likewise. * gcc.dg/torture/builtin-fp-int-inexact-c2x.c: Likewise. * gcc.dg/torture/builtin-integral-1.c: Likewise. * gcc.dg/torture/builtin-power-1.c: Likewise. * gcc.dg/tree-ssa/copy-sign-1.c: Likewise. * gcc.dg/tree-ssa/minmax-2.c: Likewise. * gcc.dg/tree-ssa/mult-abs-2.c: Likewise. * gcc.target/i386/387-builtin-fp-int-inexact.c: Likewise. * gcc.target/i386/387-rint-inline-1.c: Likewise. * gcc.target/i386/387-rint-inline-2.c: Likewise. * gcc.target/i386/conversion.c: Likewise. * gcc.target/i386/pr47312.c: Likewise. * gcc.target/i386/sse2-builtin-fp-int-inexact.c:
Grow GGC after cgraph and summary streaming
Hi,
we use ggc_grow to prevent garbage collector from triggering at WPA
time. After streaming in global trees we know that basically all of them
will stay reachable until end of WPA compilation.
This no longer works because tree memory use got down to about 123MB out
of 700MB of GGC memory needed for cc1 build.
Addition 105MB is used by symbol table and 145 by symbol summaries which
is all explicitly ggc_freed. So I am adding ggc_grow after those two
stremaing steps. I want to keep all three ggc_grow calls because with
checking collection is triggered and we keep testing that stuff if GGC
safe and no garbage is produced.
We still ggc collect once for cc1 WPA collecing of 118MB of garbage.
I think it is mostly produced by ICF but this needs to be
double-checked.
Bootstrapped/regtested x86_64-linux, comitted.
* lto-common.c (read_cgraph_and_symbols): Grow ggc memory use after
summary streaming.
Index: lto-common.c
===
--- lto-common.c(revision 276935)
+++ lto-common.c(working copy)
@@ -2781,7 +2781,6 @@ read_cgraph_and_symbols (unsigned nfiles
/* At this stage we know that majority of GGC memory is reachable.
Growing the limits prevents unnecesary invocation of GGC. */
ggc_grow ();
- ggc_collect ();
/* Set the hooks so that all of the ipa passes can read in their data. */
lto_set_in_hooks (all_file_decl_data, get_section_data, free_section_data);
@@ -2852,7 +2851,11 @@ read_cgraph_and_symbols (unsigned nfiles
if (tree_with_vars)
ggc_free (tree_with_vars);
tree_with_vars = NULL;
- ggc_collect ();
+ /* During WPA we want to prevent ggc collecting by default. Grow limits
+ until after the IPA summaries are streamed in. Basically all IPA memory
+ is explcitly managed by ggc_free and ggc collect is not useful.
+ Exception are the merged declarations. */
+ ggc_grow ();
timevar_pop (TV_IPA_LTO_DECL_MERGE);
/* Each pass will set the appropriate timer. */
@@ -2866,6 +2869,8 @@ read_cgraph_and_symbols (unsigned nfiles
else
ipa_read_summaries ();
+ ggc_grow ();
+
for (i = 0; all_file_decl_data[i]; i++)
{
gcc_assert (all_file_decl_data[i]->symtab_node_encoder);
Fix duplicated renumbering of statements during streaming
Hi,
currently we renumber statements during streaming twice - once using
renumber_gimple_stmt_uids and second inside output_function.
The numbering only differs by fact that first one does mix normal and
virtual PHI sets while the second doesn't. This patch reduces
renumbering to only one per streaming.
Honza
* lto.c (lto_wpa_write_files): Do not update bodies of clones.
* lto-streamer-out.c (collect_block_tree_leafs): Renumber statements
so non-virutal are before virutals.
(output_function): Avoid body modifications.
Index: lto/lto.c
===
--- lto/lto.c (revision 276870)
+++ lto/lto.c (working copy)
@@ -308,7 +308,7 @@ lto_wpa_write_files (void)
/* Do body modifications needed for streaming before we fork out
worker processes. */
FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
-if (gimple_has_body_p (node->decl))
+if (!node->clone_of && gimple_has_body_p (node->decl))
lto_prepare_function_for_streaming (node);
/* Generate a prefix for the LTRANS unit files. */
Index: lto-streamer-out.c
===
--- lto-streamer-out.c (revision 276870)
+++ lto-streamer-out.c (working copy)
@@ -2066,14 +2066,54 @@ collect_block_tree_leafs (tree root, vec
void
lto_prepare_function_for_streaming (struct cgraph_node *node)
{
- if (number_of_loops (DECL_STRUCT_FUNCTION (node->decl)))
+ struct function *fn = DECL_STRUCT_FUNCTION (node->decl);
+ basic_block bb;
+
+ if (number_of_loops (fn))
{
- push_cfun (DECL_STRUCT_FUNCTION (node->decl));
+ push_cfun (fn);
loop_optimizer_init (AVOID_CFG_MODIFICATIONS);
loop_optimizer_finalize ();
pop_cfun ();
}
- renumber_gimple_stmt_uids (DECL_STRUCT_FUNCTION (node->decl));
+ /* We will renumber the statements. The code that does this uses
+ the same ordering that we use for serializing them so we can use
+ the same code on the other end and not have to write out the
+ statement numbers. We do not assign UIDs to PHIs here because
+ virtual PHIs get re-computed on-the-fly which would make numbers
+ inconsistent. */
+ set_gimple_stmt_max_uid (fn, 0);
+ FOR_ALL_BB_FN (bb, fn)
+{
+ for (gphi_iterator gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
+ gsi_next ())
+ {
+ gphi *stmt = gsi.phi ();
+
+ /* Virtual PHIs are not going to be streamed. */
+ if (!virtual_operand_p (gimple_phi_result (stmt)))
+ gimple_set_uid (stmt, inc_gimple_stmt_max_uid (fn));
+ }
+ for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
+ gsi_next ())
+ {
+ gimple *stmt = gsi_stmt (gsi);
+ gimple_set_uid (stmt, inc_gimple_stmt_max_uid (fn));
+ }
+}
+ /* To avoid keeping duplicate gimple IDs in the statements, renumber
+ virtual phis now. */
+ FOR_ALL_BB_FN (bb, fn)
+{
+ for (gphi_iterator gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
+ gsi_next ())
+ {
+ gphi *stmt = gsi.phi ();
+ if (virtual_operand_p (gimple_phi_result (stmt)))
+ gimple_set_uid (stmt, inc_gimple_stmt_max_uid (fn));
+ }
+}
+
}
/* Output the body of function NODE->DECL. */
@@ -2144,45 +2184,6 @@ output_function (struct cgraph_node *nod
/* Output any exception handling regions. */
output_eh_regions (ob, fn);
-
- /* We will renumber the statements. The code that does this uses
-the same ordering that we use for serializing them so we can use
-the same code on the other end and not have to write out the
-statement numbers. We do not assign UIDs to PHIs here because
-virtual PHIs get re-computed on-the-fly which would make numbers
-inconsistent. */
- set_gimple_stmt_max_uid (fn, 0);
- FOR_ALL_BB_FN (bb, fn)
- {
- for (gphi_iterator gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
- gsi_next ())
- {
- gphi *stmt = gsi.phi ();
-
- /* Virtual PHIs are not going to be streamed. */
- if (!virtual_operand_p (gimple_phi_result (stmt)))
- gimple_set_uid (stmt, inc_gimple_stmt_max_uid (fn));
- }
- for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
- gsi_next ())
- {
- gimple *stmt = gsi_stmt (gsi);
- gimple_set_uid (stmt, inc_gimple_stmt_max_uid (fn));
- }
- }
- /* To avoid keeping duplicate gimple IDs in the statements, renumber
-virtual phis now. */
- FOR_ALL_BB_FN (bb, fn)
- {
- for (gphi_iterator gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
- gsi_next ())
- {
- gphi *stmt = gsi.phi ();
- if (virtual_operand_p (gimple_phi_result (stmt)))
- gimple_set_uid (stmt,
[PATCH 4/5] PRU: Remove TARGET_HARD_REGNO_CALL_PART_CLOBBERED
Per clarification in [1], macro is supposed to check for partial
clobbering of single HW registers. Since PRU declares only 8-bit
HW registers, and ABI does not define individual bit clobbering,
it is safe to remove the implementation.
[1] https://gcc.gnu.org/ml/gcc-patches/2019-09/msg00778.html
gcc/ChangeLog:
2019-10-13 Dimitar Dimitrov
* config/pru/pru.c (pru_hard_regno_call_part_clobbered): Remove.
(TARGET_HARD_REGNO_CALL_PART_CLOBBERED): Remove.
Signed-off-by: Dimitar Dimitrov
---
gcc/config/pru/pru.c | 34 --
1 file changed, 34 deletions(-)
diff --git a/gcc/config/pru/pru.c b/gcc/config/pru/pru.c
index dabea2b59c8..d66b989eb2d 100644
--- a/gcc/config/pru/pru.c
+++ b/gcc/config/pru/pru.c
@@ -556,37 +556,6 @@ pru_hard_regno_scratch_ok (unsigned int regno)
}
-/* Implement TARGET_HARD_REGNO_CALL_PART_CLOBBERED. */
-
-static bool
-pru_hard_regno_call_part_clobbered (unsigned, unsigned regno,
- machine_mode mode)
-{
- HARD_REG_SET caller_saved_set;
- HARD_REG_SET callee_saved_set;
-
- CLEAR_HARD_REG_SET (caller_saved_set);
- CLEAR_HARD_REG_SET (callee_saved_set);
-
- /* r0 and r1 are caller saved. */
- add_range_to_hard_reg_set (_saved_set, 0, 2 * 4);
-
- add_range_to_hard_reg_set (_saved_set, FIRST_ARG_REGNUM,
-LAST_ARG_REGNUM + 1 - FIRST_ARG_REGNUM);
-
- /* Treat SP as callee saved. */
- add_range_to_hard_reg_set (_saved_set, STACK_POINTER_REGNUM, 4);
-
- /* r3 to r13 are callee saved. */
- add_range_to_hard_reg_set (_saved_set, FIRST_CALLEE_SAVED_REGNUM,
-LAST_CALEE_SAVED_REGNUM + 1
-- FIRST_CALLEE_SAVED_REGNUM);
-
- return overlaps_hard_reg_set_p (caller_saved_set, mode, regno)
-&& overlaps_hard_reg_set_p (callee_saved_set, mode, regno);
-}
-
-
/* Worker function for `HARD_REGNO_RENAME_OK'.
Return nonzero if register OLD_REG can be renamed to register NEW_REG. */
@@ -2935,9 +2904,6 @@ pru_unwind_word_mode (void)
#undef TARGET_HARD_REGNO_SCRATCH_OK
#define TARGET_HARD_REGNO_SCRATCH_OK pru_hard_regno_scratch_ok
-#undef TARGET_HARD_REGNO_CALL_PART_CLOBBERED
-#define TARGET_HARD_REGNO_CALL_PART_CLOBBERED \
- pru_hard_regno_call_part_clobbered
#undef TARGET_FUNCTION_ARG
#define TARGET_FUNCTION_ARG pru_function_arg
--
2.20.1
[PATCH 3/5] PRU: Fix comment about R3/RA
Comment had a typo. Fix it and clarify. gcc/ChangeLog: 2019-10-13 Dimitar Dimitrov * config/pru/pru.h: Clarify R3/RA ABI. Signed-off-by: Dimitar Dimitrov --- gcc/config/pru/pru.h | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/gcc/config/pru/pru.h b/gcc/config/pru/pru.h index 15fb637dec6..f2bdd1ef02b 100644 --- a/gcc/config/pru/pru.h +++ b/gcc/config/pru/pru.h @@ -125,7 +125,8 @@ 1 r1 Caller Saved. Also used as a temporary by function. profiler and function prologue/epilogue. 2 r2 spStack Pointer. - 3* r3.w0raReturn Address (16-bit). + 3* r3.w0 ABI does not specify if it is caller or callee saved. + 3* r3.w2raReturn Address (16-bit). 4 r4 fpFrame Pointer, also called Argument Pointer in ABI. 5-13 r5-r13 Callee Saved Registers. 14-29 r14-r29Register Arguments. Caller Saved Registers. @@ -148,6 +149,11 @@ of 8 bit sub-registers (e.g. RA starts at r12). When outputting assembly, GCC will take into account the RTL operand size (e.g. r12:HI) in order to translate to the conventional PRU ISA format expected by GAS (r3.w0). + + TI ISA documentation (SPRUHV7C) does not mark r3.w0 as neither + caller-saved nor callee-saved. So until TI clarifies, let's mark + it as fixed. + */ #define FIXED_REGISTERS\ -- 2.20.1
[PATCH 5/5] Add pru to compare-all-tests
contrib/ChangeLog:
2019-10-13 Dimitar Dimitrov
* compare-all-tests (all_targets): Add pru target.
Signed-off-by: Dimitar Dimitrov
---
contrib/compare-all-tests | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/compare-all-tests b/contrib/compare-all-tests
index 502cc64f522..6d0f29e052f 100644
--- a/contrib/compare-all-tests
+++ b/contrib/compare-all-tests
@@ -34,7 +34,7 @@ s390_opts='-m31 -m31/-mzarch -m64'
sh_opts='-m3 -m3e -m4 -m4a -m4al -m4/-mieee -m1 -m1/-mno-cbranchdi -m2a
-m2a/-mieee -m2e -m2e/-mieee'
sparc_opts='-mcpu=v8/-m32 -mcpu=v9/-m32 -m64'
-all_targets='alpha arm avr bfin cris fr30 frv h8300 ia64 iq2000 m32c m32r m68k
mcore mips mmix mn10300 pa pdp11 ppc sh sparc v850 vax xstormy16 xtensa' # e500
+all_targets='alpha arm avr bfin cris fr30 frv h8300 ia64 iq2000 m32c m32r m68k
mcore mips mmix mn10300 pa pdp11 ppc pru sh sparc v850 vax xstormy16 xtensa' #
e500
test_one_file ()
{
--
2.20.1
[PATCH 1/5] PRU: Fix comment to avoid fall through warning
gcc/ChangeLog: 2019-10-13 Dimitar Dimitrov * config/pru/pru.c (pru_print_operand): Fix comment. Signed-off-by: Dimitar Dimitrov --- gcc/config/pru/pru.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/config/pru/pru.c b/gcc/config/pru/pru.c index 16d1451262e..59b004b774f 100644 --- a/gcc/config/pru/pru.c +++ b/gcc/config/pru/pru.c @@ -1650,7 +1650,7 @@ pru_print_operand (FILE *file, rtx op, int letter) return; case 'Q': cond = swap_condition (cond); - /* Fall through to reverse. */ + /* Fall through. */ case 'R': fprintf (file, "%s", pru_comparison_str (reverse_condition (cond))); return; -- 2.20.1
[PATCH 2/5] PRU: Simplify machine description
Use the new @insn syntax for simpler gen_* invocation.
gcc/ChangeLog:
2019-10-13 Dimitar Dimitrov
* config/pru/pru.c (pru_emit_doloop): Use new gen_doloop_end_internal
and gen_doloop_begin_internal.
(pru_reorg_loop): Use gen_pruloop with mode.
* config/pru/pru.md: Use new @insn syntax.
Signed-off-by: Dimitar Dimitrov
---
gcc/config/pru/pru.c | 44 +--
gcc/config/pru/pru.md | 6 +++---
2 files changed, 16 insertions(+), 34 deletions(-)
diff --git a/gcc/config/pru/pru.c b/gcc/config/pru/pru.c
index 59b004b774f..dabea2b59c8 100644
--- a/gcc/config/pru/pru.c
+++ b/gcc/config/pru/pru.c
@@ -2345,26 +2345,14 @@ pru_emit_doloop (rtx *operands, int is_end)
tag = GEN_INT (cfun->machine->doloop_tags - 1);
machine_mode opmode = GET_MODE (operands[0]);
+ gcc_assert (opmode == HImode || opmode == SImode);
+
if (is_end)
-{
- if (opmode == HImode)
- emit_jump_insn (gen_doloop_end_internalhi (operands[0],
- operands[1], tag));
- else if (opmode == SImode)
- emit_jump_insn (gen_doloop_end_internalsi (operands[0],
- operands[1], tag));
- else
- gcc_unreachable ();
-}
+emit_jump_insn (gen_doloop_end_internal (opmode, operands[0],
+operands[1], tag));
else
-{
- if (opmode == HImode)
- emit_insn (gen_doloop_begin_internalhi (operands[0], operands[0], tag));
- else if (opmode == SImode)
- emit_insn (gen_doloop_begin_internalsi (operands[0], operands[0], tag));
- else
- gcc_unreachable ();
-}
+emit_insn (gen_doloop_begin_internal (opmode, operands[0],
+ operands[0], tag));
}
@@ -2607,6 +2595,7 @@ pru_reorg_loop (rtx_insn *insns)
/* Case (1) or (2). */
rtx_code_label *repeat_label;
rtx label_ref;
+ rtx loop_rtx;
/* Create a new label for the repeat insn. */
repeat_label = gen_label_rtx ();
@@ -2616,23 +2605,16 @@ pru_reorg_loop (rtx_insn *insns)
will utilize an internal for the PRU core LOOP register. */
label_ref = gen_rtx_LABEL_REF (VOIDmode, repeat_label);
machine_mode loop_mode = GET_MODE (loop->begin->loop_count);
- if (loop_mode == HImode)
- emit_insn_before (gen_pruloophi (loop->begin->loop_count, label_ref),
- loop->begin->insn);
- else if (loop_mode == SImode)
- {
- rtx loop_rtx = gen_pruloopsi (loop->begin->loop_count, label_ref);
- emit_insn_before (loop_rtx, loop->begin->insn);
- }
- else if (loop_mode == VOIDmode)
+ if (loop_mode == VOIDmode)
{
gcc_assert (CONST_INT_P (loop->begin->loop_count));
gcc_assert (UBYTE_INT ( INTVAL (loop->begin->loop_count)));
- rtx loop_rtx = gen_pruloopsi (loop->begin->loop_count, label_ref);
- emit_insn_before (loop_rtx, loop->begin->insn);
+ loop_mode = SImode;
}
- else
- gcc_unreachable ();
+ gcc_assert (loop_mode == HImode || loop_mode == SImode);
+ loop_rtx = gen_pruloop (loop_mode, loop->begin->loop_count, label_ref);
+ emit_insn_before (loop_rtx, loop->begin->insn);
+
delete_insn (loop->begin->insn);
/* Insert the repeat label before the first doloop_end.
diff --git a/gcc/config/pru/pru.md b/gcc/config/pru/pru.md
index 53fa73dec03..567f41960b4 100644
--- a/gcc/config/pru/pru.md
+++ b/gcc/config/pru/pru.md
@@ -887,7 +887,7 @@
;; This insn is volatile because we'd like it to stay in its original
;; position, just before the loop header. If it stays there, we might
;; be able to convert it into a "loop" insn.
-(define_insn "doloop_begin_internal"
+(define_insn "@doloop_begin_internal"
[(set (match_operand:HISI 0 "register_operand" "=r")
(unspec_volatile:HISI
[(match_operand:HISI 1 "reg_or_ubyte_operand" "rI")
@@ -909,7 +909,7 @@
; Note: "JUMP_INSNs and CALL_INSNs are not allowed to have any output
; reloads;". Hence this insn must be prepared for a counter that is
; not a register.
-(define_insn "doloop_end_internal"
+(define_insn "@doloop_end_internal"
[(set (pc)
(if_then_else (ne (match_operand:HISI 0 "nonimmediate_operand" "+r,*m")
(const_int 1))
@@ -951,7 +951,7 @@
DONE;
})
-(define_insn "pruloop"
+(define_insn "@pruloop"
[(set (reg:HISI LOOPCNTR_REGNUM)
(unspec:HISI [(match_operand:HISI 0 "reg_or_ubyte_operand" "rI")
(label_ref (match_operand 1))]
--
2.20.1
[PATCH 0/5] Assorted minor cleanups for PRU backend
Apart from the last change, these are all minor cleanups to the PRU backend. Dimitar Dimitrov (5): PRU: Fix comment to avoid fall through warning PRU: Simplify machine description PRU: Fix comment about R3/RA PRU: Remove TARGET_HARD_REGNO_CALL_PART_CLOBBERED Add pru to compare-all-tests contrib/compare-all-tests | 2 +- gcc/config/pru/pru.c | 80 +++ gcc/config/pru/pru.h | 8 +++- gcc/config/pru/pru.md | 6 +-- 4 files changed, 25 insertions(+), 71 deletions(-) -- 2.20.1
