[PATCH] Fix up sqrt(x) < c and sqrt(x) >= c match.pd folding (PR tree-optimization/91734)

[Jakub Jelinek]

Hi!

As mentioned in the PR, the sqrt (x) < c optimization into x < c*c
sometimes breaks the boundary case, if c2=c*c is inexact then in some cases
we need to optimize it into x <= c*c rather than x < c*c.  The original
bugreport is when c is small and c2 is 0.0, then obviously we need <= 0.0
rather than < 0.0, but the testcase includes another example where it makes
a difference, plus has a >= testcase too.

Bootstrapped/regtested on powerpc64le-linux, ok for trunk?

2019-09-13  Jakub Jelinek  

PR tree-optimization/91734
* generic-match-head.c: Include fold-const-call.h.
* match.pd (sqrt(x) < c, sqrt(x) >= c): Check the boundary value and
in case inexact computation of c*c affects comparison of the boundary,
turn LT_EXPR into LE_EXPR or GE_EXPR into GT_EXPR.

* gcc.dg/pr91734.c: New test.

--- gcc/generic-match-head.c.jj 2019-07-20 21:02:09.296821929 +0200
+++ gcc/generic-match-head.c2019-09-12 10:52:33.091366624 +0200
@@ -29,6 +29,7 @@ along with GCC; see the file COPYING3.
 #include "cgraph.h"
 #include "vec-perm-indices.h"
 #include "fold-const.h"
+#include "fold-const-call.h"
 #include "stor-layout.h"
 #include "tree-dfa.h"
 #include "builtins.h"
--- gcc/match.pd.jj 2019-09-11 21:50:54.933504293 +0200
+++ gcc/match.pd2019-09-12 11:12:11.150987786 +0200
@@ -3541,56 +3541,71 @@ (define_operator_list COND_TERNARY
  if x is negative or NaN.  Due to -funsafe-math-optimizations,
  the results for other x follow from natural arithmetic.  */
(cmp @0 @1)))
- (if (cmp == GT_EXPR || cmp == GE_EXPR)
+ (if (cmp == LT_EXPR || cmp == LE_EXPR || cmp == GT_EXPR || cmp == GE_EXPR)
   (with
{
- REAL_VALUE_TYPE c2;
+REAL_VALUE_TYPE c2;
+enum tree_code ncmp = cmp;
 real_arithmetic (, MULT_EXPR,
  _REAL_CST (@1), _REAL_CST (@1));
 real_convert (, TYPE_MODE (TREE_TYPE (@0)), );
+/* See PR91734: if c2 is inexact and sqrt(c2) < c (or sqrt(c2) >= c),
+   then change LT_EXPR into LE_EXPR or GE_EXPR into GT_EXPR.  */
+if ((cmp == LT_EXPR || cmp == GE_EXPR) && !REAL_VALUE_ISINF (c2))
+  {
+tree c3 = fold_const_call (CFN_SQRT, TREE_TYPE (@0),
+   build_real (TREE_TYPE (@0), c2));
+if (c3 == NULL_TREE || TREE_CODE (c3) != REAL_CST)
+  ncmp = ERROR_MARK;
+else if (real_less (_REAL_CST (c3), _REAL_CST (@1)))
+  ncmp = cmp == LT_EXPR ? LE_EXPR : GT_EXPR;
+  }
}
-   (if (REAL_VALUE_ISINF (c2))
-   /* sqrt(x) > y is x == +Inf, when y is very large.  */
-   (if (HONOR_INFINITIES (@0))
-(eq @0 { build_real (TREE_TYPE (@0), c2); })
-{ constant_boolean_node (false, type); })
-   /* sqrt(x) > c is the same as x > c*c.  */
-   (cmp @0 { build_real (TREE_TYPE (@0), c2); }
- (if (cmp == LT_EXPR || cmp == LE_EXPR)
-  (with
-   {
-REAL_VALUE_TYPE c2;
-real_arithmetic (, MULT_EXPR,
- _REAL_CST (@1), _REAL_CST (@1));
-real_convert (, TYPE_MODE (TREE_TYPE (@0)), );
-   }
-   (if (REAL_VALUE_ISINF (c2))
-(switch
-/* sqrt(x) < y is always true, when y is a very large
-   value and we don't care about NaNs or Infinities.  */
-(if (! HONOR_NANS (@0) && ! HONOR_INFINITIES (@0))
- { constant_boolean_node (true, type); })
-/* sqrt(x) < y is x != +Inf when y is very large and we
-   don't care about NaNs.  */
-(if (! HONOR_NANS (@0))
- (ne @0 { build_real (TREE_TYPE (@0), c2); }))
-/* sqrt(x) < y is x >= 0 when y is very large and we
-   don't care about Infinities.  */
-(if (! HONOR_INFINITIES (@0))
- (ge @0 { build_real (TREE_TYPE (@0), dconst0); }))
-/* sqrt(x) < y is x >= 0 && x != +Inf, when y is large.  */
-(if (GENERIC)
- (truth_andif
-  (ge @0 { build_real (TREE_TYPE (@0), dconst0); })
-  (ne @0 { build_real (TREE_TYPE (@0), c2); }
-   /* sqrt(x) < c is the same as x < c*c, if we ignore NaNs.  */
-   (if (! HONOR_NANS (@0))
-(cmp @0 { build_real (TREE_TYPE (@0), c2); })
-/* sqrt(x) < c is the same as x >= 0 && x < c*c.  */
-(if (GENERIC)
- (truth_andif
-  (ge @0 { build_real (TREE_TYPE (@0), dconst0); })
-  (cmp @0 { build_real (TREE_TYPE (@0), c2); })
+   (if (cmp == GT_EXPR || cmp == GE_EXPR)
+   (if (REAL_VALUE_ISINF (c2))
+/* sqrt(x) > y is x == +Inf, when y is very large.  */
+(if (HONOR_INFINITIES (@0))
+ (eq @0 { build_real (TREE_TYPE (@0), c2); })
+ { constant_boolean_node (false, type); })
+/* sqrt(x) > c is the same as x > c*c.  */
+(if (ncmp != ERROR_MARK)
+ (if (ncmp == GE_EXPR)
+  (ge @0 { build_real (TREE_TYPE (@0), c2); })
+   

[patch, testsuite, arm] Fix ICE in gcc.dg/gimplefe-28.c

[Sandra Loosemore]

For the default multilib on arm-none-eabi, gcc.dg/gimplefe-28 has been 
getting an ICE because, while the target-supports infrastructure is 
probing to see if it can add the command-line options to enable the sqrt 
insn ("-mfpu=vfp -mfloat-abi=softfp"), it is not actually adding those 
options when building this testcase.  :-S  The hook to do this is 
already there; it just needs a case for arm.


OK to commit?

-Sandra
2019-09-13  Sandra Loosemore  

	gcc/testsuite/
	* lib/target-supports.exp (add_options_for_sqrt_insn): Add
	arm options consistent with check_effective_target_arm_vfp_ok.
Index: gcc/testsuite/lib/target-supports.exp
===
--- gcc/testsuite/lib/target-supports.exp	(revision 275699)
+++ gcc/testsuite/lib/target-supports.exp	(working copy)
@@ -6670,6 +6670,9 @@ proc add_options_for_sqrt_insn { flags }
 if { [istarget amdgcn*-*-*] } {
 	return "$flags -ffast-math"
 }
+if { [istarget arm*-*-*] } {
+	return "$flags -mfpu=vfp -mfloat-abi=softfp"
+}
 return $flags
 }
 

Re: [PATCH 4/4] Modifications to the testsuite

[Jan Hubicka]

> This are all modifications to the testsuite required to get to the
> state described in the cover letter of the entire IPA-SRA
> patch-series.  Please note that ipa/ipa-sra-2.c and ipa/ipa-sra-6.c
> should actually be svn rm-ed instead as they try to invoke
> functionality that the new IPA-SRA does not have (splitting aggregates
> passed by reference into individual bits passed by reference).  For
> more information, see the cover letter of the whole IPA-SRA patch-set.
> 
> This patch has already been approved by Jeff, but I'm re-sending it
> for completeness.  There has actually been a conflict in the options
> of an LTO testcase, that is the only change compared to the previous
> submission.
> 
> Martin
> 
> 2019-08-20  Martin Jambor  
> 
> * g++.dg/ipa/pr81248.C: Adjust dg-options and dump-scan.
> * gcc.dg/ipa/ipa-sra-1.c: Likewise.
> * gcc.dg/ipa/ipa-sra-10.c: Likewise.
> * gcc.dg/ipa/ipa-sra-11.c: Likewise.
> * gcc.dg/ipa/ipa-sra-3.c: Likewise.
> * gcc.dg/ipa/ipa-sra-4.c: Likewise.
> * gcc.dg/ipa/ipa-sra-5.c: Likewise.
> * gcc.dg/ipa/ipacost-2.c: Disable ipa-sra.
> * gcc.dg/ipa/ipcp-agg-9.c: Likewise.
> * gcc.dg/ipa/pr78121.c: Adjust scan pattern.
> * gcc.dg/ipa/vrp1.c: Likewise.
> * gcc.dg/ipa/vrp2.c: Likewise.
> * gcc.dg/ipa/vrp3.c: Likewise.
> * gcc.dg/ipa/vrp7.c: Likewise.
> * gcc.dg/ipa/vrp8.c: Likewise.
> * gcc.dg/noreorder.c: use noipa attribute instead of noinline.
> * gcc.dg/ipa/20040703-wpa.c: New test.
> * gcc.dg/ipa/ipa-sra-12.c: New test.
> * gcc.dg/ipa/ipa-sra-13.c: Likewise.
> * gcc.dg/ipa/ipa-sra-14.c: Likewise.
> * gcc.dg/ipa/ipa-sra-15.c: Likewise.
> * gcc.dg/ipa/ipa-sra-16.c: Likewise.
> * gcc.dg/ipa/ipa-sra-17.c: Likewise.
> * gcc.dg/ipa/ipa-sra-18.c: Likewise.
> * gcc.dg/ipa/ipa-sra-19.c: Likewise.
> * gcc.dg/ipa/ipa-sra-20.c: Likewise.
> * gcc.dg/ipa/ipa-sra-21.c: Likewise.
> * gcc.dg/ipa/ipa-sra-22.c: Likewise.
> * gcc.dg/sso/ipa-sra-1.c: Likewise.
> * g++.dg/ipa/ipa-sra-2.C: Likewise.
> * g++.dg/ipa/ipa-sra-3.C: Likewise.
> * gcc.dg/tree-ssa/ipa-cp-1.c: Make return value used.
> * g++.dg/ipa/devirt-19.C: Add missing return, add -fipa-cp-clone
> option.
> * g++.dg/lto/devirt-19_0.C: Add -fipa-cp-clone option.
> 
> * gcc.dg/ipa/ipa-sra-2.c: Mark to be removed.
> * gcc.dg/ipa/ipa-sra-6.c: Likewise.

OK,
thanks!
Honza

Re: [PATCH 3/4] New IPA-SRA implementation

[Jan Hubicka]

> This patch actually adds the analysis bits of IPA-SRA - both the
> function summary generation and the interprocedural analysis and
> decision stage.  The transformation itself then happens in the call
> graph cloning infrastructure changes which are in the previous patch.
> Please see the cover letter of the whole patch-set for more
> information.
> 
> This is mostly only a rebase on the current trunk of the earlier
> submission, the only functional change is that the pass does not clone
> when all the work (unused parameter removal) has already been done by
> IPA-CP.
> 
> Martin
> 
> 2019-08-20  Martin Jambor  
> 
> * Makefile.in (GTFILES): Added ipa-sra.c.
> (OBJS): Added ipa-sra.o.
> * dbgcnt.def: Added ipa_sra_params and ipa_sra_retvalues.
> * doc/invoke.texi (ipa-sra-max-replacements): New.
> * ipa-sra.c: New file.
> * lto-section-in.c (lto_section_name): Added ipa-sra section.
> * lto-streamer.h (lto_section_type): Likewise.
> * params.def (PARAM_IPA_SRA_MAX_REPLACEMENTS): New.
> * passes.def: Add new IPA-SRA.
> * tree-pass.h (make_pass_ipa_sra): Declare.
OK
> +#define IPA_SRA_MAX_PARAM_FLOW_LEN 7
I would move this to the place ISRA_ARG_SIZE_LIMIT_BITS is defined so
they appaer at same place.  Perhaps C++ way would be to use constant
member variable?
> +
> +/* Structure to describe which formal parameters feed into a particular 
> actual
> +   arguments.  */
> +
> +struct isra_param_flow
> +{
> +  /* Number of elements in array inputs that contain valid data.  */
> +  char length;
> +  /* Indices of formal parameters that feed into the described actual 
> argument.
> + If aggregate_pass_through or pointer_pass_through below are true, it 
> must
> + contain exactly one element which is passed through from a formal
> + parameter if the given number.  Otherwise, the array contains indices of
> + collee's formal parameters which are used to calculate value of this
> + actual argument. */
> +  unsigned char inputs[IPA_SRA_MAX_PARAM_FLOW_LEN];
> +
> +  /* Offset within the formal parameter.  */
> +  unsigned unit_offset;
> +  /* Size of the portion of the formal parameter that is being passed.  */
> +  unsigned unit_size : ISRA_ARG_SIZE_LIMIT_BITS;

> +
> +  /* True when the value of this actual copy is a portion of a formal
> + parameter.  */
> +  unsigned aggregate_pass_through : 1;
> +  /* True when the value of this actual copy is a verbatim pass through of an
> + obtained pointer.  */
> +  unsigned pointer_pass_through : 1;
> +  /* True when it is safe to copy access candidates here from the callee, 
> which
> + would mean introducing dereferences into callers of the caller.  */
> +  unsigned safe_to_import_accesses : 1;
> +};
> +
> +/* Strucutre used to convey information about calls from the intra-procedurwl
> +   analysis stage to inter-procedural one.  */
> +
> +class isra_call_summary
> +{
> +public:
> +  isra_call_summary ()
> +: m_arg_flow (), m_return_ignored (false), m_return_returned (false),
> +  m_bit_aligned_arg (false)
> +  {}
> +
> +  void init_inputs (unsigned arg_count);
> +  void dump (FILE *f);
> +
> +  /* Information about what formal parameters of the caller are used to 
> compute
> + indivisual actual arguments of this call.  */
> +  auto_vec  m_arg_flow;
> +
> +  /* Set to true if the call statement does not have a LHS.  */
> +  unsigned m_return_ignored : 1;
> +
> +  /* Set to true if the LHS of call statement is only used to construct the
> + return value of the caller.  */
> +  unsigned m_return_returned : 1;
> +
> +  /* Set when any of the call arguments are not byte-aligned.  */
> +  unsigned m_bit_aligned_arg : 1;
> +};
> +
> +/* Class to manage function summaries.  */
> +
> +class GTY((user)) ipa_sra_function_summaries
> +  : public function_summary 
> +{
> +public:
> +  ipa_sra_function_summaries (symbol_table *table, bool ggc):
> +function_summary (table, ggc) { }
> +
> +  virtual void duplicate (cgraph_node *, cgraph_node *,
> +   isra_func_summary *old_sum,
> +   isra_func_summary *new_sum);
> +};
> +
> +/* Hook that is called by summary when a node is duplicated.  */
> +
> +void
> +ipa_sra_function_summaries::duplicate (cgraph_node *, cgraph_node *,
> +isra_func_summary *old_sum,
> +isra_func_summary *new_sum)
> +{
> +  /* TODO: Somehow stop copying when ISRA is doing the cloning, it is
> + useless.  */
> +  new_sum->m_candidate  = old_sum->m_candidate;
> +  new_sum->m_returns_value = old_sum->m_returns_value;
> +  new_sum->m_return_ignored = old_sum->m_return_ignored;
> +  gcc_assert (!old_sum->m_queued);
> +  new_sum->m_queued = false;
> +
> +  unsigned param_count = vec_safe_length (old_sum->m_parameters);
> +  if (!param_count)
> +return;
> +  vec_safe_reserve_exact (new_sum->m_parameters, param_count);
> +  

[PING^2][PATCH 0/3] GNAT test suite fixes for build sysroot

[Maciej W. Rozycki]

On Tue, 14 May 2019, Maciej W. Rozycki wrote:

>  In the course of setting up GCC regression testing for the RISC-V target 
> I have discovered that the GNAT test suite does not correctly respond to 
> the test environment settings passed from the test harness in my setup and 
> consequently no test case works correctly.

 Ping for:




  Maciej

[committed v2 1/3][GCC] gnatmake: Accept the `--sysroot=' GCC driver option

[Maciej W. Rozycki]

According to `gnatmake' documentation:

"Any uppercase or multi-character switch that is not a 'gnatmake' switch
is passed to 'gcc' (e.g., '-O', '-gnato,' etc.)"

however the `--sysroot=' switch is actually rejected:

gnatmake: invalid switch: --sysroot=...

likely because it is one of the very few GCC driver options that have a 
leading double dash and therefore we don't have a blanket fall-through 
for such switches that would satisfy what our documentation claims.

The option is actually shared between the compiler and the linker, so 
pass the switch to both build stages if requested, removing GNAT 
testsuite issues like:

gnatmake: invalid switch: --sysroot=.../sysroot
compiler exited with status 1
Executing on host: .../gcc/gnatclean -c -q -n ./abstract_with_anonymous_result  
 (timeout = 300)
spawn -ignore SIGHUP .../gcc/gnatclean -c -q -n ./abstract_with_anonymous_result
PASS: gnat.dg/abstract_with_anonymous_result.adb (test for excess errors)
UNRESOLVED: gnat.dg/abstract_with_anonymous_result.adb compilation failed to 
produce executable

in a test environment where `--with-build-sysroot=.../sysroot' has been 
used to build a cross-compiler.  Passing to the compilation stage only 
would lead to errors like:

.../bin/riscv64-linux-gnu-ld: cannot find crt1.o: No such file or directory
.../bin/riscv64-linux-gnu-ld: cannot find -lc
collect2: error: ld returned 1 exit status
gnatlink: error when calling .../gcc/xgcc
gnatmake: *** link failed.
compiler exited with status 1
Executing on host: .../gcc/gnatclean -c -q -n ./abstract_with_anonymous_result  
 (timeout = 300)
spawn -ignore SIGHUP .../gcc/gnatclean -c -q -n ./abstract_with_anonymous_result
./abstract_with_anonymous_result.ali
./abstract_with_anonymous_result.o
FAIL: gnat.dg/abstract_with_anonymous_result.adb (test for excess errors)
Excess errors:
.../bin/riscv64-linux-gnu-ld: cannot find crt1.o: No such file or directory
.../bin/riscv64-linux-gnu-ld: cannot find -lc
gnatlink: error when calling .../gcc/xgcc

UNRESOLVED: gnat.dg/abstract_with_anonymous_result.adb compilation failed to 
produce executable

instead.

gcc/ada/
* make.adb (Scan_Make_Arg): Also accept `--sysroot=' for the 
compiler and the linker.
---
Hi,

On Thu, 20 Jun 2019, Arnaud Charlet wrote:

> > > Have you resolved your copyright assignment issues since then?
> > 
> >  The ball is now in FSF's court I'm told.
> 
> OK

 This has now been sorted.

> > > The above patch needs to use "or else" instead of "or". OK with this 
> > > change
> > > on the above patch.
> > 
> >  OK, I have updated that in my patch.
> > 
> >  Technically both variants of the expression achieve the same effect here 
> > as there is no problem with evaluating both sides of the OR operation in 
> > all cases, but your suggestion might help the readers avoid scratching 
> > their heads.
> 
> The performance isn't the same, and more importantly, this is the documented
> Ada coding style for GNAT: 
> https://gcc.gnu.org/onlinedocs/gnat-style/Statements.html#Statements

 Ack.

 This is the version I have committed.  Thank you for your review.

  Maciej
---
 gcc/ada/make.adb |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

gcc-gnatmake-sysroot.diff
Index: gcc/gcc/ada/make.adb
===
--- gcc.orig/gcc/ada/make.adb
+++ gcc/gcc/ada/make.adb
@@ -4516,7 +4516,9 @@ package body Make is
end;
 end if;
 
- elsif Argv'Length >= 8 and then Argv (1 .. 8) = "--param=" then
+ elsif (Argv'Length >= 8 and then Argv (1 .. 8) = "--param=")
+   or else (Argv'Length >= 10 and then Argv (1 .. 10) = "--sysroot=")
+ then
 Add_Switch (Argv, Compiler);
 Add_Switch (Argv, Linker);
 

Re: [PATCH 2/4] New parameter manipulation infrastructure

[Jan Hubicka]

> 2019-08-20  Martin Jambor  
> 
> * Makefile.in (GTFILES): Added ipa-param-manipulation.h.
> * cgraph.h (ipa_replace_map): Removed fields old_tree, replace_p
> and ref_p, added fields param_adjustments and performed_splits.
> (struct cgraph_clone_info): Remove ags_to_skip and
> combined_args_to_skip, new field param_adjustments.
> (cgraph_node::create_clone): Changed parameters to use
> ipa_param_adjustments.
> (cgraph_node::create_virtual_clone): Likewise.
> (cgraph_node::create_virtual_clone_with_body): Likewise.
> (tree_function_versioning): Likewise.
> (cgraph_build_function_type_skip_args): Removed.
> * cgraph.c (cgraph_edge::redirect_call_stmt_to_callee): Convert to
> using ipa_param_adjustments.
> (clone_of_p): Likewise.
> * cgraphclones.c (cgraph_build_function_type_skip_args): Removed.
> (build_function_decl_skip_args): Likewise.
> (duplicate_thunk_for_node): Adjust parameters using
> ipa_param_body_adjustments, copy param_adjustments instead of
> args_to_skip.
> (cgraph_node::create_clone): Convert to using ipa_param_adjustments.
> (cgraph_node::create_virtual_clone): Likewise.
> (cgraph_node::create_version_clone_with_body): Likewise.
> (cgraph_materialize_clone): Likewise.
> (symbol_table::materialize_all_clones): Likewise.
> * coretypes.h (cgraph_edge): Declare.
> * ipa-cp.c (get_replacement_map): Do not initialize removed fields.
> (initialize_node_lattices): Make aware that some parameters might have
> already been removed.
> (want_remove_some_param_p): New function.
> (create_specialized_node): Convert to using ipa_param_adjustments and
> deal with possibly pre-existing adjustments.
> * ipa-fnsummary.c (ipa_fn_summary_t::duplicate): Simplify
> ipa_replace_map check.
> * ipa-inline-transform.c (save_inline_function_body): Update to
> refelct new tree_function_versioning signature.
> * ipa-param-manipulation.c: Rewrite.
> * ipa-param-manipulation.h: Likewise.
> * ipa-prop.c (adjust_agg_replacement_values): Use a helper from
> ipa_param_adjustments to get current parameter indices.
> (ipcp_modif_dom_walker::before_dom_children): Likewise.
> (ipcp_update_bits): Likewise.
> (ipcp_update_vr): Likewise.
> * ipa-split.c (split_function): Convert to using 
> ipa_param_adjustments.
> * lto-cgraph.c (output_cgraph_opt_summary_p): Likewise.
> (output_node_opt_summary): Do not stream removed fields.  Stream
> parameter adjustments instead of argumetns to skip.
> (input_node_opt_summary): Likewise.
> (input_node_opt_summary): Likewise.
> * multiple_target.c (create_target_clone): Update to reflet new type
> of create_version_clone_with_body.
> * omp-simd-clone.c (simd_clone_vector_of_formal_parm_types): Adjust
> for the new interface.
> (simd_clone_clauses_extract): Likewise, make args an auto_vec.
> (simd_clone_compute_base_data_type): Likewise.
> (simd_clone_init_simd_arrays): Adjust for the new interface.
> (simd_clone_adjust_argument_types): Likewise.
> (struct modify_stmt_info): Likewise.
> (ipa_simd_modify_stmt_ops): Likewise.
> (ipa_simd_modify_function_body): Likewise.
> (simd_clone_adjust): Likewise.
> * trans-mem.c (ipa_tm_create_version): Update to reflect new type of
> tree_function_versioning.
> * tree-inline.h (copy_body_data): New fields killed_new_ssa_names and
> param_body_adjs.
> (copy_decl_to_var): Declare.
> * tree-inline.c (update_clone_info): Do not remap old_tree.
> (remap_gimple_stmt): Use ipa_param_body_adjustments to modify gimple
> statements, walk all extra generated statements and remap their
> operands.
> (redirect_all_calls): Add killed SSA names to a hash set.
> (remap_ssa_name): Do not remap killed SSA names.
> (copy_arguments_for_versioning): Renames to copy_arguments_nochange,
> half of functionality moved to ipa_param_body_adjustments.
> (copy_decl_to_var): Make exported.
> (copy_body): Destroy killed_new_ssa_names hash set.
> (expand_call_inline): Remap performed splits.
> (update_clone_info): Likewise.
> (tree_function_versioning): Simplify tree_map processing.  Updated to
> accept ipa_param_adjustments and use ipa_param_body_adjustments.

OK
> +/* Modify actual arguments of a function call in statement STMT, assuming it
> +   calls CALLEE_DECL.  CALLER_ADJ must be the description of parameter
> +   adjustments of the caller or NULL if there are none.  Return the new
> +   statement that replaced the old one.  When invoked, cfun and
> +   

Re: [PATCH 1/4] Remove old IPA-SRA, introduce tree-sra.h

[Jan Hubicka]

> This patch removes the old IPA-SRA.  Please see the covert letter for
> more information about the whole patch-set.
> 
> Martin
> 
> 2019-07-23  Martin Jambor  
> 
> * dbgcnt.def: Remove eipa_sra.
> * passes.def: Remove old IPA-SRA.
> * tree-pass.h (make_pass_early_ipa_sra): Remove declaration.
> * tree-sra.c: Removed IPA-SRA.  Include tree-sra.h.
> (type_internals_preclude_sra_p): Make public.
> * tree-sra.h: New file.

this patch is OK (in combination with adding IPA-SRA)
thanks,
Honza

libgo patch committed: Don't use \? in grep pattern

[Ian Lance Taylor]

This libgo patch avoids using \? in a grep pattern.  It's not
supported by Solaris grep.  Just use * instead; it matches more but it
shouldn't matter.  This fixes GCC PR 91764.  Bootstrapped and ran Go
tests on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 275698)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-ceb1e4f5614b4772eed44f9cf57780e52f44753e
+5af62eda697da21155091cf5375ed9edb4639b67
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/match.sh
===
--- libgo/match.sh  (revision 275698)
+++ libgo/match.sh  (working copy)
@@ -135,7 +135,7 @@ for f in $gofiles; do
 
 if test x$tag1 != xnonmatchingtag -a x$tag2 != xnonmatchingtag; then
# Pipe through cat so that `set -e` doesn't affect fgrep.
-   tags=`sed '/^package /q' < $f | grep '^// \?+build ' | cat`
+   tags=`sed '/^package /q' < $f | grep '^// *+build ' | cat`
omatch=true
first=true
match=false
Index: libgo/testsuite/gotest
===
--- libgo/testsuite/gotest  (revision 275698)
+++ libgo/testsuite/gotest  (working copy)
@@ -326,7 +326,7 @@ x)
esac
 
if test x$tag1 != xnonmatchingtag -a x$tag2 != xnonmatchingtag; then
-   tags=`sed '/^package /q' < $f | grep '^// \?+build '`
+   tags=`sed '/^package /q' < $f | grep '^// *+build '`
omatch=true
first=true
match=false

Re: [PATCH] Fix PR 91708

[Richard Biener]

On Fri, 13 Sep 2019, Bernd Edlinger wrote:

> On 9/13/19 1:23 PM, Richard Biener wrote:
> > On Thu, 12 Sep 2019, Bernd Edlinger wrote:
> > 
> >> On 9/12/19 10:08 AM, Richard Biener wrote:
> >>> On Wed, 11 Sep 2019, Bernd Edlinger wrote:
> >>>
>  On 9/11/19 8:30 PM, Richard Biener wrote:
> >>>
> >>> More like the following?  I wonder if we can assert that
> >>> MEM_NOTRAP_P () are equal (see all the for_gcse checks in exp_equiv_p).
> >>> But as said earlier I wonder in which cases a replacement is
> >>> profitable at all - thus, if a
> >>>
> >>>   else if (MEM_P (trial))
> >>> /* Do not replace anything with a MEM.  */
> >>> ;
> >>>
> >>
> >> Yes, I like that better, since there is essentially nothing stopping
> >> it from replacing a REG with a MEM, except the rtxcost function, maybe.
> >>
> >> It turned out that the previous version got into an endless loop here,
> >> since the loop termination happens when replacing MEM by itself, except
> >> when something else terminates the search.
> > 
> > Is that how it works without the patch as well?  I admit the loop
> > is a bit hard to follow since it is not only iterating
> > over elt->next_same_value ...
> > 
> 
> Yes, that seems to be the case, since the memory reference itself
> is always in the set.  There was either an endless loop, when
> the src variable was set this if can always be taken, which makes
> no progress:
> 
>   else if (src
>&& preferable (src_cost, src_regcost,
>   src_eqv_cost, src_eqv_regcost) <= 0
>&& preferable (src_cost, src_regcost,
>   src_related_cost, src_related_regcost) <= 0
>&& preferable (src_cost, src_regcost,
>   src_elt_cost, src_elt_regcost) <= 0)
> trial = src, src_cost = MAX_COST;
> 
> or there was a crash when the above was not taken then:
> 
>   else
> {
>   trial = elt->exp;
>   elt = elt->next_same_value;
>   src_elt_cost = MAX_COST;
> }
> 
> crashes, because elt == NULL at some point.
> 
> >> So how about this?
> >>
> >> The only possible MEM->MEM transformations are now:
> >> - replacing trial = dest (result mov dest, dest; which is a no-op insn)
> >> - replacing trial = src (which is a no-op transformation, and exits the 
> >> loop)
> >>
> >> Therefore the overlapping mem move handling no longer necessary. 
> >>
> >>
> >> Bootstrap and reg-tested on x86_64-pc-linux-gnu.
> >> Is it OK for trunk?
> > 
> > Looks OK to me.  Can you see if this has any unexpected code-generation
> > effects?  I would expect it to not make a difference at all, but you
> > never know -- any differences in cc1files?
> > 
> 
> I tried to install the patch after _stage1_ was competed,
> but there was no bootstrap miscompare (on x86_64-pc-linux-gnu)
> so, no this does not make any difference here.

The patch is OK for trunk then.

Thanks,
Richard.

[PATCH testsuite, arm] cache fp16 hw effective-target tests

[Sandra Loosemore]

In some bare-metal environments, the tests for fp16 runtime support fail 
in a way that causes a timeout rather than immediate failure.  (E.g., 
the runtime might provide a do-nothing exception handler that just sits 
in a tight loop and never returns.)  This patch changes the 
effective-target tests for fp16 hardware support to cache the result of 
the test so that we don't have to do this more than once.  I think it 
was probably just an oversight that it wasn't done this way originally, 
since the target is hardly likely to sprout fp16 instruction support 
midway through the test run anyway.  ;-)  Anyway, test results are the 
same with this patch, they just run faster.  OK to commit?


-Sandra
2019-09-13  Sandra Loosemore  

	gcc/testsuite/
	* lib/target-supports.exp
	(check_effective_target_arm_neon_fp16_hw)
	(check_effective_target_arm_fp16_hw): Use check_runtime
	instead of check_runtime_nocache.
Index: gcc/testsuite/lib/target-supports.exp
===
--- gcc/testsuite/lib/target-supports.exp	(revision 275699)
+++ gcc/testsuite/lib/target-supports.exp	(working copy)
@@ -3909,7 +3909,7 @@ proc check_effective_target_arm_neon_fp1
 	return 0
 }
 global et_arm_neon_fp16_flags
-check_runtime_nocache arm_neon_fp16_hw {
+check_runtime arm_neon_fp16_hw {
 	int
 	main (int argc, char **argv)
 	{
@@ -4162,7 +4162,7 @@ proc check_effective_target_arm_fp16_hw
 	return 0
 }
 global et_arm_fp16_flags
-check_runtime_nocache arm_fp16_hw {
+check_runtime arm_fp16_hw {
 	int
 	main (int argc, char **argv)
 	{

Re: [PATCH] Fix PR 91708

[Bernd Edlinger]

On 9/13/19 1:23 PM, Richard Biener wrote:
> On Thu, 12 Sep 2019, Bernd Edlinger wrote:
> 
>> On 9/12/19 10:08 AM, Richard Biener wrote:
>>> On Wed, 11 Sep 2019, Bernd Edlinger wrote:
>>>
 On 9/11/19 8:30 PM, Richard Biener wrote:
>>>
>>> More like the following?  I wonder if we can assert that
>>> MEM_NOTRAP_P () are equal (see all the for_gcse checks in exp_equiv_p).
>>> But as said earlier I wonder in which cases a replacement is
>>> profitable at all - thus, if a
>>>
>>>   else if (MEM_P (trial))
>>> /* Do not replace anything with a MEM.  */
>>> ;
>>>
>>
>> Yes, I like that better, since there is essentially nothing stopping
>> it from replacing a REG with a MEM, except the rtxcost function, maybe.
>>
>> It turned out that the previous version got into an endless loop here,
>> since the loop termination happens when replacing MEM by itself, except
>> when something else terminates the search.
> 
> Is that how it works without the patch as well?  I admit the loop
> is a bit hard to follow since it is not only iterating
> over elt->next_same_value ...
> 

Yes, that seems to be the case, since the memory reference itself
is always in the set.  There was either an endless loop, when
the src variable was set this if can always be taken, which makes
no progress:

  else if (src
   && preferable (src_cost, src_regcost,
  src_eqv_cost, src_eqv_regcost) <= 0
   && preferable (src_cost, src_regcost,
  src_related_cost, src_related_regcost) <= 0
   && preferable (src_cost, src_regcost,
  src_elt_cost, src_elt_regcost) <= 0)
trial = src, src_cost = MAX_COST;

or there was a crash when the above was not taken then:

  else
{
  trial = elt->exp;
  elt = elt->next_same_value;
  src_elt_cost = MAX_COST;
}

crashes, because elt == NULL at some point.

>> So how about this?
>>
>> The only possible MEM->MEM transformations are now:
>> - replacing trial = dest (result mov dest, dest; which is a no-op insn)
>> - replacing trial = src (which is a no-op transformation, and exits the loop)
>>
>> Therefore the overlapping mem move handling no longer necessary. 
>>
>>
>> Bootstrap and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
> 
> Looks OK to me.  Can you see if this has any unexpected code-generation
> effects?  I would expect it to not make a difference at all, but you
> never know -- any differences in cc1files?
> 

I tried to install the patch after _stage1_ was competed,
but there was no bootstrap miscompare (on x86_64-pc-linux-gnu)
so, no this does not make any difference here.


Bernd.

> Not really my primary area of expertise...
> 
> Thanks,
> Richard.
> 

Re: [PATCH][GCC] Update my email address

[Sam Tebbs]

Patch attached.

On 13/09/2019 13:34, Sam Tebbs wrote:
> Hi all,
>
> This patch changes my email address in the MAINTAINERS file. Committed 
> as r275697.
>
> 2019-09-13  Sam Tebbs  
>
> gcc/ChangeLog
>
>     * MAINTAINERS (Sam Tebbs): Update email address.
>
>
diff --git a/MAINTAINERS b/MAINTAINERS
index 42f08a57767cc8a5a6c222a4aff0341983b3e3dc..948d56d8346ba2df42142955910d4e8a74f568e5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -598,7 +598,7 @@ Gabriele Svelto	
 Toma Tabacu	
 Sriraman Tallam	
 Samuel Tardieu	
-Sam Tebbs	
+Sam Tebbs	
 Dinar Temirbulatov
 Kresten Krab Thorup
 Kai Tietz	

doc patch committed: Fix typo

[Ian Lance Taylor]

I committed this patch to fix a typo in the -flto docs.

Ian

2019-09-13  Ian Lance Taylor  

* doc/invoke.texi (Optimize Options): Fix typo.
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi (revision 275698)
+++ gcc/doc/invoke.texi (working copy)
@@ -10343,7 +10343,7 @@ To enable debug info generation you need
 compile-time.  If any of the input files at link time were built
 with debug info generation enabled the link will enable debug info
 generation as well.  Any elaborate debug info settings
-like the dwarf level @option{-gdwarf-5} need to be explicitely repeated
+like the dwarf level @option{-gdwarf-5} need to be explicitly repeated
 at the linker command line and mixing different settings in different
 translation units is discouraged.
 

Re: [PATCH] DWARF array bounds missing from C++ array definitions

[Richard Biener]

On Fri, Sep 13, 2019 at 1:32 AM Alexandre Oliva  wrote:
>
> On Sep 12, 2019, Richard Biener  wrote:
>
> > Your new predicate looks a bit excessive here given it builds the type
> > again?
>
> Err, there seems to be some misunderstanding here.  The predicate avoids
> outputting a type for the definition if it's the same DIE that would go
> in the specification.  Now, when it's a different DIE, sometimes it
> might still refer to the same type, as in array-2.C, but I think that's
> not just acceptable, it's desirable, for it reflects the different ways
> used to denote the same type.

Yes.

> Before posting the patch, I added an inform() to the case in which
> completing_type_p would return true (bringing about a debug info
> change), and reviewed all of the messages in a bootstrap.  The only ones
> that weren't just adding array element count were those that inspired
> array-2.C and array-3.C.
>
> > So I'm obviously fine with your patch and I guess if we independently
> > arrive at this solution that answers my question what "correct DWARF"
> > is by a majority decision ;)
>
> Good.  Once we clear up the misunderstanding (I'm not sure whether you
> misunderstood the patch, or I misunderstood your response), I'll be glad
> to put it in.
>
> > So - maybe we can have the patch a bit cleaner by adding
> > a flag to add_type_attribute saying we only want it if it's
> > different from that already present (on the specification DIE)?
>
> That's exactly what I meant completing_type_p to check.  Do you mean it
> should be stricter, do it more cheaply, or what?

I meant to do it more cheaply, also the name completing_type_p is
misleading IMHO since it simply checks (re-)creating the type DIE
will yield a different one.  In case the FE would be so stupid to
first dwarf2out_early_global_decl with a complete type and then
later with an incomplete we'd still place a type DIE with the now
incomplete type on the specification DIE ...

In my view what the FE does is fishy (re-use the DECL node but
replace its type).  What we'd need here is probably the
merge_decls equivalent in the dwarf2out API, but not exactly
sure how that would look like (create a new variable DIE and
splice out "common" parts into an abstract DIE used by
both the old and the new DIE?)

Anyhow, I arrived at the same solution - if the type DIE we
created in gen_decl_die is different than the one already
present and we're creating a specification, put that type
on the new DIE, "shadowing" the declaration ones.

Richard.


>
> --
> Alexandre Oliva, freedom fighter  he/him   https://FSFLA.org/blogs/lxo
> Be the change, be Free!   FSF.org & FSF Latin America board member
> GNU Toolchain EngineerFree Software Evangelist
> Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara

[PATCH][GCC] Update my email address

[Sam Tebbs]

Hi all,

This patch changes my email address in the MAINTAINERS file. Committed 
as r275697.

2019-09-13  Sam Tebbs  

gcc/ChangeLog

     * MAINTAINERS (Sam Tebbs): Update email address.

Re: [PATCH 3/9] operand_equal_p: add support for OBJ_TYPE_REF.

[Martin Liška]

On 8/20/19 8:39 AM, Richard Biener wrote:

Anyhow, the original patch is OK if you compare
OBJ_TYPE_REF_TOKEN/OBJECT irrespective of virtual_method_call_p
and order the types_same_for_odr last since that's most expensive.


Hi.

It's done in the attached patch that survives bootstrap and regression
tests.

Martin
>From 645e2df84ccd1f9a4b41f0a73a5398ff81696cdc Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Thu, 15 Aug 2019 10:34:41 +0200
Subject: [PATCH 3/9] operand_equal_p: add support for OBJ_TYPE_REF.

gcc/ChangeLog:

2019-07-24  Martin Liska  

	* fold-const.c (operand_equal_p): Support OBJ_TYPE_REF.
	* tree.c (add_expr): Hash parts of OBJ_TYPE_REF.
---
 gcc/fold-const.c | 18 ++
 gcc/tree.c   |  6 ++
 2 files changed, 24 insertions(+)

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index f57fffb9655..12f5a06a524 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -3325,6 +3325,24 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
 	  flags &= ~OEP_ADDRESS_OF;
 	  return OP_SAME (1) && OP_SAME (2);
 
+	/* Virtual table call.  */
+	case OBJ_TYPE_REF:
+	  {
+	if (!operand_equal_p (OBJ_TYPE_REF_EXPR (arg0),
+  OBJ_TYPE_REF_EXPR (arg1), flags))
+	  return false;
+	if (tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg0))
+		!= tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg1)))
+	  return false;
+	if (!operand_equal_p (OBJ_TYPE_REF_OBJECT (arg0),
+  OBJ_TYPE_REF_OBJECT (arg1), flags))
+	  return false;
+	if (!types_same_for_odr (obj_type_ref_class (arg0),
+ obj_type_ref_class (arg1)))
+	  return false;
+	return true;
+	  }
+
 	default:
 	  return false;
 	}
diff --git a/gcc/tree.c b/gcc/tree.c
index b5e5876bbb3..20eb1682435 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -8028,6 +8028,12 @@ add_expr (const_tree t, inchash::hash , unsigned int flags)
 	  inchash::add_expr (TARGET_EXPR_SLOT (t), hstate, flags);
 	  return;
 
+	/* Virtual table call.  */
+	case OBJ_TYPE_REF:
+	  inchash::add_expr (OBJ_TYPE_REF_EXPR (t), hstate, flags);
+	  inchash::add_expr (OBJ_TYPE_REF_TOKEN (t), hstate, flags);
+	  inchash::add_expr (OBJ_TYPE_REF_OBJECT (t), hstate, flags);
+	  return;
 	default:
 	  break;
 	}
-- 
2.23.0

Re: [PATCH] Fix PR 91708

[Richard Biener]

On Thu, 12 Sep 2019, Bernd Edlinger wrote:

> On 9/12/19 10:08 AM, Richard Biener wrote:
> > On Wed, 11 Sep 2019, Bernd Edlinger wrote:
> > 
> >> On 9/11/19 8:30 PM, Richard Biener wrote:
> > 
> > More like the following?  I wonder if we can assert that
> > MEM_NOTRAP_P () are equal (see all the for_gcse checks in exp_equiv_p).
> > But as said earlier I wonder in which cases a replacement is
> > profitable at all - thus, if a
> > 
> >   else if (MEM_P (trial))
> > /* Do not replace anything with a MEM.  */
> > ;
> > 
> 
> Yes, I like that better, since there is essentially nothing stopping
> it from replacing a REG with a MEM, except the rtxcost function, maybe.
> 
> It turned out that the previous version got into an endless loop here,
> since the loop termination happens when replacing MEM by itself, except
> when something else terminates the search.

Is that how it works without the patch as well?  I admit the loop
is a bit hard to follow since it is not only iterating
over elt->next_same_value ...

> So how about this?
> 
> The only possible MEM->MEM transformations are now:
> - replacing trial = dest (result mov dest, dest; which is a no-op insn)
> - replacing trial = src (which is a no-op transformation, and exits the loop)
> 
> Therefore the overlapping mem move handling no longer necessary. 
> 
> 
> Bootstrap and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?

Looks OK to me.  Can you see if this has any unexpected code-generation
effects?  I would expect it to not make a difference at all, but you
never know -- any differences in cc1files?

Not really my primary area of expertise...

Thanks,
Richard.

Re: [PATCH] Fix PR fortran/91716

[Janne Blomqvist]

On Fri, Sep 13, 2019 at 1:07 PM Bernd Edlinger
 wrote:
>
> Hi,
>
> this fixes a test case where a short string constant is put in a larger 
> memory object.
>
> The consistency check in varasm.c is failed because both types should agree.
>
> Since the failed assertion is just a gcc_checking_assert I think a back-port 
> of this fix
> to the gcc-9 branch will not be necessary.
>
>
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
>
>
> Thanks
> Bernd.

Ok.

-- 
Janne Blomqvist

[PATCH] Fix PR fortran/91716

[Bernd Edlinger]

Hi,

this fixes a test case where a short string constant is put in a larger memory 
object.

The consistency check in varasm.c is failed because both types should agree.

Since the failed assertion is just a gcc_checking_assert I think a back-port of 
this fix
to the gcc-9 branch will not be necessary.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.2019-09-13  Bernd Edlinger  

	PR fortran/91716
	* trans-array.c (gfc_conv_array_initializer): Always assign the
	array type of the field to the string constant.

testsuite:
2019-09-13  Bernd Edlinger  

	PR fortran/91716
	* gfortran.dg/pr91716.f90: New test.

Index: gcc/fortran/trans-array.c
===
--- gcc/fortran/trans-array.c	(revision 275685)
+++ gcc/fortran/trans-array.c	(working copy)
@@ -6108,9 +6108,12 @@ gfc_conv_array_initializer (tree type, gfc_expr *
 		  tree atype = type;
 		  while (TREE_CODE (TREE_TYPE (atype)) == ARRAY_TYPE)
 		atype = TREE_TYPE (atype);
-		  if (TREE_CODE (TREE_TYPE (atype)) == INTEGER_TYPE
-		  && tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (se.expr)))
-			 > tree_to_uhwi (TYPE_SIZE_UNIT (atype)))
+		  gcc_checking_assert (TREE_CODE (TREE_TYPE (atype))
+   == INTEGER_TYPE);
+		  gcc_checking_assert (TREE_TYPE (TREE_TYPE (se.expr))
+   == TREE_TYPE (atype));
+		  if (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (se.expr)))
+		  > tree_to_uhwi (TYPE_SIZE_UNIT (atype)))
 		{
 		  unsigned HOST_WIDE_INT size
 			= tree_to_uhwi (TYPE_SIZE_UNIT (atype));
@@ -6117,8 +6120,8 @@ gfc_conv_array_initializer (tree type, gfc_expr *
 		  const char *p = TREE_STRING_POINTER (se.expr);
 
 		  se.expr = build_string (size, p);
-		  TREE_TYPE (se.expr) = atype;
 		}
+		  TREE_TYPE (se.expr) = atype;
 		}
 	  break;
 
Index: gcc/testsuite/gfortran.dg/pr91716.f90
===
--- gcc/testsuite/gfortran.dg/pr91716.f90	(revision 0)
+++ gcc/testsuite/gfortran.dg/pr91716.f90	(working copy)
@@ -0,0 +1,8 @@
+! { dg-do compile }
+! PR fortran/91716
+! Code contributed by Gerhard Steinmetz
+module m
+   type t
+  character :: c(2) = [character(-1) :: 'a', 'b']
+   end type
+end

Re: [PATCH] Fortran - character type names in errors and warning - for review

[Mark Eggleston]

On 13/09/2019 07:54, Janne Blomqvist wrote:

On Mon, Sep 9, 2019 at 4:52 PM Mark Eggleston
 wrote:

To work around these problems I added a new length field to gfc_typespec
to used to produce the name of a character type if the character length
structure is not present.
The addition of the length field is a bit of kludge any pointers
regarding a better solution will be appreciated.

Thanks for the patch, I agree that we should print character type
names better. However, I'm not really happy with this approach.
Requiring us to keep track of the character length in two places
sounds like a recipe for confusing bugs. I don't really have a good
solution thought out for this, but I think this should be solved
somehow before committing the patch.
I agree, it was an east fix to get it to work.  The issues with existing 
location for character length should investigated further and a better 
solution found.


Secondly, character lengths can be longer than what fits into int. In
gfortran.h you'll find

typedef HOST_WIDE_INT gfc_charlen_t;

and then you should use gfc_mpz_get_hwi() instead of mpz_get_si(). And
for the *printf() format string you should use
HOST_WIDE_INT_PRINT_DEC.

Acknowledged.


Thanks,


Will submit an updated patch when fixed.

regards,

Mark

--
https://www.codethink.co.uk/privacy.html

Re: [Contrib PATCH] Add scripts to convert GCC repo from SVN to Git

[Maxim Kuvyrkov]

> On Aug 24, 2019, at 12:30 AM, Joseph Myers  wrote:
> 
> On Fri, 23 Aug 2019, Maxim Kuvyrkov wrote:
> 
>> I propose that we switch to gcc-pretty.git repository, because it has 
>> accurate Committer and Author fields.  Developer names and email 
>> addresses are extracted from source history, and accurately track people 
>> changing companies, email addresses, and names.  IMO, it is more 
>> important for people to get credit for open-source contributions on 
>> github, ohloh, etc., than the inconvenience of rebasing local git 
>> branches.  It's also an important marketing tool for open-source 
>> companies to show stats of their corporate email addresses appearing in 
>> git commit logs.
> 
> I concur that accurately crediting contributors is important and means we 
> should not start from the existing mirror (though we should keep its 
> branches available, so references to them and to their commit hashes 
> continue to work - either keeping the existing repository available under 
> a different name, or renaming the branches to put them in the new 
> repository - which should not enlarge the repository much because blob and 
> tree objects will generally be shared between the two versions of the 
> history).
> 
> I note that the Go conversion of reposurgeon is now just five test 
> failures away from passing the whole reposurgeon testsuite (at which point 
> it should be ready for an attempt on the GCC conversion).  Given the good 
> progress being made there at present, I thus suggest we plan to compare 
> this conversion with one from reposurgeon (paying special attention to the 
> messiest parts of the repository, such as artifacts from cvs2svn 
> attempting to locate branchpoints), unless those last five goreposurgeon 
> test failures prove unexpectedly time-consuming to get resolved.

Could you upload GCC repo converted with reposurgeon somewhere public?  And 
also list expected artifacts in its current version?

>From my side, the machine on which the conversion ran ran out of disk space 
>about 3 weeks ago.  I'll clean it up and restart the conversion updates.

I'll also improve author entries a bit, so gcc-pretty.git's history will change 
ever so slightly.

> 
> There are of course plenty of things to do relating to a git conversion 
> that do not depend on the particular choice of a converted repository - 
> such as writing git hooks and git versions of the maintainer-scripts 
> scripts that currently work with SVN, or working out a specific choice of 
> how to arrange annotated tags to allow "git describe" to give the sort of 
> monotonic version number some contributors want.
> 
> A reasonable starting point for hooks would be that they closely 
> approximate what the current SVN hooks do for commit mails to gcc-cvs and 
> for Bugzilla updates, as what the current hooks do is clearly OK at 
> present and we shouldn't need to entangle substantive changes to what the 
> hooks do with the actual conversion to git; we can always discuss changes 
> later.

Would the community please assign a volunteer for this at Cauldron? :-P

Thank you,

--
Maxim Kuvyrkov
www.linaro.org

Re: [PATCH] Fortran - character type names in errors and warning - for review

[Janne Blomqvist]

On Mon, Sep 9, 2019 at 4:52 PM Mark Eggleston
 wrote:
> To work around these problems I added a new length field to gfc_typespec
> to used to produce the name of a character type if the character length
> structure is not present.

> The addition of the length field is a bit of kludge any pointers
> regarding a better solution will be appreciated.

Thanks for the patch, I agree that we should print character type
names better. However, I'm not really happy with this approach.
Requiring us to keep track of the character length in two places
sounds like a recipe for confusing bugs. I don't really have a good
solution thought out for this, but I think this should be solved
somehow before committing the patch.

Secondly, character lengths can be longer than what fits into int. In
gfortran.h you'll find

typedef HOST_WIDE_INT gfc_charlen_t;

and then you should use gfc_mpz_get_hwi() instead of mpz_get_si(). And
for the *printf() format string you should use
HOST_WIDE_INT_PRINT_DEC.

Thanks,
-- 
Janne Blomqvist