Re: [Driver] Add support for -fuse-ld=lld
Ping
> Le 20 oct. 2018 à 12:18, Romain Geissler a
> écrit :
>
> Hi,
>
> I would like to raise again the question of supporting -fuse-ld=ldd. A
> patch implementing it was already submitted in
> https://gcc.gnu.org/ml/gcc-patches/2016-06/msg01722.html by Davide
> Italiano. This patch still applies correctly to current trunk. I am CC-ing
> the original author and re-posting it here unchanged for reference.
>
> I think we can consider this patch as relevant despite the goals and
> licence difference of LLVM vs GNU, based on what was written by Mike Stump
> in https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00157.html
>
> Back then, the technical problem raised by lld was reported as
> https://bugs.llvm.org/show_bug.cgi?id=28414 now closed. In this bug, every
> reported problems have been fixed except the last one. H.J. Lu mentions
> this last problem (lld does not produces symbol versions predecessor
> relationship while ld.bfd and ld.gold do, which seems to be a decision
> taken on purpose and advertised as a harmless change) as being one reason
> against supporting in -fuse-ld=ldd in gcc. Is it still the case today, and
> if yes, why ?
>
> Is there any other reason why -fuse-ld=ldd shall not be supported by gcc ?
>
> Cheers,
> Romain
>
> From 323c23d79c91d7dcee2f29b9ced8c1c00703d346 Mon Sep 17 00:00:00 2001
> From: Davide Italiano
> Date: Thu, 23 Jun 2016 20:51:53 -0700
> Subject: [PATCH] Driver: Add support for -fuse-ld=lld.
>
> * collect2.c (main): Support -fuse-ld=lld.
>
> * common.opt: Add fuse-ld=lld
>
> * doc/invoke.texi: Document -fuse-ld=lld
>
> * opts.c: Ignore -fuse-ld=lld
> ---
> gcc/collect2.c | 11 ---
> gcc/common.opt | 4
> gcc/doc/invoke.texi | 4
> gcc/opts.c | 1 +
> 4 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/collect2.c b/gcc/collect2.c
> index bffac80..6a8387c 100644
> --- a/gcc/collect2.c
> +++ b/gcc/collect2.c
> @@ -831,6 +831,7 @@ main (int argc, char **argv)
> USE_PLUGIN_LD,
> USE_GOLD_LD,
> USE_BFD_LD,
> + USE_LLD_LD,
> USE_LD_MAX
> } selected_linker = USE_DEFAULT_LD;
> static const char *const ld_suffixes[USE_LD_MAX] =
> @@ -838,7 +839,8 @@ main (int argc, char **argv)
> "ld",
> PLUGIN_LD_SUFFIX,
> "ld.gold",
> - "ld.bfd"
> + "ld.bfd",
> + "ld.lld"
> };
> static const char *const real_ld_suffix = "real-ld";
> static const char *const collect_ld_suffix = "collect-ld";
> @@ -1004,6 +1006,8 @@ main (int argc, char **argv)
> selected_linker = USE_BFD_LD;
> else if (strcmp (argv[i], "-fuse-ld=gold") == 0)
> selected_linker = USE_GOLD_LD;
> + else if (strcmp (argv[i], "-fuse-ld=lld") == 0)
> +selected_linker = USE_LLD_LD;
>
> #ifdef COLLECT_EXPORT_LIST
> /* These flags are position independent, although their order
> @@ -1093,7 +1097,8 @@ main (int argc, char **argv)
> /* Maybe we know the right file to use (if not cross). */
> ld_file_name = 0;
> #ifdef DEFAULT_LINKER
> - if (selected_linker == USE_BFD_LD || selected_linker == USE_GOLD_LD)
> + if (selected_linker == USE_BFD_LD || selected_linker == USE_GOLD_LD ||
> + selected_linker == USE_LLD_LD)
> {
> char *linker_name;
> # ifdef HOST_EXECUTABLE_SUFFIX
> @@ -1307,7 +1312,7 @@ main (int argc, char **argv)
> else if (!use_collect_ld
> && strncmp (arg, "-fuse-ld=", 9) == 0)
> {
> - /* Do not pass -fuse-ld={bfd|gold} to the linker. */
> + /* Do not pass -fuse-ld={bfd|gold|lld} to the linker. */
> ld1--;
> ld2--;
> }
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 5d90385..2a95a1f 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2536,6 +2536,10 @@ fuse-ld=gold
> Common Driver Negative(fuse-ld=bfd)
> Use the gold linker instead of the default linker.
>
> +fuse-ld=lld
> +Common Driver Negative(fuse-ld=lld)
> +Use the lld LLVM linker instead of the default linker.
> +
> fuse-linker-plugin
> Common Undocumented Var(flag_use_linker_plugin)
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 2c87c53..4b8acff 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -10651,6 +10651,10 @@ Use the @command{bfd} linker instead of the default
> linker.
> @opindex fuse-ld=gold
> Use the @command{gold} linker instead of the default linker.
>
> +@item -fuse-ld=lld
> +@opindex fuse-ld=lld
> +Use the LLVM @command{lld} linker instead of the default linker.
> +
> @cindex Libraries
> @item -l@var{library}
> @itemx -l @var{library}
> diff --git a/gcc/opts.c b/gcc/opts.c
> index 7406210..f2c86f7 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -2178,6 +2178,7 @@ common_handle_option (struct gcc_options *opts,
>
> case OPT_fuse_ld_bfd:
> case OPT_fuse_ld_gold:
> +case OPT_fuse_ld_lld:
> case OPT_fuse_linker_plugin:
> /* No-op. Used by the driver and passed to us bec
[patch, fortran, committed] Clarify warning on missing location information
Originally, the idea about checking for missing location information
when not configured for release is to catch errors when adding new
code, so forgetting to add the info will show up right away on
regression testing.
From time to time, a user can stumble across something like that
and get a confusing error message. This tries to make sure
people don't mistake this for their own error.
Committed as obvious, r265559. No test case because... well, this
is not supposed to happen in the first place.
Regards
Thomas
2018-10-27 Thomas Koenig
PR fortran/86907
* frontend-passes.c (check_locus_code): Add information that
warning about missing location information points to an
inconsisten internal state.
(check_locus_expr): Likewise.
Index: frontend-passes.c
===
--- frontend-passes.c (Revision 265502)
+++ frontend-passes.c (Arbeitskopie)
@@ -190,7 +190,8 @@ check_locus_code (gfc_code **c, int *walk_subtrees
{
current_code = c;
if (c && *c && (((*c)->loc.nextc == NULL) || ((*c)->loc.lb == NULL)))
-gfc_warning_internal (0, "No location in statement");
+gfc_warning_internal (0, "Inconsistent internal state: "
+ "No location in statement");
return 0;
}
@@ -205,7 +206,8 @@ check_locus_expr (gfc_expr **e, int *walk_subtrees
{
if (e && *e && (((*e)->where.nextc == NULL || (*e)->where.lb == NULL)))
-gfc_warning_internal (0, "No location in expression near %L",
+gfc_warning_internal (0, "Inconsistent internal state: "
+ "No location in expression near %L",
&((*current_code)->loc));
return 0;
}
Re: [patch, fortran] Implement FINDLOC
Am 23.10.18 um 23:02 schrieb Thomas Koenig: So, I think this should be clear for trunk now. I will supply the documentation later. Ping ** 0.571428 ?
Re: [PATCH] Fix PR 86572
On 10/22/18 9:03 PM, Martin Sebor wrote: > On 10/22/2018 09:08 AM, Bernd Edlinger wrote: >> Hi! >> >> This makes c_strlen avoid an unsafe strlen folding of const arguments >> with non-const offset. Currently a negative out of bounds offset >> makes the strlen function return an extremely large number, and >> at the same time, prevents the VRP machinery, to determine the correct >> range if the strlen function in this case. >> >> Fixed by doing the whole computation in size_t and casting the >> result back to ssize_t. >> >> >> Bootstrapped and reg-tested on x86_64-pc-linux-gnu. >> Is it OK for trunk? > > In general, I view folding to a "safe" value as done in this patch > (and as Richard suggests in his comment on the bug) far preferable > -- as in far safer/more secure -- to avoiding folding in these > cases (currently the prevailing strategy). > Well, in this case we have the slightly different situation, where 'strlen(string_cst + x)' is folded to 'x <= len ? len - x : 0'. The string_cst does not have embedded nuls, and may be declared like 'const char string_cst[100] = "string"'. So the COND_EXPR is not there to separate valid from invalid inputs, but instead we may assume that x does never exceed the array bounds. However it still results in more accurate range infos, to do all computations in unsigned arithmetics, and a slightly more safe result if the x is negative at the same time, so win-win. But I have no doubt that it would be even more safe to have a trap if x is out of bounds, however if we add another conditional branch, that would be something that needs to be optional, and results in slightly less efficient code. > If there is consensus on adopting this approach for strlen I'd like > to see it applied in other such cases as well as a matter of policy. > > Some other similar examples to consider include: > > * calling other built-ins on such arrays, including strnlen, > memchr/strchr, and strpbrk > * indexing into such an array (note that accesses at constant > out-of-bounds indices into constant arrays are already folded > to zero, although that may be an accidental rather than > a conscious decision made to avoid the worst kind of fallout). > I would argue that if the undefinedness of any such construct can be determined statically, there should be a warning, and the folding should not be done, the out-of-bounds index handling is also something that I would remove eventually. The folding should not be done when it is evident that the index exceeds the memory size, as returned by string_constant, while it is still okay to fold to 0, if the index is somewhere in between the string length and the memory size. That can be determined statically, and comes at no extra cost. Bernd.
Re: [PATCH v4] Avoid unnecessarily numbering cloned symbols.
Hi, On 2018-10-26 10:25 a.m., Jan Hubicka wrote: >> From aea94273e7a477a03d1ee10a5d9043d6d13b8e8d Mon Sep 17 00:00:00 2001 >> From: Michael Ploujnikov >> Date: Thu, 25 Oct 2018 13:16:36 -0400 >> Subject: [PATCH] Avoid unnecessarily numbering cloned symbols. >> >> gcc/ChangeLog: >> >> 2018-10-26 Michael Ploujnikov >> >> * cgraph.h (clone_function_name_1): Replaced by new >>clone_function_name_numbered that takes name as string; for >>privatize_symbol_name_1 use only. >>(clone_function_name): Renamed to >>clone_function_name_numbered to be explicit about numbering. >>(clone_function_name): New two-argument function that does >>not number its output. >>(clone_function_name): New three-argument function that >>takes a number to append to its output. >> * cgraphclones.c (duplicate_thunk_for_node): >>(clone_function_name_1): Renamed. >>(clone_function_name_numbered): Two new functions. >>(clone_function_name): Improved documentation. >>(cgraph_node::create_virtual_clone): Use clone_function_name_numbered. >> * config/rs6000/rs6000.c (make_resolver_func): Ditto. >> * final.c (final_scan_insn_1): Use the new clone_function_name >>without numbering. >> * multiple_target.c (create_dispatcher_calls): Ditto. >>(create_target_clone): Ditto. >> * omp-expand.c (grid_expand_target_grid_body): Ditto. >> * omp-low.c (create_omp_child_function_name): Ditto. >> * omp-simd-clone.c (simd_clone_create): Ditto. >> * symtab.c (simd_symtab_node::noninterposable_alias): Use the >>new clone_function_name without numbering. >> >> gcc/lto/ChangeLog: >> >> 2018-10-26 Michael Ploujnikov >> >> * lto-partition.c (privatize_symbol_name_1): Use >>clone_function_name_numbered. >> >> gcc/testsuite/ChangeLog: >> >> 2018-10-26 Michael Ploujnikov >> >> * gcc.dg/tree-prof/cold_partition_label.c: Update for cold >>section names without numbers. >> * gcc.dg/tree-prof/section-attr-1.c: Ditto. >> * gcc.dg/tree-prof/section-attr-2.c: Ditto. >> * gcc.dg/tree-prof/section-attr-3.c: Ditto. > > OK, > thanks! > Honza > Thanks again for the review. This is my first patch and I don't have commit access. What should I do? - Michael signature.asc Description: OpenPGP digital signature
[PATCH][rs6000] use index form addresses more often for ldbrx/stdbrx
At Segher's suggestion, I looked into changing the predicates on
bswapdi2_{load,store}
from memory_operand to indexed_or_indirect_operand and putting some code into
bswapdi2
to make the address indirect if it wasn't already.
The motivating case for this was the code I was seeing for the gpr expansion of
strncmp.
Before I would typically see something like this:
addi 9,3,8
ldbrx 10,0,9
addi 9,4,8
ldbrx 8,0,9
subf. 9,8,10
bne 0,.L13
cmpb 10,10,9
cmpdi 0,10,0
bne 0,.L9
addi 9,3,16
ldbrx 10,0,9
addi 9,4,16
ldbrx 8,0,9
subf. 9,8,10
bne 0,.L13
cmpb 10,10,9
cmpdi 0,10,0
bne 0,.L9
For each comparison block it is doing the add separately and using 0 for one
input
of the ldbrx.
After this change, it is more like this:
ldbrx 8,3,27
ldbrx 7,4,27
cmpb 9,8,9
cmpb 10,8,7
orc. 9,9,10
bne 0,.L13
ldbrx 8,3,24
ldbrx 7,4,24
cmpb 10,8,9
cmpb 9,8,7
orc. 9,10,9
bne 0,.L13
Here it has created temps with constants and hoisted them out of a loop, but I
have
other cases where it will update them if there is more register pressure. in
either
case the code is more compact and makes full use of the indexed addressing of
ldbrx.
Bootstrap/regtest passed on ppc64le targeting power7/power8/power9, ok for
trunk?
Thanks!
Aaron
2018-10-27 Aaron Sawdey
* config/rs6000/rs6000.md (bswapdi2): Force address into register
if not in one already.
(bswapdi2_load): Change predicate to indexed_or_indirect_operand.
(bswapdi2_store): Ditto.
Index: gcc/config/rs6000/rs6000.md
===
--- gcc/config/rs6000/rs6000.md (revision 265393)
+++ gcc/config/rs6000/rs6000.md (working copy)
@@ -2512,9 +2512,27 @@
if (TARGET_POWERPC64 && TARGET_LDBRX)
{
if (MEM_P (src))
- emit_insn (gen_bswapdi2_load (dest, src));
+{
+ rtx addr = XEXP (src, 0);
+ if (!legitimate_indirect_address_p (addr, reload_completed)
+ && !legitimate_indexed_address_p (addr, reload_completed))
+{
+ addr = force_reg (Pmode, addr);
+ src = replace_equiv_address_nv (src, addr);
+}
+ emit_insn (gen_bswapdi2_load (dest, src));
+}
else if (MEM_P (dest))
- emit_insn (gen_bswapdi2_store (dest, src));
+{
+ rtx addr = XEXP (dest, 0);
+ if (!legitimate_indirect_address_p (addr, reload_completed)
+ && !legitimate_indexed_address_p (addr, reload_completed))
+{
+ addr = force_reg (Pmode, addr);
+ dest = replace_equiv_address_nv (dest, addr);
+}
+ emit_insn (gen_bswapdi2_store (dest, src));
+}
else if (TARGET_P9_VECTOR)
emit_insn (gen_bswapdi2_xxbrd (dest, src));
else
@@ -2535,13 +2553,13 @@
;; Power7/cell has ldbrx/stdbrx, so use it directly
(define_insn "bswapdi2_load"
[(set (match_operand:DI 0 "gpc_reg_operand" "=r")
- (bswap:DI (match_operand:DI 1 "memory_operand" "Z")))]
+ (bswap:DI (match_operand:DI 1 "indexed_or_indirect_operand" "Z")))]
"TARGET_POWERPC64 && TARGET_LDBRX"
"ldbrx %0,%y1"
[(set_attr "type" "load")])
(define_insn "bswapdi2_store"
- [(set (match_operand:DI 0 "memory_operand" "=Z")
+ [(set (match_operand:DI 0 "indexed_or_indirect_operand" "=Z")
(bswap:DI (match_operand:DI 1 "gpc_reg_operand" "r")))]
"TARGET_POWERPC64 && TARGET_LDBRX"
"stdbrx %1,%y0"
--
Aaron Sawdey, Ph.D. acsaw...@linux.vnet.ibm.com
050-2/C113 (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain
Re: [PATCH][rs6000] use index form addresses more often for ldbrx/stdbrx
Hi Aaron,
On Sat, Oct 27, 2018 at 11:20:01AM -0500, Aaron Sawdey wrote:
> --- gcc/config/rs6000/rs6000.md (revision 265393)
> +++ gcc/config/rs6000/rs6000.md (working copy)
> @@ -2512,9 +2512,27 @@
>if (TARGET_POWERPC64 && TARGET_LDBRX)
> {
>if (MEM_P (src))
> - emit_insn (gen_bswapdi2_load (dest, src));
> +{
> + rtx addr = XEXP (src, 0);
> + if (!legitimate_indirect_address_p (addr, reload_completed)
> + && !legitimate_indexed_address_p (addr, reload_completed))
Should you use indexed_or_indirect operand instead here?
> +{
> + addr = force_reg (Pmode, addr);
> + src = replace_equiv_address_nv (src, addr);
> +}
> + emit_insn (gen_bswapdi2_load (dest, src));
> +}
You could maybe make this a utility routine as well (in rs6000.c)...
Something like force_indexed_or_indirect_mem. So this code will be just
if (MEM_P (src))
force_indexed_or_indirect_mem (src);
then.
Could you try those things please?
Segher
Re: [RFC][PATCH LRA] WIP patch to fix one part of PR87507
On 10/22/18 6:45 PM, Peter Bergner wrote:
> Bah, my bootstrap failed and I need to make a small change. Let me do that
> and verify my bootstraps get all the way thru before I give you the updated
> patch. Sorry.
Ok, the following updated patch survives bootstrap and regtesting on
powerpc64le-linux, x86_64-linux and s390x-linux with no regressions.
Changes from the previous patch is that checking for illegal hard register
usage in inline asm statements has been moved to expand time. Secondly, the
lra constraints code now checks for both HARD_REGISTER_P and REG_USERVAR_P.
This was because I was seeing combine forcing hard regs into a pattern
(not from inline asm) which lra needed to spill to match constraints,
which it should be able to do.
Jeff, can you give this a try on your testers to see how it behaves on
the other arches that were having problems?
Peter
PR rtl-optimization/87600
* cfgexpand.c (expand_asm_stmt): Catch illegal asm constraint usage.
* lra-constraints.c (process_alt_operands): Skip illegal hard
register usage. Prefer reloading non hard register operands.
Index: gcc/cfgexpand.c
===
--- gcc/cfgexpand.c (revision 265402)
+++ gcc/cfgexpand.c (working copy)
@@ -3010,6 +3010,55 @@ expand_asm_stmt (gasm *stmt)
&allows_mem, &allows_reg, &is_inout))
return;
+ /* If the output is a hard register, verify it doesn't conflict with
+any other operand's possible hard register use. */
+ if (DECL_P (val)
+ && REG_P (DECL_RTL (val))
+ && HARD_REGISTER_P (DECL_RTL (val)))
+ {
+ unsigned j, output_hregno = REGNO (DECL_RTL (val));
+ bool early_clobber_p = strchr (constraints[i], '&') != NULL;
+ unsigned long match;
+
+ /* Verify the other outputs do not use the same hard register. */
+ for (j = i + 1; j < noutputs; ++j)
+ if (DECL_P (output_tvec[j])
+ && REG_P (DECL_RTL (output_tvec[j]))
+ && HARD_REGISTER_P (DECL_RTL (output_tvec[j]))
+ && output_hregno == REGNO (DECL_RTL (output_tvec[j])))
+ error ("invalid hard register usage between output operands");
+
+ /* Verify matching constraint operands use the same hard register
+and that the non-matching constraint operands do not use the same
+hard register if the output is an early clobber operand. */
+ for (j = 0; j < ninputs; ++j)
+ if (DECL_P (input_tvec[j])
+ && REG_P (DECL_RTL (input_tvec[j]))
+ && HARD_REGISTER_P (DECL_RTL (input_tvec[j])))
+ {
+ unsigned input_hregno = REGNO (DECL_RTL (input_tvec[j]));
+ switch (*constraints[j + noutputs])
+ {
+ case '0': case '1': case '2': case '3': case '4':
+ case '5': case '6': case '7': case '8': case '9':
+ match = strtoul (constraints[j + noutputs], NULL, 10);
+ break;
+ default:
+ match = ULONG_MAX;
+ break;
+ }
+ if (i == match
+ && output_hregno != input_hregno)
+ error ("invalid hard register usage between output operand "
+"and matching constraint operand");
+ else if (early_clobber_p
+&& i != match
+&& output_hregno == input_hregno)
+ error ("invalid hard register usage between earlyclobber "
+"operand and input operand");
+ }
+ }
+
if (! allows_reg
&& (allows_mem
|| is_inout
Index: gcc/lra-constraints.c
===
--- gcc/lra-constraints.c (revision 265402)
+++ gcc/lra-constraints.c (working copy)
@@ -2146,9 +2146,30 @@ process_alt_operands (int only_alternati
}
else
{
- /* Operands don't match. Both operands must
- allow a reload register, otherwise we
- cannot make them match. */
+ /* Operands don't match. If the operands are
+ different user defined explicit hard registers,
+ then we cannot make them match. */
+ if ((REG_P (*curr_id->operand_loc[nop])
+|| SUBREG_P (*curr_id->operand_loc[nop]))
+ && (REG_P (*curr_id->operand_loc[m])
+ || SUBREG_P (*curr_id->operand_loc[m])))
+ {
+ rtx nop_reg = *curr_id->operand_loc[nop];
+ if (SUBREG_P (nop_reg))
+
[Patch, fortran] PR40196 - [F03] [F08] Type parameter inquiry (str%len, a%kind) and Complex parts (z%re, z%im)
I was triggered to do this by one of the comments in response to Anton
Shterenlikht's standards survey. The comment was sufficiently
inconsiderate that my first thought was not to respond. However,
curiosity got the better of me... so said the dead cat!
There is a lot of this patch but it is (more or less) straight
forward. The tricky parts were to get the logic right in
gfc_match_varspec and in expr.c. One more step on the way to real
F2002 and F2008 compliance!
Bootstraps and regtests on FC28/x86_64 - OK for trunk?
Paul
2018-10-27 Paul Thomas
PR fortran/40196
* dependency.c (are_identical_variables): Return false if the
inquiry refs are not the same.
(gfc_ref_needs_temporary_p): Break on an inquiry ref.
* dump_parse_tree.c (show_ref): Show the inquiry ref type.
* expr.c (gfc_free_ref_list): Break on an inquiry ref.
(gfc_copy_ref): Copy the inquiry ref types.
(find_inquiry_ref): New function.
(simplify_const_ref, simplify_ref_chain): Call it. Add new arg
to simplify_ref_chain.
(gfc_simplify_expr): Use the new arg in call to
simplify_ref_chain.
(gfc_get_full_arrayspec_from_expr, gfc_is_coarray): Break on
inquiry ref.
(gfc_traverse_expr): Return true for inquiry ref.
* frontend-passes.c (gfc_expr_walker): Break on inquiry ref.
* gfortran.h : Add enums and union member in gfc_ref to
implement inquiry refs.
* intrinsic.c : Fix white nois.
* match.c (gfc_match_assignment): A constant lavlue is an
error.
* module.c : Add DECL_MIO_NAME for inquiry_type and the mstring
for inquiry_types.
(mio_ref): Handle inquiry refs.
* primary.c (is_inquiry_ref): New function.
(gfc_match_varspec): Handle inquiry refs calling new function.
(gfc_variable_attr): Detect inquiry ref for disambiguation
with components.
(caf_variable_attr): Treat inquiry and substring refs in the
same way.
* resolve.c (find_array_spec): ditto.
(gfc_resolve_substring_charlen): If there is neither a charlen
ref not an inquiry ref, return.
(resolve_ref): Handle inqiry refs as appropriate.
(resolve_allocate_expr): ENtities with an inquiry ref cannot be
allocated.
* simplify.c (simplify_bound, simplify_cobound): Punt on
inquiry refs.
* trans-array.c (get_array_ctor_var_strlen): Break on inquiry
ref.
*trans-expr.c (conv_inquiry): New function.
(gfc_conv_variable): Retain the last typespec to pass to
conv_inquiry on detecting an inquiry ref.
2018-10-27 Paul Thomas
PR fortran/40196
* gfortran.dg/inquiry_part_ref_1.f08: New test.
Index: gcc/fortran/dependency.c
===
*** gcc/fortran/dependency.c (revision 265411)
--- gcc/fortran/dependency.c (working copy)
*** are_identical_variables (gfc_expr *e1, g
*** 189,194
--- 189,199
break;
+ case REF_INQUIRY:
+ if (r1->u.i != r2->u.i)
+ return false;
+ break;
+
default:
gfc_internal_error ("are_identical_variables: Bad type");
}
*** gfc_ref_needs_temporary_p (gfc_ref *ref)
*** 905,910
--- 910,916
return subarray_p;
case REF_COMPONENT:
+ case REF_INQUIRY:
break;
}
Index: gcc/fortran/dump-parse-tree.c
===
*** gcc/fortran/dump-parse-tree.c (revision 265411)
--- gcc/fortran/dump-parse-tree.c (working copy)
*** show_ref (gfc_ref *p)
*** 308,313
--- 308,330
fputc (')', dumpfile);
break;
+ case REF_INQUIRY:
+ switch (p->u.i)
+ {
+ case INQUIRY_KIND:
+ fprintf (dumpfile, " INQUIRY_KIND ");
+ break;
+ case INQUIRY_LEN:
+ fprintf (dumpfile, " INQUIRY_LEN ");
+ break;
+ case INQUIRY_RE:
+ fprintf (dumpfile, " INQUIRY_RE ");
+ break;
+ case INQUIRY_IM:
+ fprintf (dumpfile, " INQUIRY_IM ");
+ }
+ break;
+
default:
gfc_internal_error ("show_ref(): Bad component code");
}
*** write_decl (gfc_typespec *ts, gfc_array_
*** 3167,3173
fputs (sym_name, dumpfile);
fputs (post, dumpfile);
!
if (rok == T_WARN)
fprintf (dumpfile," /* WARNING: Converting '%s' to interoperable type */",
gfc_typename (ts));
--- 3184,3190
fputs (sym_name, dumpfile);
fputs (post, dumpfile);
!
if (rok == T_WARN)
fprintf (dumpfile," /* WARNING: Converting '%s' to interoperable type */",
gfc_typename (ts));
Index: gcc/fortran/expr.c
===
*** gcc/fortran/expr.c (revision 265411)
--- gcc/fortran/expr.c (working copy)
*** gfc_free_ref_list (gfc_ref *p)
*** 599,604
--- 599,605
break;
case REF_COMPONENT:
+ case REF_INQUIRY:
break;
}
*** gfc_copy_ref (gfc_ref *src)
*** 756,761
--- 757,766
dest->u.c = src->u.c;
break
Don’t build gdb/readline/libreadline.a, when --with-system-readline is supplied
Building GDB always builds the bundles libreadline.a, even if use of the libreadline installed on the system was requested with --with- system-readline. The change below is for binutils-gdb’s/configure.ac, which is maintained by gcc. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87741 [GCC] and https://sourceware.org/bugzilla/show_bug.cgi?id=18632 [GDB] for details. diff --git a/configure.ac b/configure.ac --- a/configure.ac +++ b/configure.ac @@ -253,6 +253,12 @@ if test x$with_system_zlib = xyes ; then noconfigdirs="$noconfigdirs zlib" fi +# Don't compile the bundled readline/libreadline.a in gdb-binutils if +# --with-system-readline is provided. +if test x$with_system_readline = xyes ; then + noconfigdirs="$noconfigdirs readline" +fi + # some tools are so dependent upon X11 that if we're not building with X, # it's not even worth trying to configure, much less build, that tool.
[nios2, committed] Fix PR80024
I've checked in this patch to fix a diagnostic message in the nios2
backend. This is in the target attribute parsing code; it now uses the
same wording as the similar diagnostic for an incorrect value given to
the corresponding command-line argument.
-Sandra
2018-10-27 Sandra Loosemore
PR target/80024
gcc/
* config/nios2/nios2.c (nios2_valid_target_attribute_rec): Fix
error message.
Index: gcc/config/nios2/nios2.c
===
--- gcc/config/nios2/nios2.c (revision 265560)
+++ gcc/config/nios2/nios2.c (working copy)
@@ -4264,8 +4264,8 @@ nios2_valid_target_attribute_rec (tree a
continue;
if (!ISDIGIT (*t))
{
- error ("`custom-%s=' argument requires "
- "numeric digits", N2FPU_NAME (code));
+ error ("% argument should be "
+ "a non-negative integer", N2FPU_NAME (code));
return false;
}
}
[PR87469] ICE in record_estimate, at tree-ssa-loop-niter.c
Hi,
In the testcase provided in the bug report, max value for niter
estimation is off by one when it is INTEGER_CST. As a results it
asserts at the place where it is checked for equality.
Attached patch fixes this. Bootstrapped and regression tested on
x86_64-linux-gnu with no new regression. Is this OK?
Thanks,
Kugan
gcc/testsuite/ChangeLog:
2018-10-26 Kugan Vivekanandarajah
PR middle-end/87469
* g++.dg/pr87469.C: New test.
gcc/ChangeLog:
2018-10-26 Kugan Vivekanandarajah
PR middle-end/87469
* tree-ssa-loop-niter.c (number_of_iterations_popcount): Fix niter
max value.
From 359f6aa2d603784b900feedb7ad450523695e191 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah
Date: Fri, 26 Oct 2018 09:04:47 +1100
Subject: [PATCH] pr87469 V2
Change-Id: If1f7da7450ae27e24baf638861c97ff416f8484a
---
gcc/testsuite/g++.dg/pr87469.C | 15 +++
gcc/tree-ssa-loop-niter.c | 8 +++-
2 files changed, 18 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/pr87469.C
diff --git a/gcc/testsuite/g++.dg/pr87469.C b/gcc/testsuite/g++.dg/pr87469.C
new file mode 100644
index 000..2f6de97
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr87469.C
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-c -w -O2" } */
+long a;
+struct c {
+void d(unsigned f) {
+ long e = f;
+ while (e & (e - 1))
+ e &= e - 1;
+ a = e;
+}
+};
+void g() {
+c b;
+b.d(4 + 2);
+}
diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
index e2bc936..e763b35 100644
--- a/gcc/tree-ssa-loop-niter.c
+++ b/gcc/tree-ssa-loop-niter.c
@@ -2589,11 +2589,9 @@ number_of_iterations_popcount (loop_p loop, edge exit,
if (TREE_CODE (call) == INTEGER_CST)
max = tree_to_uhwi (call);
else
-{
- max = TYPE_PRECISION (TREE_TYPE (src));
- if (adjust)
- max = max - 1;
-}
+max = TYPE_PRECISION (TREE_TYPE (src));
+ if (adjust)
+max = max - 1;
niter->niter = iter;
niter->assumptions = boolean_true_node;
--
2.7.4
Re: [PATCH v3 1/3] or1k: libgcc: initial support for openrisc
Hi! On Sat, Oct 27, 2018 at 01:37:00PM +0900, Stafford Horne wrote: > + /* Given R = X * Y ... */ > +1: l.sfeq r4, r0 /* while (y != 0) */ > + l.bf2f > + l.andi r5, r4, 1 /* if (y & 1) ... */ Do the extra leading spaces mean something? > +l.sfeqi r4, 0 /* division by zero; return 0. > */ In some places (like here) you ident with 8 spaces instead of a tab. > +/* For signed division we do: > + * > + * -x / y = x / -y = -(x / y) > + * -x % y = -(x % y) > + * x % -y = x % b > + * > + * which has the property that (x/y)*y + (x%y) = x. > + */ You mean "y" instead of "b" I think. Segher
Re: [PATCH v3 1/3] or1k: libgcc: initial support for openrisc
Hi, Thanks for the review. On Sat, Oct 27, 2018 at 06:25:04PM -0500, Segher Boessenkool wrote: > Hi! > > On Sat, Oct 27, 2018 at 01:37:00PM +0900, Stafford Horne wrote: > > + /* Given R = X * Y ... */ > > +1: l.sfeq r4, r0 /* while (y != 0) */ > > + l.bf2f > > +l.andi r5, r4, 1 /* if (y & 1) ... */ > > Do the extra leading spaces mean something? Yes, we put those to indicate a branch delay slot instruction. > > +l.sfeqir4, 0 /* division by zero; return 0. > > */ > > In some places (like here) you ident with 8 spaces instead of a tab. Thanks, I will fix those, it should be tab. > > +/* For signed division we do: > > + * > > + * -x / y = x / -y = -(x / y) > > + * -x % y = -(x % y) > > + * x % -y = x % b > > + * > > + * which has the property that (x/y)*y + (x%y) = x. > > + */ > > You mean "y" instead of "b" I think. I believe so, I will read through it. This part was done by Richard I should have reviewed it better. -Stafford
Re: [PATCH v3 1/3] or1k: libgcc: initial support for openrisc
On 10/27/18 5:37 AM, Stafford Horne wrote: > +/* Here _init and _fini are empty because .init_array/.fini_array are used > + exclusively. However, the functions are still needed as required when > + linking. */ > + .align 4 > + .global _init > + .type _init,@function > +_init: > + .global _fini > + .type _fini,@function > +_fini: > + l.jrr9 > + l.nop Where are they referenced from? Perhaps just a binutils bug, in that the linker script needs adjustment? > + /* Given R = X * Y ... */ > +1: l.sfeq r4, r0 /* while (y != 0) */ > + l.bf2f > + l.andi r5, r4, 1 /* if (y & 1) ... */ > + l.add r12, r11, r3 > + l.sfne r5, r0 > +#if defined(__or1k_cmov__) > + l.cmov r11, r12, r11 /* ... r += x. */ > + l.srli r4, r4, 1 /* y >>= 1 */ > +#else > + l.bnf 3f > + l.srli r4, r4, 1 /* y >>= 1 */ > + l.ori r11, r12, 0 This move could be the add to save 1 cycle in the !cmov case. > + /* Shift Y back to the right again, subtracting from X. */ > +2: l.add r7, r11, r6 /* tmp1 = quot + mask */ > +3: l.srli r6, r6, 1 /* mask >>= 1 */ > + l.sub r8, r12, r4 /* tmp2 = x - y */ > + l.sfleu r4, r12 /* y <= x */ > + l.srli r4, r4, 1 /* y >>= 1 */ > +#if defined(__or1k_cmov__) > + l.cmov r11, r7, r11/* if (y <= x) quot = tmp1 */ > + l.cmov r12, r8, r12/* if (y <= x) x = tmp2 */ > +#else > + l.bnf 4f > + l.nop > + l.ori r11, r7, 0 > + l.ori r12, r8, 0 Simiarly. Although both mul nor div are correct as-is, and need not be fixed immediately. I'm only concerned about _init and _fini. r~
Re: [PATCH v3 2/3] or1k: testsuite: initial support for openrisc
On 10/27/18 5:37 AM, Stafford Horne wrote: > -mm-dd Stafford Horne > Richard Henderson > > gcc/testsuite/ChangeLog: > > * gcc.c-torture/execute/20101011-1.c: Adjust for OpenRISC. > * gcc.dg/20020312-2.c: Likewise. > * gcc.dg/attr-alloc_size-11.c: Likewise. > * gcc.dg/builtin-apply2.c: Likewise. > * gcc.dg/nop.h: Likewise. > * gcc.dg/torture/stackalign/builtin-apply-2.c: Likewise. > * gcc.dg/tree-ssa/20040204-1.c: Likewise. > * gcc.dg/tree-ssa/reassoc-33.c: Likewise. > * gcc.dg/tree-ssa/reassoc-34.c: Likewise. > * gcc.dg/tree-ssa/reassoc-35.c: Likewise. > * gcc.dg/tree-ssa/reassoc-36.c: Likewise. > * lib/target-supports.exp > (check_effective_target_logical_op_short_circuit): Add or1k*-*-*. > * gcc.target/or1k/*: New. > --- Ok. r~
Re: [PATCH v3 0/3] OpenRISC port
On 10/27/18 5:36 AM, Stafford Horne wrote: > Changes Since v2: > - Add RTEMS patches from Joel Sherrill > - Disable t-softfp-excl as suggsted by Joseph Myers > - Add new architecture flags needed to run on real FPGA's found in testing >* -mror - enable l.ror (rotate right) >* -mshftimm - enable shift/rorate by immediate instructions > - Binutils requirements are now in upstream git I'll just note quickly that you missed gcc-patches in the Cc. ;-) r~
Re: [PATCH v3 3/3] or1k: gcc: initial support for openrisc
On 10/27/18 5:37 AM, Stafford Horne wrote: > +(define_insn "zero_extendhisi2" > + [(set (match_operand:SI 0 "register_operand""=r,r") > + (zero_extend:SI (match_operand:HI 1 "nonimmediate_operand" "r,m")))] > + "" > + "@ > + l.exthz\t%0, %1 > + l.lhz\t%0, %1" > + [(set_attr "insn_support" "sext,*")]) > + > +(define_insn "zero_extendqisi2" > + [(set (match_operand:SI 0 "register_operand""=r,r") > + (zero_extend:SI (match_operand:QI 1 "nonimmediate_operand" "r,m")))] > + "" > + "@ > + l.extbz\t%0, %1 > + l.lbz\t%0, %1" > + [(set_attr "insn_support" "sext,*")]) The !sext r/r case is just l.andi. > +;; Sign extension patterns > + > +;; We can do memory extensions with a single load > +(define_insn "extendhisi2" > + [(set (match_operand:SI 0 "register_operand" "=r,r") > + (sign_extend:SI (match_operand:HI 1 "nonimmediate_operand" "r,m")))] > + "" > + "@ > + l.exths\t%0, %1 > + l.lhs\t%0, %1" > + [(set_attr "insn_support" "sext,*")]) > + > +(define_insn "extendqisi2" > + [(set (match_operand:SI 0 "register_operand" "=r,r") > + (sign_extend:SI (match_operand:QI 1 "nonimmediate_operand" "r,m")))] > + "" > + "@ > + l.extbs\t%0, %1 > + l.lbs\t%0, %1" > + [(set_attr "insn_support" "sext,*")]) You don't really want to give the register allocator no choice but to spill to memory in the !sext case. Another r/r case with a splitter that is conditional on !sext would work. Otherwise, OK. r~
Re: [PATCH v3 3/3] or1k: gcc: initial support for openrisc
Hi Stafford,
Some minor comments. I didn't read the atomics much, but I did look at
everything else, and it looks fine :-)
On Sat, Oct 27, 2018 at 01:37:02PM +0900, Stafford Horne wrote:
> +case ${target} in
> +or1k*-*-linux*)
> +tm_file="${tm_file} gnu-user.h linux.h glibc-stdint.h"
> +tm_file="${tm_file} or1k/linux.h"
> +;;
Spaces instead of tabs.
> + /* Number of bytes saved on the stack for outgoing/sub-fucntion args. */
Typo ("function").
> + /* The sum of sizes: locals vars, called saved regs, stack pointer
> + * and an optional frame pointer.
> + * Used in expand_prologue () and expand_epilogue(). */
We don't use leading stars in comments (just spaces), normally.
> + /* Set address to volitile to ensure the store doesn't get optimized out.
> */
"volatile"
> +/* Helper for defining INITIAL_ELIMINATION_OFFSET.
> + We allow the following eliminiations:
> + FP -> HARD_FP or SP
> + AP -> HARD_FP or SP
> +
> + HFP and AP are the same which is handled below. */
> +
> +HOST_WIDE_INT
> +or1k_initial_elimination_offset (int from, int to)
You could calculate this as some_offset (from) - some_offset (to) with
some_offset a simple helper function. That gives you all possible
eliminations :-)
(Each offset is very cheap to compute in your case, so that's not a problem).
> +static rtx
> +or1k_function_value (const_tree valtype,
> + const_tree fn_decl_or_type ATTRIBUTE_UNUSED,
> + bool outgoing ATTRIBUTE_UNUSED)
Since we use C++ now you can write this as
bool /*outgoing*/)
or such, for enhanced readability.
> + split. Symbols are lagitimized using split relocations. */
"legitimized"
> +void
> +or1k_expand_move (machine_mode mode, rtx op0, rtx op1)
> +{
> + if (MEM_P (op0))
> +{
> + if (!const0_operand(op1, mode))
Space before (.
> +#undef TARGET_RTX_COSTS
> +#define TARGET_RTX_COSTS or1k_rtx_costs
You may want TARGET_INSN_COST as well (it is easier to get (more) correct).
> + if (hi != 0)
> + {
> + rtx scratch2 = gen_rtx_REG (Pmode, RV_REGNUM);
> + emit_move_insn (scratch2, GEN_INT (hi));
> + emit_insn (gen_add2_insn (scratch, scratch2));
> +}
Tab instead of spaces.
> + /* Generate a tail call to the target function. */
> + if (! TREE_USED (function))
No space after !.
> +#define TARGET_RETURN_IN_MEMORY or1k_return_in_memory
> +#define TARGET_PASS_BY_REFERENCE or1k_pass_by_reference
These two have a tab instead of a space (as the rest do).
> +#define WCHAR_TYPE_SIZE 32
And this.
> + This ABI has no adjacent call-saved register, which means that
> + DImode/DFmode pseudos cannot be call-saved and will always be
> + spilled across calls. To solve this without changing the ABI,
> + remap the compiler internal register numbers to place the even
> + call-saved registers r16-r30 in 24-31, and the odd call-clobbered
> + registers r17-r31 in 16-23. */
Ooh evilness :-)
> +#define PmodeSImode
> +#define FUNCTION_MODESImode
Some more tabs.
> +#define FUNCTION_ARG_REGNO_P(r) (r >= 3 && r <= 8)
IN_RANGE ?
> + { ARG_POINTER_REGNUM,STACK_POINTER_REGNUM },\
Weird tab here, too.
> +#define EH_RETURN_REGNUM HW_TO_GCC_REGNO (23)
And here.
> +(define_insn "xorsi3"
> + [(set (match_operand:SI 0 "register_operand" "=r,r")
> + (xor:SI
> +(match_operand:SI 1 "register_operand" "%r,r")
> +(match_operand:SI 2 "reg_or_s16_operand" " r,I")))]
> + ""
> + "@
> + l.xor\t%0, %1, %2
> + l.xori\t%0, %1, %2")
Is this correct? Should this be unsigned (u16 and K)?
https://sourceware.org/cgen/gen-doc/openrisc-insn.html suggest so, but I
don't know how up-to-date or relevant that is. Well. From the atomics
much below it seems to be correct, and signed is nice for making a bit
inverse. Is there some better documentation? Something to put at
https://gcc.gnu.org/readings.html (this is in the CVS repo, still see
https://gcc.gnu.org/about.html#cvs ).
> +(define_expand "mov"
> + [(set (match_operand:I 0 "nonimmediate_operand" "")
> + (match_operand:I 1 "general_operand" ""))]
You can completely leave out empty constraint strings, for example in the
expanders.
> +mhard-div
> +Target RejectNegative InverseMask(SOFT_DIV)
> +Use hardware divide instructions, use -msoft-div for emulation.
> +
> +mhard-mul
> +Target RejectNegative InverseMask(SOFT_MUL).
> +Use hardware multiply instructions, use -msoft-mul for emulation.
Maybe put the -msoft-* options near here then?
This was a lovely read. Thank you! Your port looks great.
Segher
