Re: [PATCH][GCC][DOCS] Remove rtl.texi references to old RTX code class names

[Matthew Malcomson]

On 15/08/18 20:26, Sandra Loosemore wrote:


Use @item for the first item in a group, @itemx for all the others.

-Sandra


Thanks for the spot, updated patch attached.

diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi
index a37d9ac538991c507985bda4f825023534321b76..f406a99ed78b6ce1fd24e768eefb0e530d7977d6 100644
--- a/gcc/doc/rtl.texi
+++ b/gcc/doc/rtl.texi
@@ -144,7 +144,8 @@ Currently, @file{rtl.def} defines these classes:
 An RTX code that represents an actual object, such as a register
 (@code{REG}) or a memory location (@code{MEM}, @code{SYMBOL_REF}).
 @code{LO_SUM}) is also included; instead, @code{SUBREG} and
-@code{STRICT_LOW_PART} are not in this class, but in class @code{x}.
+@code{STRICT_LOW_PART} are not in this class, but in class
+@code{RTX_EXTRA}.
 
 @item RTX_CONST_OBJ
 An RTX code that represents a constant object.  @code{HIGH} is also
@@ -166,7 +167,7 @@ An RTX code for a unary arithmetic operation, such as @code{NEG},
 @item RTX_COMM_ARITH
 An RTX code for a commutative binary operation, such as @code{PLUS} or
 @code{AND}.  @code{NE} and @code{EQ} are comparisons, so they have class
-@code{<}.
+@code{RTX_COMM_COMPARE}.
 
 @item RTX_BIN_ARITH
 An RTX code for a non-commutative binary operation, such as @code{MINUS},
@@ -284,26 +285,28 @@ Some classes of RTX codes always have the same format.  For example, it
 is safe to assume that all comparison operations have format @code{ee}.
 
 @table @code
-@item 1
+@item RTX_UNARY
 All codes of this class have format @code{e}.
 
-@item <
-@itemx c
-@itemx 2
+@item RTX_BIN_ARITH
+@itemx RTX_COMM_ARITH
+@itemx RTX_COMM_COMPARE
+@itemx RTX_COMPARE
 All codes of these classes have format @code{ee}.
 
-@item b
-@itemx 3
+@item RTX_BITFIELD_OPS
+@itemx RTX_TERNARY
 All codes of these classes have format @code{eee}.
 
-@item i
+@item RTX_INSN
 All codes of this class have formats that begin with @code{iuueiee}.
 @xref{Insns}.  Note that not all RTL objects linked onto an insn chain
-are of class @code{i}.
+are of class @code{RTX_INSN}.
 
-@item o
-@itemx m
-@itemx x
+@item RTX_CONST_OBJ
+@itemx RTX_OBJ
+@itemx RTX_MATCH
+@itemx RTX_EXTRA
 You can make no assumptions about the format of these codes.
 @end table
 

[GCC][PATCH][Aarch64] Make common aarch64 options target-dependent

[Sam Tebbs]

Hi all,

This patch replaces the "Common" attribute in some aarch64 options with "Target"
to make them target-dependent. This doesn't affect their behaviour or how they
are documented with "--help", but does mean that they follow the option
specification that target-specific options are given the "Target" attribute.

Tested by ensuring the changed options are still accepted as before and that
they aren't invokable from an invalid target. Bootstrapped and regression tested
on aarch64-none-elf with ... regressions.

Is this ok for trunk? If so could someone commit it on my behalf? I don't have
commit rights.

gcc/
2018-08-16  Sam Tebbs

* config/aarch64/aarch64.opt (mlow-precision-recip-sqrt,
mlow-precision-sqrt, mlow-precision-div, mverbose-cost-dump): Replace
"Common" with "Target".

diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index bc9b22a4464c1e82b9183410c088e6757721e79b..c8e820422245496eaf09dd6546472e1ff9e90d4f 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -167,20 +167,20 @@ EnumValue
 Enum(aarch64_ra_sign_scope_t) String(all) Value(AARCH64_FUNCTION_ALL)
 
 mlow-precision-recip-sqrt
-Common Var(flag_mrecip_low_precision_sqrt) Optimization
+Target Var(flag_mrecip_low_precision_sqrt) Optimization
 Enable the reciprocal square root approximation.  Enabling this reduces
 precision of reciprocal square root results to about 16 bits for
 single precision and to 32 bits for double precision.
 
 mlow-precision-sqrt
-Common Var(flag_mlow_precision_sqrt) Optimization
+Target Var(flag_mlow_precision_sqrt) Optimization
 Enable the square root approximation.  Enabling this reduces
 precision of square root results to about 16 bits for
 single precision and to 32 bits for double precision.
 If enabled, it implies -mlow-precision-recip-sqrt.
 
 mlow-precision-div
-Common Var(flag_mlow_precision_div) Optimization
+Target Var(flag_mlow_precision_div) Optimization
 Enable the division approximation.  Enabling this reduces
 precision of division results to about 16 bits for
 single precision and to 32 bits for double precision.
@@ -212,7 +212,7 @@ Target RejectNegative Joined Enum(sve_vector_bits) Var(aarch64_sve_vector_bits)
 -msve-vector-bits=N	Set the number of bits in an SVE vector register to N.
 
 mverbose-cost-dump
-Common Undocumented Var(flag_aarch64_verbose_cost)
+Target Undocumented Var(flag_aarch64_verbose_cost)
 Enables verbose cost model dumping in the debug dump files.
 
 mtrack-speculation

Re: [PATCH] Merge Ignore and Deprecated in .opt files.

[Martin Liška]

On 08/15/2018 06:38 PM, Joseph Myers wrote:
> On Wed, 15 Aug 2018, Martin Liška wrote:
> 
>> Ok, so you have very similar opinion as Jakub. Thus I'm sending new 
>> version that preserves status quo, it only does:
> 
> This is removing RejectNegative from some Deprecated options.  Won't that 
> result in the -fno-* variants of those options starting to be accepted, 
> when they never were accepted when the positive versions of the options 
> did something useful?
> 

That's not intended, I fixed that. It the patch acceptable in form in which it 
is?

Thanks,
Martin

>From 0a7d5cd6cd6ca0586a350b95cd8f6ded095ba9c8 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Wed, 18 Jul 2018 13:40:24 +0200
Subject: [PATCH] Merge Ignore and Deprecated in .opt files.

gcc/ChangeLog:

2018-07-18  Martin Liska  

	* common.opt: Remove Warn, Init and Report for options with
Ignore/Deprecated flag. Warning is done automatically for
Deprecated flags.
	* config/i386/i386.opt: Likewise.
	* config/ia64/ia64.opt: Likewise.
	* config/rs6000/rs6000.opt: Likewise.
	* cppbuiltin.c (define_builtin_macros_for_compilation_flags):
Remove usage of flag_check_pointer_bounds.
	* lto-wrapper.c (merge_and_complain): Do not handle
OPT_fcheck_pointer_bounds.
	(append_compiler_options): Likewise.
	* opt-functions.awk: Do not handle Deprecated.
	* optc-gen.awk: Check that Var, Report and Init are not
used for an option with Ignore/Deprecated flag.
	* opts-common.c (decode_cmdline_option): Do not report
CL_ERR_DEPRECATED.
	(read_cmdline_option): Report warning for OPT_SPECIAL_deprecated
options.
	* opts.h (struct cl_option): Remove cl_deprecated flag.
	(CL_ERR_DEPRECATED): Remove error enum value.

gcc/testsuite/ChangeLog:

2018-07-18  Martin Liska  

	* g++.dg/opt/mpx.C: Fix scanned pattern.
	* gcc.target/i386/mpx.c: Likewise.
	* g++.dg/warn/Wunreachable-code-1.C: Remove.
	* g++.dg/warn/Wunreachable-code-2.C: Likewise.
	* gcc.dg/torture/pr52969.c: Likewise.
	* g++.dg/warn/pr31246-2.C: Likewise.
	* g++.dg/warn/pr31246.C: Likewise.
	* gcc.dg/pr33092.c: Likewise.
	* g++.dg/opt/eh1.C: Remove a deprecated option.
	* g++.dg/template/inline1.C: Likewise.
	* g++.dg/tree-ssa/pr81408.C: Likewise.
	* gcc.dg/pr41837.c: Likewise.
	* gcc.dg/pr41841.c: Likewise.
	* gcc.dg/pr42250.c: Likewise.
	* gcc.dg/pr43084.c: Likewise.
	* gcc.dg/pr43317.c: Likewise.
	* gcc.dg/pr51879-18.c: Likewise.
	* gcc.dg/torture/pr36066.c: Likewise.
	* gcc.dg/tree-ssa/ifc-8.c: Likewise.
	* gcc.dg/tree-ssa/ifc-cd.c: Likewise.
	* gcc.dg/tree-ssa/pr19210-1.c: Likewise.
	* gcc.dg/tree-ssa/pr45122.c: Likewise.
	* gcc.target/i386/pr45352-2.c: Likewise.
	* gcc.target/i386/zee.c: Likewise.
	* gfortran.dg/auto_char_len_2.f90: Likewise.
	* gfortran.dg/auto_char_len_4.f90: Likewise.
	* gfortran.dg/c_ptr_tests_15.f90: Likewise.
	* gfortran.dg/char_array_structure_constructor.f90: Likewise.
	* gfortran.dg/gomp/pr47331.f90: Likewise.
	* gfortran.dg/pr40999.f: Likewise.
	* gfortran.dg/pr41011.f: Likewise.
	* gfortran.dg/pr42051.f03: Likewise.
	* gfortran.dg/pr46804.f90: Likewise.
	* gfortran.dg/pr83149_1.f90: Likewise.
	* gfortran.dg/pr83149_b.f90: Likewise.
	* gfortran.dg/whole_file_1.f90: Likewise.
	* gfortran.dg/whole_file_10.f90: Likewise.
	* gfortran.dg/whole_file_11.f90: Likewise.
	* gfortran.dg/whole_file_12.f90: Likewise.
	* gfortran.dg/whole_file_13.f90: Likewise.
	* gfortran.dg/whole_file_14.f90: Likewise.
	* gfortran.dg/whole_file_15.f90: Likewise.
	* gfortran.dg/whole_file_16.f90: Likewise.
	* gfortran.dg/whole_file_17.f90: Likewise.
	* gfortran.dg/whole_file_18.f90: Likewise.
	* gfortran.dg/whole_file_19.f90: Likewise.
	* gfortran.dg/whole_file_2.f90: Likewise.
	* gfortran.dg/whole_file_20.f03: Likewise.
	* gfortran.dg/whole_file_3.f90: Likewise.
	* gfortran.dg/whole_file_4.f90: Likewise.
	* gfortran.dg/whole_file_5.f90: Likewise.
	* gfortran.dg/whole_file_6.f90: Likewise.
	* gfortran.dg/whole_file_7.f90: Likewise.
	* gfortran.dg/whole_file_8.f90: Likewise.
	* gfortran.dg/whole_file_9.f90: Likewise.
	* gcc.dg/vect/vect.exp: Likewise.

gcc/c-family/ChangeLog:

2018-07-18  Martin Liska  

	* c.opt: Remove Warn, Init and Report for options with
Ignore/Deprecated flag. Warning is done automatically for
Deprecated flags.
---
 gcc/c-family/c.opt| 92 +--
 gcc/common.opt| 10 +-
 gcc/config/i386/i386.opt  |  2 +-
 gcc/config/ia64/ia64.opt  |  4 +-
 gcc/config/rs6000/rs6000.opt  |  5 +-
 gcc/cppbuiltin.c  |  3 -
 gcc/dwarf2out.c   |  1 +
 gcc/lto-opts.c|  1 +
 gcc/lto-wrapper.c |  6 +-
 gcc/opt-functions.awk |  1 -
 gcc/optc-gen.awk  | 20 +++-
 gcc/opth-gen.awk  |  1 +
 gcc/opts-common.c

Re: [PATCH v2 2/4] libgcc: add crt{begin,end} for powerpc-wrs-vxworks target

[Rasmus Villemoes]

On 2018-06-28 10:43, Rasmus Villemoes wrote:
> In order to allow ZCX on VxWorks, we need the frame_dummy function to do
> the register_frame_info(). So make sure crtbegin.o and crtend.o are
> available for use with a custom spec file.

Hi Olivier

Can I also have your explicit ok for patch 2/4 (I'll fix the changelog
as for the other patches)?

Thanks,
Rasmus

Re: [GCC][PATCH][Aarch64] Make common aarch64 options target-dependent

[Richard Earnshaw (lists)]

On 16/08/18 09:45, Sam Tebbs wrote:
> Hi all,
> 
> This patch replaces the "Common" attribute in some aarch64 options with
> "Target"
> to make them target-dependent. This doesn't affect their behaviour or
> how they
> are documented with "--help", but does mean that they follow the option
> specification that target-specific options are given the "Target"
> attribute.
> 
> Tested by ensuring the changed options are still accepted as before and
> that
> they aren't invokable from an invalid target. Bootstrapped and
> regression tested
> on aarch64-none-elf with ... regressions.
> 
> Is this ok for trunk? If so could someone commit it on my behalf? I
> don't have
> commit rights.
> 
> gcc/
> 2018-08-16  Sam Tebbs

Two spaces between your name and email address.

> 
>     * config/aarch64/aarch64.opt (mlow-precision-recip-sqrt,
>     mlow-precision-sqrt, mlow-precision-div, mverbose-cost-dump):
> Replace

If you split a function list across a line break, end each line with a
close parenthesis and start the next with an open-parenthesis.  So:

 * config/aarch64/aarch64.opt (mlow-precision-recip-sqrt)
 (mlow-precision-sqrt, mlow-precision-div, mverbose-cost-dump):

>     "Common" with "Target".
> 

I've fixed this up and applied.

R.

PS, if you can use hard tabs for indenting your ChangeLog text it will
make my life a lot easier, as I don't have to fix it up manually.


> 
> latest.patch
> 
> 
> diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
> index 
> bc9b22a4464c1e82b9183410c088e6757721e79b..c8e820422245496eaf09dd6546472e1ff9e90d4f
>  100644
> --- a/gcc/config/aarch64/aarch64.opt
> +++ b/gcc/config/aarch64/aarch64.opt
> @@ -167,20 +167,20 @@ EnumValue
>  Enum(aarch64_ra_sign_scope_t) String(all) Value(AARCH64_FUNCTION_ALL)
>  
>  mlow-precision-recip-sqrt
> -Common Var(flag_mrecip_low_precision_sqrt) Optimization
> +Target Var(flag_mrecip_low_precision_sqrt) Optimization
>  Enable the reciprocal square root approximation.  Enabling this reduces
>  precision of reciprocal square root results to about 16 bits for
>  single precision and to 32 bits for double precision.
>  
>  mlow-precision-sqrt
> -Common Var(flag_mlow_precision_sqrt) Optimization
> +Target Var(flag_mlow_precision_sqrt) Optimization
>  Enable the square root approximation.  Enabling this reduces
>  precision of square root results to about 16 bits for
>  single precision and to 32 bits for double precision.
>  If enabled, it implies -mlow-precision-recip-sqrt.
>  
>  mlow-precision-div
> -Common Var(flag_mlow_precision_div) Optimization
> +Target Var(flag_mlow_precision_div) Optimization
>  Enable the division approximation.  Enabling this reduces
>  precision of division results to about 16 bits for
>  single precision and to 32 bits for double precision.
> @@ -212,7 +212,7 @@ Target RejectNegative Joined Enum(sve_vector_bits) 
> Var(aarch64_sve_vector_bits)
>  -msve-vector-bits=N  Set the number of bits in an SVE vector register to N.
>  
>  mverbose-cost-dump
> -Common Undocumented Var(flag_aarch64_verbose_cost)
> +Target Undocumented Var(flag_aarch64_verbose_cost)
>  Enables verbose cost model dumping in the debug dump files.
>  
>  mtrack-speculation
> 

Re: GCC 8 backports

[Martin Liška]

Hi.

Another 2 patches that I tested.

Martin
>From 733fad0936b46794f9e1ba6742471bd9f351a4c9 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Thu, 2 Aug 2018 10:17:34 +
Subject: Backport r263248

gcc/ChangeLog:

2018-08-02  Martin Liska  

PR gcov-profile/86817
	* gcov.c (process_all_functions): New function.
	(main): Call it.
	(process_file): Move functions processing to
process_all_functions.

---
diff --git a/gcc/gcov.c b/gcc/gcov.c
index 78a3e0e19e9..43dfc9a4b2c 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -543,6 +543,7 @@ static int process_args (int, char **);
 static void print_usage (int) ATTRIBUTE_NORETURN;
 static void print_version (void) ATTRIBUTE_NORETURN;
 static void process_file (const char *);
+static void process_all_functions (void);
 static void generate_results (const char *);
 static void create_file_names (const char *);
 static char *canonicalize_name (const char *);
@@ -798,6 +799,7 @@ main (int argc, char **argv)

   if (flag_intermediate_format || argno == argc - 1)
 	{
+	  process_all_functions ();
 	  generate_results (argv[argno]);
 	  release_structures ();
 	}
@@ -1145,11 +1147,14 @@ process_file (const char *file_name)
 {
   create_file_names (file_name);
   read_graph_file ();
-  if (functions.empty ())
-return;
-
   read_count_file ();
+}

+/* Process all functions in all files.  */
+
+static void
+process_all_functions (void)
+{
   hash_map fn_map;

   /* Identify group functions.  */
@@ -1226,7 +1231,6 @@ process_file (const char *file_name)
 	  if (fn->is_group)
 	fn->lines.resize (fn->end_line - fn->start_line + 1);

-
 	  solve_flow_graph (fn);
 	  if (fn->has_catch)
 	find_exception_blocks (fn);
--
2.18.0
>From 1a1934d31636a8bd72a6d93143dbe216ff6e6ad4 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Thu, 2 Aug 2018 09:32:58 +
Subject: Backport r263246

libsanitizer/ChangeLog:

2018-08-02  Martin Liska  

PR sanitizer/86022
	* sanitizer_common/sanitizer_linux_libcdep.cc (ThreadDescriptorSize):
Cherry-pick compiler-rt revision 338606.

---
diff --git a/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc b/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc
index d27a8435802..3b1a2174c46 100644
--- a/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc
+++ b/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc
@@ -235,7 +235,7 @@ uptr ThreadDescriptorSize() {
 val = FIRST_32_SECOND_64(1168, 1776);
   else if (minor == 11 || (minor == 12 && patch == 1))
 val = FIRST_32_SECOND_64(1168, 2288);
-  else if (minor <= 13)
+  else if (minor <= 14)
 val = FIRST_32_SECOND_64(1168, 2304);
   else
 val = FIRST_32_SECOND_64(1216, 2304);
--
2.18.0

Re: GCC 7 backport

[Martin Liška]

Hi.

I'm going to install one more patch.

Martin
>From 1a1934d31636a8bd72a6d93143dbe216ff6e6ad4 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Thu, 2 Aug 2018 09:32:58 +
Subject: Backport r263246

libsanitizer/ChangeLog:

2018-08-02  Martin Liska  

PR sanitizer/86022
	* sanitizer_common/sanitizer_linux_libcdep.cc (ThreadDescriptorSize):
Cherry-pick compiler-rt revision 338606.

---
diff --git a/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc b/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc
index d27a8435802..3b1a2174c46 100644
--- a/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc
+++ b/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc
@@ -235,7 +235,7 @@ uptr ThreadDescriptorSize() {
 val = FIRST_32_SECOND_64(1168, 1776);
   else if (minor == 11 || (minor == 12 && patch == 1))
 val = FIRST_32_SECOND_64(1168, 2288);
-  else if (minor <= 13)
+  else if (minor <= 14)
 val = FIRST_32_SECOND_64(1168, 2304);
   else
 val = FIRST_32_SECOND_64(1216, 2304);
--
2.18.0

Re: [GCC][PATCH][Aarch64] Make common aarch64 options target-dependent

[Sam Tebbs]

On 08/16/2018 10:46 AM, Richard Earnshaw (lists) wrote:

I've fixed this up and applied.

R.

PS, if you can use hard tabs for indenting your ChangeLog text it will
make my life a lot easier, as I don't have to fix it up manually.

Thanks for the tips and for committing.

Sam

Re: [RFC][PATCH][mid-end] Optimize immediate choice in comparisons.

[Richard Sandiford]

Vlad Lazar  writes:
> On 13/08/18 15:00, Richard Sandiford wrote:
>> Vlad Lazar  writes:
>>> diff --git a/gcc/expmed.h b/gcc/expmed.h
>>> index 
>>> 2890d9c9bbd034f01030dd551d544bf73e73b784..86a32a643fdd0fc9f396bd2c7904244bd484df16
>>>  100644
>>> --- a/gcc/expmed.h
>>> +++ b/gcc/expmed.h
>>> @@ -702,6 +702,10 @@ extern rtx emit_store_flag (rtx, enum rtx_code, rtx, 
>>> rtx, machine_mode,
>>>extern rtx emit_store_flag_force (rtx, enum rtx_code, rtx, rtx,
>>>   machine_mode, int, int);
>>>
>>> +extern void canonicalize_comparison (machine_mode, enum rtx_code *, rtx *);
>>> +
>>> +extern enum rtx_code canonicalized_cmp_code (enum rtx_code);
>> 
>> It would probably be better to make the second function static (local
>> to expmed.c), since it's only used internally by canonicalize_comparison.
>> Switching the order of the two functions in expmed.c would avoid the need
>> for a forward declaration.
>> 
>>> @@ -6153,6 +6156,97 @@ emit_store_flag_force (rtx target, enum rtx_code 
>>> code, rtx op0, rtx op1,
>>>
>>>  return target;
>>>}
>>> +
>>> +/* Choose the more appropiate immediate in comparisons.  The purpose of 
>>> this
>> 
>> Maybe worth saying "scalar integer comparisons", since the function
>> doesn't handle vector or floating-point comparisons.
>> 
>>> + is to end up with an immediate which can be loaded into a register
>>> in fewer
>>> +   moves, if possible.
>>> +
>>> +   For each integer comparison there exists an equivalent choice:
>>> + i)   a >  b or a >= b + 1
>>> + ii)  a <= b or a <  b + 1
>>> + iii) a >= b or a >  b - 1
>>> + iv)  a <  b or a <= b - 1
>>> +
>>> +   The function is called in the gimple expanders which handle 
>>> GIMPLE_ASSIGN
>>> + and GIMPLE_COND.  It assumes that the first operand of the
>>> comparison is a
>>> +   register and the second is a constant.
>> 
>> This last paragraph doesn't seem accurate any more.  Probably best to
>> drop it, since there's no real need to say who the callers are if we
>> describe the interface.
>> 
>>> +   mode is the mode of the first operand.
>>> +   code points to the comparison code.
>>> +   imm points to the rtx containing the immediate.  */
>> 
>> GCC's convention is to put parameter names in caps in comments,
>> so "MODE is...", etc.
>> 
>> On the IMM line, it might be worth adding "*IMM must satisfy
>> CONST_SCALAR_INT_P on entry and continues to satisfy CONST_SCALAR_INT_P
>> on exit."
>> 
>>> +void canonicalize_comparison (machine_mode mode, enum rtx_code
>>> *code, rtx *imm)
>>> +{
>>> +  int to_add = 0;
>>> +
>>> +  if (!SCALAR_INT_MODE_P (mode))
>>> +return;
>>> +
>>> +  /* Extract the immediate value from the rtx.  */
>>> +  wide_int imm_val = wi::shwi (INTVAL (*imm), mode);
>> 
>> This should be:
>> 
>>rtx_mode_t imm_val (*imm, mode);
>> 
>> so that it copes with all CONST_SCALAR_INT_P, not just CONST_INT.
>> 
>>> +  if (*code == GT || *code == GTU || *code == LE || *code == LEU)
>>> +to_add = 1;
>>> +
>>> +  if (*code == GE || *code == GEU || *code == LT || *code == LTU)
>>> +to_add = -1;
>> 
>> Might be better to have:
>> 
>>if (*code == GT || *code == GTU || *code == LE || *code == LEU)
>>  to_add = 1;
>>else if (*code == GE || *code == GEU || *code == LT || *code == LTU)
>>  to_add = -1;
>>else
>>  return;
>> 
>> so that we exit early for other comparisons, rather than doing wasted work.
>> 
>>> +  /* Check for overflow/underflow in the case of signed values and
>>> + wrapping around in the case of unsigned values.  If any occur
>>> + cancel the optimization.  */
>>> +  wide_int max_val = wi::max_value (mode, SIGNED);
>>> +  wide_int min_val = wi::min_value (mode, SIGNED);
>>> +  if ((wi::cmps (imm_val, max_val) == 0 && to_add == 1)
>>> +  || (wi::cmps (imm_val, min_val) == 0 && to_add == -1))
>>> +return;
>> 
>> This needs to use the SIGNED/UNSIGNED choice appropriate for the
>> comparison (SIGNED for GT, UNSIGNED for GTU, etc.).  There doesn't
>> seem to be an existing function that says whether an rtx comparison
>> operation is signed or not (bit of a surprise), but there is
>> unsigned_condition, which returns the unsigned form of an rtx
>> comparison operator.  It might be worth adding something like:
>> 
>>/* Return true if integer comparison operator CODE interprets its operands
>>   as unsigned.  */
>> 
>>inline bool
>>unsigned_condition_p (rtx_code code)
>>{
>>  return unsigned_condition (code) == code;
>>}
>> 
>> and using that to choose between SIGNED and UNSIGNED.
>> 
>> Rather than using wi::cmp, a more direct way of checking for overflow
>> is to change this:
>> 
>>> +  /* Generate a mov instruction for both cases and see whether the change
>>> + was profitable.  */
>>> +  wide_int imm_modif = wi::add (imm_val, to_add);
>> 
>> to use the overflow form of wi::add, i.e.:
>> 
>>wide_int imm_modif = wi::add (imm_val, to_add, sign, &overflow);
>> 
>> and return 

Re: [2/2] Add AddressSanitizer annotations to std::string.

[Mikhail Kashkarov]

^^ gentle ping.

On 07/16/2018 07:16 PM, Mikhail Kashkarov wrote:
> Rebased and update patch (typos, add missing annotations),
> add ASan teststo verify string annotation.
>
>
> On 06/28/2018 11:09 AM, Mikhail Kashkarov wrote:
>> ^ gentle ping.
>>
>>
>> On 06/08/2018 05:54 PM, Mikhail Kashkarov wrote:
>>> Hello,
>>>
>>> I've updated patches for std::string sanitization and disabling CXX11
>>> string SSO usage for correct sanitization.
>>>
>>>    >>       _M_destroy(_M_allocated_capacity);
>>>    >>+    else
>>>    >>+      __annotate_delete();
>>>    >
>>>    >Do these calls definitely optimize away completely when not
>>>    >sanitizing? Even for -O1, -Os and -Og?
>>>    >
>>>    >For std::vector annotation I used macros to add these 
>>> annotations, so
>>>    >there is no change to the generated code when annotations are
>>>    >disabled. But it makes the code quite ugly.
>>>
>>> I've checked asm code for string-inst.o and it looks like this calls 
>>> are
>>> optimized away, but there are some light changes after patch fir .
>>>
>>>    > Right, I was wondering specifically about the 
>>>    > instantiations. I could be wrong but I don't think anything in
>>>    >  creates, destroys or modifies a std::basic_string.
>>>
>>> I was confused by reinterpret_cast's on strings in fstream.tcc, looks
>>> like this is not needed, you are right.
>>>
>>>    >>   // calling 4.0.x+ _S_create.
>>>    >>   __p->_M_set_sharable();
>>>    >>+#if _GLIBCXX_SANITIZER_ANNOTATE_STRING
>>>    >>+  __p->_M_length = 0;
>>>    >>+#endif
>>>    >
>>>    > Why is this needed? I think a comment explaining it would help 
>>> (like
>>>    > the one above explaining why calling _M_set_sharable() is needed).
>>>
>>> Checked current version without this change, looks like now it works,
>>> reverted.
>>>
>>> Short summary:
>>> The reason for changing strings layout under sanitization is to 
>>> place string
>>> char buffer on heap for correct aligning in 32-bit environments,
>>> both pre-CXX11 and CXX11 strings ABI.
>>>
>>> | Sanitize string | string type | ABI is changed? | 32-bit | 64-bit |
>>> |-+-+-++|
>>> | FULL    | SSO-string  | yes | +  | +  |
>>> | | COW-string  | yes | +  | +  |
>>> |-+-+-++|
>>> | PARTIAL | SSO-string  | no  | -+(*)  | +  |
>>> | | COW-string  | no  | -  | +  |
>>> *only longs strings are sanitized for 32-bit
>>>
>>> Some functions with new define looks a bit messy without changing 
>>> internal
>>> functions(operator=), I'm also not sure if disabling string SSO 
>>> usage define
>>> should also affects other parts that relies on basic_string class size
>>> (checks
>>> with static_assert in exceptions/shim-facets).
>>>
>>>
>>> Any thoughts?
>>>
>>> On 05/29/2018 06:55 PM, Jonathan Wakely wrote:
 On 29/05/18 18:18 +0300, Kashkarov Mikhail wrote:
> Jonathan Wakely  writes:
>>> --- a/libstdc++-v3/include/bits/fstream.tcc
>>> +++ b/libstdc++-v3/include/bits/fstream.tcc
>>> @@ -1081,6 +1081,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>
>>>     // Inhibit implicit instantiations for required instantiations,
>>>     // which are defined via explicit instantiations elsewhere.
>>> +#if !_GLIBCXX_SANITIZE_STRING
>>> #if _GLIBCXX_EXTERN_TEMPLATE
>>>     extern template class basic_filebuf;
>>>     extern template class basic_ifstream;
>>> @@ -1094,6 +1095,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>     extern template class basic_fstream;
>>> #endif
>>> #endif
>>> +#endif // !_GLIBCXX_SANITIZE_STRING
>> Why do we need to disable these explicit instantiation declarations?
>> Are they affected by the std::string layout changes? Is that just
>> because of the constructors taking std::string, or something else?
> Libstdc++ build is not sanitized, so macroses that requires
> AddressSanitizer support will not applied and these templates will be
> instantate without support for ASan annotations.
 Right, I was wondering specifically about the 
 instantiations. I could be wrong but I don't think anything in
  creates, destroys or modifies a std::basic_string.





>

-- 
Best regards,
Kashkarov Mikhail
Samsung R&D Institute Russia

[PATCH, Darwin, drivers] Split DWARF is not supported on Darwin.

[Iain Sandoe]

The Darwin toolchains have a separate debug linker (dsymutil) so that the
link-time penalty for debug data is not usually seen.  At present, it's not
clear how we would support split DWARF on Darwin (or if it would bring
any additional benefit over dsymutil).

This patch produces diagnostic output to note that it's not supported and
disables it.  We also skip test cases that invoke -gsplit-dwarf.

NOTE that the tests failed because of PR81685 and this fix will need to
be back-ported after the fix for 81685.

OK for trunk?
affected branches (7,8)?

thanks
Iain

gcc/

   config/darwin.c
   config/darwin.h

gcc/testsuite/

  g++.dg/debug/dwarf2/pr85302.C
  gcc.dg/lto/pr83719_0.c
  gcc.dg/pr86064.c
---
 gcc/config/darwin.c | 8 
 gcc/config/darwin.h | 5 -
 gcc/testsuite/g++.dg/debug/dwarf2/pr85302.C | 1 +
 gcc/testsuite/gcc.dg/lto/pr83719_0.c| 1 +
 gcc/testsuite/gcc.dg/pr86064.c  | 1 +
 5 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/gcc/config/darwin.c b/gcc/config/darwin.c
index db988850fd..41a1cef250 100644
--- a/gcc/config/darwin.c
+++ b/gcc/config/darwin.c
@@ -3190,6 +3190,14 @@ darwin_override_options (void)
   if (!global_options_set.x_dwarf_version)
 dwarf_version = 2;
 
+  if (global_options_set.x_dwarf_split_debug_info)
+{
+  inform (input_location,
+ "-gsplit-dwarf is not supported on this platform, ignored");
+  dwarf_split_debug_info = 0;
+  global_options_set.x_dwarf_split_debug_info = 0;
+}
+
   /* Do not allow unwind tables to be generated by default for m32.  
  fnon-call-exceptions will override this, regardless of what we do.  */
   if (generating_for_darwin_version < 10
diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index bec19b6488..f954ba3cab 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -122,7 +122,8 @@ extern GTY(()) int darwin_ms_struct;
   "%{gfull:-g -fno-eliminate-unused-debug-symbols} %

[PATCH, testsuite] Make pr80263 work for Darwin by suppressing pubnames/types.

[Iain Sandoe]

Darwin emits pubnames/types by default which masks the intended check.

OK for trunk?
Iain

 gcc/testsuite

* gcc.dg/debug/dwarf2/pr80263.c: Suppress pubtypes output for Darwin.


---
 gcc/testsuite/gcc.dg/debug/dwarf2/pr80263.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr80263.c 
b/gcc/testsuite/gcc.dg/debug/dwarf2/pr80263.c
index 57633b4f0e..f1a6a33616 100644
--- a/gcc/testsuite/gcc.dg/debug/dwarf2/pr80263.c
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr80263.c
@@ -1,6 +1,8 @@
 /* PR debug/80263 */
 /* { dg-do compile } */
 /* { dg-options "-g -dA" } */
+/* Darwin emits pubnames/types by default - suppress this for the test. */
+/* { dg-additional-options "-gno-pubnames" { target *-*-darwin* } }  */
 
 char array[1];
 
-- 
2.17.1

Re: [PATCH][GCC][DOCS] Remove rtl.texi references to old RTX code class names

[Sandra Loosemore]

On 08/16/2018 02:42 AM, Matthew Malcomson wrote:


On 15/08/18 20:26, Sandra Loosemore wrote:


Use @item for the first item in a group, @itemx for all the others.


Thanks for the spot, updated patch attached.


This version looks fine with me.  Nobody's made any objection about 
technical incorrectness, either, so I think it's OK to check in.


-Sandra

Re: [Patch wwwdocs] Document arm-8-branch

[Gerald Pfeifer]

On Wed, 15 Aug 2018, Ramana Radhakrishnan wrote:
> As $subject.
> 
> Ok ?

Yes, and you do not need anyone's approval. :-)

Though since you asked:

+  This branch provides bug fixes and minor enhancements for GCC when
+  used targeting the AArch64 and the Arm architecture. Most patches

Could this be just

  "...when targeting the AArch64 and Arm architectures..." ?

Gerald

Re: [PATCH][GCC][DOCS] Remove rtl.texi references to old RTX code class names

[Jeff Law]

On 08/16/2018 08:44 AM, Sandra Loosemore wrote:
> On 08/16/2018 02:42 AM, Matthew Malcomson wrote:
>>
>> On 15/08/18 20:26, Sandra Loosemore wrote:
>>>
>>> Use @item for the first item in a group, @itemx for all the others.
>>
>> Thanks for the spot, updated patch attached.
> 
> This version looks fine with me.  Nobody's made any objection about
> technical incorrectness, either, so I think it's OK to check in.
I think it's fine from the technical side as well.
jeff

Re: [PATCH, testsuite] Make pr80263 work for Darwin by suppressing pubnames/types.

[Jeff Law]

On 08/16/2018 08:35 AM, Iain Sandoe wrote:
> 
> Darwin emits pubnames/types by default which masks the intended check.
> 
> OK for trunk?
> Iain
> 
>  gcc/testsuite
> 
>   * gcc.dg/debug/dwarf2/pr80263.c: Suppress pubtypes output for Darwin.
OK
jeff

Re: [PATCH] convert braced initializers to strings (PR 71625)

[Martin Sebor]

On 08/15/2018 03:34 PM, Jeff Law wrote:

On 08/15/2018 03:02 PM, Martin Sebor wrote:

On 08/15/2018 06:07 AM, Joseph Myers wrote:

On Tue, 14 Aug 2018, Martin Sebor wrote:


This is with Bison 3.0.4, should the version used to produce
intl/plural.c
prove relevant.


Can you send me the translation unit and the options it was compiled
with that triggered the errors?


I've attached plural.i.  The error is a static link error linking sln,
but
maybe comparing results of compiling plural.i before and after the
changes
may be enlightening (unless it's actually a difference in code elsewhere
in glibc causing a link error reported in plural.o).

Compiled with:

alpha-glibc-linux-gnu-gcc
/scratch/jmyers/glibc/many9/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/intl/plural.c

-c -std=gnu11 -fgnu89-inline  -O2 -Wall -Werror -Wundef -Wwrite-strings
-fmerge-all-constants -fno-stack-protector -frounding-math -g
-Wstrict-prototypes -Wold-style-definition -fno-math-errno
-mlong-double-128 -mieee -mfp-rounding-mode=d
-ftls-model=initial-exec
-I../include
-I/scratch/jmyers/glibc/many9/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/intl

-I/scratch/jmyers/glibc/many9/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu

-I../sysdeps/unix/sysv/linux/alpha/alpha
-I../sysdeps/unix/sysv/linux/alpha/fpu  -I../sysdeps/alpha/fpu
-I../sysdeps/unix/sysv/linux/alpha  -I../sysdeps/alpha/nptl
-I../sysdeps/unix/sysv/linux/wordsize-64
-I../sysdeps/ieee754/ldbl-64-128
-I../sysdeps/ieee754/ldbl-opt  -I../sysdeps/unix/sysv/linux/include
-I../sysdeps/unix/sysv/linux  -I../sysdeps/nptl  -I../sysdeps/pthread
-I../sysdeps/gnu  -I../sysdeps/unix/inet  -I../sysdeps/unix/sysv
-I../sysdeps/unix/alpha  -I../sysdeps/unix  -I../sysdeps/posix
-I../sysdeps/alpha  -I../sysdeps/wordsize-64
-I../sysdeps/ieee754/ldbl-128  -I../sysdeps/ieee754/dbl-64/wordsize-64
-I../sysdeps/ieee754/dbl-64  -I../sysdeps/ieee754/flt-32
-I../sysdeps/ieee754  -I../sysdeps/generic  -I.. -I../libio -I.
-D_LIBC_REENTRANT -include
/scratch/jmyers/glibc/many9/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/libc-modules.h

-DMODULE_NAME=libc -include ../include/libc-symbols.h
-DTOP_NAMESPACE=glibc -D'LOCALEDIR="/usr/share/locale"'
-D'LOCALE_ALIAS_PATH="/usr/share/locale"' -o
/scratch/jmyers/glibc/many9/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/intl/plural.o

-MD -MP -MF
/scratch/jmyers/glibc/many9/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/intl/plural.o.dt

-MT
/scratch/jmyers/glibc/many9/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/intl/plural.o




Thanks.  I don't see anything obviously wrong but I don't know
much about Alpha assembly.  Attached are the two .s files, with
(plural-new.s) and without (plural-old.s) the array-to-string
transformation.

I'd focus on these  insns which correspond to the error from the linker:

They're in plural.o within the fucntion gettextparse


ldah $2,yypgoto+2305843009213693936($29)!gprelhigh

Something certainly doesn't look right...


Thanks.  I also get the same exact same assembly with both
forms of initializers for the two constant arrays, i.e.,

  static const yytype_int8 yypgoto[3] = "\366\366\377";
  static const yytype_int8 yydefgoto[3] = "\377\005\006";

and

  static const yytype_int8 yypgoto[] = { -10, -10, -1 };
  static const yytype_int8 yydefgoto[] = { -1, 5, 6 };

They end up in the .sdata section either way (the former
both with and without the GCC change as might be expected):

.section.sdata,"aws"
.type   yydefgoto, @object
.size   yydefgoto, 3
  yydefgoto:
.ascii  "\377\005\006"
.type   yypgoto, @object
.size   yypgoto, 3
  yypgoto:
.ascii  "\366\366\377"
.section.rodata

and also before the GCC change:

.section.sdata,"aws"
.type   yydefgoto, @object
.size   yydefgoto, 3
  yydefgoto:
.byte   -1
.byte   5
.byte   6
.type   yypgoto, @object
.size   yypgoto, 3
  yypgoto:
.byte   -10
.byte   -10
.byte   -1


At the .o level this turns into:
218:   00 00 5d 24 ldaht1,0(gp)
218: GPRELHIGH  .sdata+0x1ff3


300:   00 00 5d 24 ldaht1,0(gp)
300: GPRELHIGH  .sdata+0x1ff3

It's not really a matter of what the instruction does, but a matter of
the relocation.  You'd have to look at the definition of GPRELHIGH which
you can find in BFD.


The definitions match the assembly with both kinds of
initializers, and with the string literal also with GCC before
the change:

  0218 GPRELHIGH .sdata+0x1ff3
  0238 GPRELLOW  .sdata+0x1ff3
  0300 GPRELHIGH .sdata+0x1ff3
  0308 GPRELLOW  .sdata+0x1ff3
  0458 GPRELHIGH .sdata+0x0003
  0468 GPRELLOW  

Re: [PATCH] convert braced initializers to strings (PR 71625)

[Jeff Law]

On 08/16/2018 09:23 AM, Martin Sebor wrote:
> On 08/15/2018 03:34 PM, Jeff Law wrote:
>> On 08/15/2018 03:02 PM, Martin Sebor wrote:
>>> On 08/15/2018 06:07 AM, Joseph Myers wrote:
 On Tue, 14 Aug 2018, Martin Sebor wrote:

>> This is with Bison 3.0.4, should the version used to produce
>> intl/plural.c
>> prove relevant.
>
> Can you send me the translation unit and the options it was compiled
> with that triggered the errors?

 I've attached plural.i.  The error is a static link error linking sln,
 but
 maybe comparing results of compiling plural.i before and after the
 changes
 may be enlightening (unless it's actually a difference in code
 elsewhere
 in glibc causing a link error reported in plural.o).

 Compiled with:

 alpha-glibc-linux-gnu-gcc
 /scratch/jmyers/glibc/many9/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/intl/plural.c


 -c -std=gnu11 -fgnu89-inline  -O2 -Wall -Werror -Wundef -Wwrite-strings
 -fmerge-all-constants -fno-stack-protector -frounding-math -g
 -Wstrict-prototypes -Wold-style-definition -fno-math-errno
 -mlong-double-128 -mieee -mfp-rounding-mode=d
 -ftls-model=initial-exec
 -I../include
 -I/scratch/jmyers/glibc/many9/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/intl


 -I/scratch/jmyers/glibc/many9/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu


 -I../sysdeps/unix/sysv/linux/alpha/alpha
 -I../sysdeps/unix/sysv/linux/alpha/fpu  -I../sysdeps/alpha/fpu
 -I../sysdeps/unix/sysv/linux/alpha  -I../sysdeps/alpha/nptl
 -I../sysdeps/unix/sysv/linux/wordsize-64
 -I../sysdeps/ieee754/ldbl-64-128
 -I../sysdeps/ieee754/ldbl-opt  -I../sysdeps/unix/sysv/linux/include
 -I../sysdeps/unix/sysv/linux  -I../sysdeps/nptl  -I../sysdeps/pthread
 -I../sysdeps/gnu  -I../sysdeps/unix/inet  -I../sysdeps/unix/sysv
 -I../sysdeps/unix/alpha  -I../sysdeps/unix  -I../sysdeps/posix
 -I../sysdeps/alpha  -I../sysdeps/wordsize-64
 -I../sysdeps/ieee754/ldbl-128  -I../sysdeps/ieee754/dbl-64/wordsize-64
 -I../sysdeps/ieee754/dbl-64  -I../sysdeps/ieee754/flt-32
 -I../sysdeps/ieee754  -I../sysdeps/generic  -I.. -I../libio -I.
 -D_LIBC_REENTRANT -include
 /scratch/jmyers/glibc/many9/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/libc-modules.h


 -DMODULE_NAME=libc -include ../include/libc-symbols.h
 -DTOP_NAMESPACE=glibc -D'LOCALEDIR="/usr/share/locale"'
 -D'LOCALE_ALIAS_PATH="/usr/share/locale"' -o
 /scratch/jmyers/glibc/many9/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/intl/plural.o


 -MD -MP -MF
 /scratch/jmyers/glibc/many9/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/intl/plural.o.dt


 -MT
 /scratch/jmyers/glibc/many9/build/compilers/alpha-linux-gnu/glibc/alpha-linux-gnu/intl/plural.o



>>>
>>> Thanks.  I don't see anything obviously wrong but I don't know
>>> much about Alpha assembly.  Attached are the two .s files, with
>>> (plural-new.s) and without (plural-old.s) the array-to-string
>>> transformation.
>> I'd focus on these  insns which correspond to the error from the linker:
>>
>> They're in plural.o within the fucntion gettextparse
>>
>>
>>     ldah $2,yypgoto+2305843009213693936($29)   
>> !gprelhigh
>>
>> Something certainly doesn't look right...
> 
> Thanks.  I also get the same exact same assembly with both
> forms of initializers for the two constant arrays, i.e.,
> 
>   static const yytype_int8 yypgoto[3] = "\366\366\377";
>   static const yytype_int8 yydefgoto[3] = "\377\005\006";
> 
> and
> 
>   static const yytype_int8 yypgoto[] = { -10, -10, -1 };
>   static const yytype_int8 yydefgoto[] = { -1, 5, 6 };
> 
> They end up in the .sdata section either way (the former
> both with and without the GCC change as might be expected):
> 
>     .section    .sdata,"aws"
>     .type   yydefgoto, @object
>     .size   yydefgoto, 3
>   yydefgoto:
>     .ascii  "\377\005\006"
>     .type   yypgoto, @object
>     .size   yypgoto, 3
>   yypgoto:
>     .ascii  "\366\366\377"
>     .section    .rodata
> 
> and also before the GCC change:
> 
>     .section    .sdata,"aws"
>     .type   yydefgoto, @object
>     .size   yydefgoto, 3
>   yydefgoto:
>     .byte   -1
>     .byte   5
>     .byte   6
>     .type   yypgoto, @object
>     .size   yypgoto, 3
>   yypgoto:
>     .byte   -10
>     .byte   -10
>     .byte   -1
> 
>> At the .o level this turns into:
>> 218:   00 00 5d 24 ldah    t1,0(gp)
>>     218: GPRELHIGH  .sdata+0x1ff3
>>
>>
>> 300:   00 00 5d 24 ldah    t1,0(gp)
>>     300: GPRELHIGH  .sdata+0x1ff3
>>
>> It's not really a matter of what the instruction does, but a matter of
>> the relocation.  You'd have to look at the d

Re: [PATCH, OpenACC] Add support for gang local storage allocation in shared memory

[Julian Brown]

On Wed, 15 Aug 2018 21:56:54 +0200
Bernhard Reutner-Fischer  wrote:

> On 15 August 2018 18:46:37 CEST, Julian Brown
>  wrote:
> >On Mon, 13 Aug 2018 12:06:21 -0700
> >Cesar Philippidis  wrote:  
> 
> atttribute has more t than strictly necessary. 
> Don't like signed integer levels where they should be some unsigned. 
> Also don't like single switch cases instead of if.
> And omitting function comments even if the hook way above is
> documented may be ok ish but is a bit lazy ;)

Here's a new version with those comments addressed. I also changed the
logic around a little to avoid adding decls to the vec in omp_context
which would never be given the gang-private attribute.

Re-tested with offloading to NVPTX.

OK?

Julian

2018-08-10  Julian Brown  
Chung-Lin Tang  

gcc/
* config/nvptx/nvptx.c (tree-hash-traits.h): Include.
(gangprivate_shared_size): New global variable.
(gangprivate_shared_align): Likewise.
(gangprivate_shared_sym): Likewise.
(gangprivate_shared_hmap): Likewise.
(nvptx_option_override): Initialize gangprivate_shared_sym,
gangprivate_shared_align.
(nvptx_file_end): Output gangprivate_shared_sym.
(nvptx_goacc_expand_accel_var): New function.
(nvptx_set_current_function): New function.
(TARGET_SET_CURRENT_FUNCTION): Define hook.
(TARGET_GOACC_EXPAND_ACCEL): Likewise.
* doc/tm.texi (TARGET_GOACC_EXPAND_ACCEL_VAR): Document new hook.
* doc/tm.texi.in (TARGET_GOACC_EXPAND_ACCEL_VAR): Likewise.
* expr.c (expand_expr_real_1): Remap decls marked with the
"oacc gangprivate" attribute.
* omp-low.c (omp_context): Add oacc_partitioning_level and
oacc_addressable_var_decls fields.
(new_omp_context): Initialize oacc_addressable_var_decls in new
omp_context.
(delete_omp_context): Delete oacc_addressable_var_decls in old
omp_context.
(lower_oacc_head_tail): Record partitioning-level count in omp context.
(oacc_record_private_var_clauses, oacc_record_vars_in_bind)
(mark_oacc_gangprivate): New functions.
(lower_omp_for): Call oacc_record_private_var_clauses with "for"
clauses.  Call mark_oacc_gangprivate for gang-partitioned loops.
(lower_omp_target): Call oacc_record_private_var_clauses with "target"
clauses.
Call mark_oacc_gangprivate for offloaded target regions.
(lower_omp_1): Call vars_in_bind for GIMPLE_BIND within OMP regions.
* target.def (expand_accel_var): New hook.

libgomp/
* testsuite/libgomp.oacc-c-c++-common/gang-private-1.c: New test.
* testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c: New test.
* testsuite/libgomp.oacc-c/pr85465.c: New test.
* testsuite/libgomp.oacc-fortran/gangprivate-attrib-1.f90: New test.
commit e276442550a85b62866ba13890eacf4e946d1079
Author: Julian Brown 
Date:   Thu Aug 9 20:27:04 2018 -0700

[OpenACC] Add support for gang local storage allocation in shared memory

2018-08-10  Julian Brown  
	Chung-Lin Tang  

	gcc/
	* config/nvptx/nvptx.c (tree-hash-traits.h): Include.
	(gangprivate_shared_size): New global variable.
	(gangprivate_shared_align): Likewise.
	(gangprivate_shared_sym): Likewise.
	(gangprivate_shared_hmap): Likewise.
	(nvptx_option_override): Initialize gangprivate_shared_sym,
	gangprivate_shared_align.
	(nvptx_file_end): Output gangprivate_shared_sym.
	(nvptx_goacc_expand_accel_var): New function.
	(nvptx_set_current_function): New function.
	(TARGET_SET_CURRENT_FUNCTION): Define hook.
	(TARGET_GOACC_EXPAND_ACCEL): Likewise.
	* doc/tm.texi (TARGET_GOACC_EXPAND_ACCEL_VAR): Document new hook.
	* doc/tm.texi.in (TARGET_GOACC_EXPAND_ACCEL_VAR): Likewise.
	* expr.c (expand_expr_real_1): Remap decls marked with the
	"oacc gangprivate" attribute.
	* omp-low.c (omp_context): Add oacc_partitioning_level and
	oacc_addressable_var_decls fields.
	(new_omp_context): Initialize oacc_addressable_var_decls in new
	omp_context.
	(delete_omp_context): Delete oacc_addressable_var_decls in old
	omp_context.
	(lower_oacc_head_tail): Record partitioning-level count in omp context.
	(oacc_record_private_var_clauses, oacc_record_vars_in_bind)
	(mark_oacc_gangprivate): New functions.
	(lower_omp_for): Call oacc_record_private_var_clauses with "for"
	clauses.  Call mark_oacc_gangprivate for gang-partitioned loops.
	(lower_omp_target): Call oacc_record_private_var_clauses with "target"
	clauses.
	Call mark_oacc_gangprivate for offloaded target regions.
	(lower_omp_1): Call vars_in_bind for GIMPLE_BIND within OMP regions.
	* target.def (expand_accel_var): New hook.

	libgomp/
	* testsuite/libgomp.oacc-c-c++-common/gang-private-1.c: New test.
	* testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c: New tes

[PATCH, RFC, rs6000, v2] folding of vec_splat

[Will Schmidt]

Hi
  Enable GIMPLE folding of the vec_splat() intrinsic. (v2).

This uses the tree_vec_extract() function out of tree-vect-generic.c
to retrieve the splat value, which is a BIT_FIELD_REF.   That function is
made non-static as part of this change.

Testcases are already in-tree.

V2 updates, per feedback previously received.
Forced arg1 into range (modulo #elements) before attempting to extract
the splat value.
Removed the (now unnecessary) code that did bounds-checking before calling
the tree_vec_extract helper.
Used arg0_type rather than lhs_type for calculating the tree size.

OK for trunk?

Thanks,
-Will

[gcc]

2018-08-16  Will Schmidt  

* config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add support for
  early gimple folding of vec_splat().
* tree-vect-generic.c: Remove static from tree_vec_extract() definition.
* gimple-fold.h:  Add an extern define for tree_vec_extract().

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 97b922f..ec92e6a 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -15766,10 +15766,58 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
 tree splat_tree = build_vector_from_val (TREE_TYPE (lhs), splat_value);
 g = gimple_build_assign (lhs, splat_tree);
 gimple_set_location (g, gimple_location (stmt));
 gsi_replace (gsi, g, true);
 return true;
+   }
+
+/* Flavors of vec_splat.  */
+/* a = vec_splat (b, 0x3) becomes a = { b[3],b[3],b[3],...};  */
+case ALTIVEC_BUILTIN_VSPLTB:
+case ALTIVEC_BUILTIN_VSPLTH:
+case ALTIVEC_BUILTIN_VSPLTW:
+case VSX_BUILTIN_XXSPLTD_V2DI:
+case VSX_BUILTIN_XXSPLTD_V2DF:
+  {
+   arg0 = gimple_call_arg (stmt, 0); /* input vector.  */
+   arg1 = gimple_call_arg (stmt, 1); /* index into arg0.  */
+   /* Only fold the vec_splat_*() if arg1 is a constant
+  5-bit unsigned literal.  */
+   if (TREE_CODE (arg1) != INTEGER_CST || TREE_INT_CST_LOW (arg1) > 0x1f)
+ return false;
+   lhs = gimple_call_lhs (stmt);
+   tree lhs_type = TREE_TYPE (lhs);
+   tree arg0_type = TREE_TYPE (arg0);
+   tree arg1_type = TREE_TYPE (arg1);
+   int n_elts = VECTOR_CST_NELTS (arg0);
+   /* force arg1 into range.  */
+   tree new_arg1 = build_int_cst (arg1_type,
+  TREE_INT_CST_LOW (arg1) % n_elts);
+   tree splat;
+   if (TREE_CODE (arg0) == VECTOR_CST)
+ splat = VECTOR_CST_ELT (arg0, TREE_INT_CST_LOW (new_arg1));
+   else
+ {
+   /* Determine (in bits) the length and start location of the
+  splat value for a call to the tree_vec_extract helper.  */
+   int tree_size_in_bits = TREE_INT_CST_LOW (size_in_bytes (arg0_type))
+   * BITS_PER_UNIT;
+   int splat_elem_size = tree_size_in_bits / n_elts;
+   int splat_start_bit = TREE_INT_CST_LOW (new_arg1) * splat_elem_size;
+   /* Do not attempt to early-fold if the size + specified offset into
+  the vector would touch outside of the source vector.  */
+   tree len = build_int_cst (bitsizetype, splat_elem_size);
+   tree start = build_int_cst (bitsizetype, splat_start_bit);
+   splat = tree_vec_extract (gsi, TREE_TYPE (lhs_type), arg0,
+ len, start);
+   }
+   /* And finally, build the new vector.  */
+   tree splat_tree = build_vector_from_val (lhs_type, splat);
+   g = gimple_build_assign (lhs, splat_tree);
+   gimple_set_location (g, gimple_location (stmt));
+   gsi_replace (gsi, g, true);
+   return true;
   }
 
 /* vec_mergel (integrals).  */
 case ALTIVEC_BUILTIN_VMRGLH:
 case ALTIVEC_BUILTIN_VMRGLW:
diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h
index 04e9bfa..e634180 100644
--- a/gcc/gimple-fold.h
+++ b/gcc/gimple-fold.h
@@ -59,10 +59,11 @@ extern tree gimple_fold_indirect_ref (tree);
 extern bool gimple_fold_builtin_sprintf (gimple_stmt_iterator *);
 extern bool gimple_fold_builtin_snprintf (gimple_stmt_iterator *);
 extern bool arith_code_with_undefined_signed_overflow (tree_code);
 extern gimple_seq rewrite_to_defined_overflow (gimple *);
 extern void replace_call_with_value (gimple_stmt_iterator *, tree);
+extern tree tree_vec_extract (gimple_stmt_iterator *, tree, tree, tree, tree);
 
 /* gimple_build, functionally matching fold_buildN, outputs stmts
int the provided sequence, matching and simplifying them on-the-fly.
Supposed to replace force_gimple_operand (fold_buildN (...), ...).  */
 extern tree gimple_build (gimple_seq *, location_t,
diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c
index 909f790..1c9701d 100644
--- a/gcc/tree-vect-generic.c
+++ b/gcc/tree-vect-generic.c
@@ -118,11 +118,11 @@ build_word_mode_vector_type (int nunits)
 
 typedef tree (*elem_op_func) (gimple_stmt_iterator *

[PATCH] Fix bootstrap with --enable-fully-dynamic-string

[Jonathan Wakely]

PR libstdc++/86447
* src/c++11/cow-stdexcept.cc [_GLIBCXX_FULLY_DYNAMIC_STRING]
(logic_error::logic_error(logic_error&&))
(logic_error::operator=(logic_error&&))
(runtime_error::runtime_error(runtime_error&&))
(runtime_error::operator=(runtime_error&&)): Copy strings instead of
moving, to avoid allocating empty reps for moved-from strings.

Tested x86_64-linux, committed to trunk.


commit b06d2a88c3ea36c41c370cf95730d54f764b9f47
Author: Jonathan Wakely 
Date:   Thu Aug 16 15:31:40 2018 +0100

Fix bootstrap with --enable-fully-dynamic-string

PR libstdc++/86447
* src/c++11/cow-stdexcept.cc [_GLIBCXX_FULLY_DYNAMIC_STRING]
(logic_error::logic_error(logic_error&&))
(logic_error::operator=(logic_error&&))
(runtime_error::runtime_error(runtime_error&&))
(runtime_error::operator=(runtime_error&&)): Copy strings instead of
moving, to avoid allocating empty reps for moved-from strings.

diff --git a/libstdc++-v3/src/c++11/cow-stdexcept.cc 
b/libstdc++-v3/src/c++11/cow-stdexcept.cc
index 54859d58820..d271be529a6 100644
--- a/libstdc++-v3/src/c++11/cow-stdexcept.cc
+++ b/libstdc++-v3/src/c++11/cow-stdexcept.cc
@@ -57,6 +57,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // These operations are noexcept even though copying a COW string is not,
   // but we know that the string member in an exception has not been "leaked"
   // so copying is a simple reference count increment.
+  // For the fully dynamic string moves are not noexcept (due to needing to
+  // allocate an empty string) so we just define the moves as copies here.
 
   logic_error::logic_error(const logic_error& e) noexcept
   : exception(e), _M_msg(e._M_msg) { }
@@ -64,10 +66,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   logic_error& logic_error::operator=(const logic_error& e) noexcept
   { _M_msg = e._M_msg; return *this; }
 
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
   logic_error::logic_error(logic_error&& e) noexcept = default;
 
   logic_error&
   logic_error::operator=(logic_error&& e) noexcept = default;
+#else
+  logic_error::logic_error(logic_error&& e) noexcept
+  : exception(e), _M_msg(e._M_msg) { }
+
+  logic_error&
+  logic_error::operator=(logic_error&& e) noexcept
+  { _M_msg = e._M_msg; return *this; }
+#endif
 
   runtime_error::runtime_error(const runtime_error& e) noexcept
   : exception(e), _M_msg(e._M_msg) { }
@@ -76,10 +87,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   runtime_error::operator=(const runtime_error& e) noexcept
   { _M_msg = e._M_msg; return *this; }
 
+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
   runtime_error::runtime_error(runtime_error&& e) noexcept = default;
 
   runtime_error&
   runtime_error::operator=(runtime_error&& e) noexcept = default;
+#else
+  runtime_error::runtime_error(runtime_error&& e) noexcept
+  : exception(e), _M_msg(e._M_msg) { }
+
+  runtime_error&
+  runtime_error::operator=(runtime_error&& e) noexcept
+  { _M_msg = e._M_msg; return *this; }
+#endif
 
   // New C++11 constructors:
 

Re: [RFC][PATCH][mid-end] Optimize immediate choice in comparisons.

[Jeff Law]

On 08/14/2018 11:01 AM, Vlad Lazar wrote:
> On 13/08/18 15:00, Richard Sandiford wrote:
>> Vlad Lazar  writes:
>>> diff --git a/gcc/expmed.h b/gcc/expmed.h
>>> index
>>> 2890d9c9bbd034f01030dd551d544bf73e73b784..86a32a643fdd0fc9f396bd2c7904244bd484df16
>>> 100644
>>> --- a/gcc/expmed.h
>>> +++ b/gcc/expmed.h
>>> @@ -702,6 +702,10 @@ extern rtx emit_store_flag (rtx, enum rtx_code,
>>> rtx, rtx, machine_mode,
>>>    extern rtx emit_store_flag_force (rtx, enum rtx_code, rtx, rtx,
>>>  machine_mode, int, int);
>>>    +extern void canonicalize_comparison (machine_mode, enum rtx_code
>>> *, rtx *);
>>> +
>>> +extern enum rtx_code canonicalized_cmp_code (enum rtx_code);
>>
>> It would probably be better to make the second function static (local
>> to expmed.c), since it's only used internally by canonicalize_comparison.
>> Switching the order of the two functions in expmed.c would avoid the need
>> for a forward declaration.
>>
>>> @@ -6153,6 +6156,97 @@ emit_store_flag_force (rtx target, enum
>>> rtx_code code, rtx op0, rtx op1,
>>>     return target;
>>>    }
>>> +
>>> +/* Choose the more appropiate immediate in comparisons.  The purpose
>>> of this
>>
>> Maybe worth saying "scalar integer comparisons", since the function
>> doesn't handle vector or floating-point comparisons.
>>
>>> +   is to end up with an immediate which can be loaded into a
>>> register in fewer
>>> +   moves, if possible.
>>> +
>>> +   For each integer comparison there exists an equivalent choice:
>>> + i)   a >  b or a >= b + 1
>>> + ii)  a <= b or a <  b + 1
>>> + iii) a >= b or a >  b - 1
>>> + iv)  a <  b or a <= b - 1
>>> +
>>> +   The function is called in the gimple expanders which handle
>>> GIMPLE_ASSIGN
>>> +   and GIMPLE_COND.  It assumes that the first operand of the
>>> comparison is a
>>> +   register and the second is a constant.
>>
>> This last paragraph doesn't seem accurate any more.  Probably best to
>> drop it, since there's no real need to say who the callers are if we
>> describe the interface.
>>
>>> +   mode is the mode of the first operand.
>>> +   code points to the comparison code.
>>> +   imm points to the rtx containing the immediate.  */
>>
>> GCC's convention is to put parameter names in caps in comments,
>> so "MODE is...", etc.
>>
>> On the IMM line, it might be worth adding "*IMM must satisfy
>> CONST_SCALAR_INT_P on entry and continues to satisfy CONST_SCALAR_INT_P
>> on exit."
>>
>>> +void canonicalize_comparison (machine_mode mode, enum rtx_code
>>> *code, rtx *imm)
>>> +{
>>> +  int to_add = 0;
>>> +
>>> +  if (!SCALAR_INT_MODE_P (mode))
>>> +    return;
>>> +
>>> +  /* Extract the immediate value from the rtx.  */
>>> +  wide_int imm_val = wi::shwi (INTVAL (*imm), mode);
>>
>> This should be:
>>
>>    rtx_mode_t imm_val (*imm, mode);
>>
>> so that it copes with all CONST_SCALAR_INT_P, not just CONST_INT.
>>
>>> +  if (*code == GT || *code == GTU || *code == LE || *code == LEU)
>>> +    to_add = 1;
>>> +
>>> +  if (*code == GE || *code == GEU || *code == LT || *code == LTU)
>>> +    to_add = -1;
>>
>> Might be better to have:
>>
>>    if (*code == GT || *code == GTU || *code == LE || *code == LEU)
>>  to_add = 1;
>>    else if (*code == GE || *code == GEU || *code == LT || *code == LTU)
>>  to_add = -1;
>>    else
>>  return;
>>
>> so that we exit early for other comparisons, rather than doing wasted
>> work.
>>
>>> +  /* Check for overflow/underflow in the case of signed values and
>>> + wrapping around in the case of unsigned values.  If any occur
>>> + cancel the optimization.  */
>>> +  wide_int max_val = wi::max_value (mode, SIGNED);
>>> +  wide_int min_val = wi::min_value (mode, SIGNED);
>>> +  if ((wi::cmps (imm_val, max_val) == 0 && to_add == 1)
>>> +  || (wi::cmps (imm_val, min_val) == 0 && to_add == -1))
>>> +    return;
>>
>> This needs to use the SIGNED/UNSIGNED choice appropriate for the
>> comparison (SIGNED for GT, UNSIGNED for GTU, etc.).  There doesn't
>> seem to be an existing function that says whether an rtx comparison
>> operation is signed or not (bit of a surprise), but there is
>> unsigned_condition, which returns the unsigned form of an rtx
>> comparison operator.  It might be worth adding something like:
>>
>>    /* Return true if integer comparison operator CODE interprets its
>> operands
>>   as unsigned.  */
>>
>>    inline bool
>>    unsigned_condition_p (rtx_code code)
>>    {
>>  return unsigned_condition (code) == code;
>>    }
>>
>> and using that to choose between SIGNED and UNSIGNED.
>>
>> Rather than using wi::cmp, a more direct way of checking for overflow
>> is to change this:
>>
>>> +  /* Generate a mov instruction for both cases and see whether the
>>> change
>>> + was profitable.  */
>>> +  wide_int imm_modif = wi::add (imm_val, to_add);
>>
>> to use the overflow form of wi::add, i.e.:
>>
>>    wide_int imm_modif = wi::add (imm_val, to_add, sign, &overflow);
>>
>> and return if "overflo

Re: [RFC][PATCH][mid-end] Optimize immediate choice in comparisons.

[Vlad Lazar]

On 16/08/18 17:35, Jeff Law wrote:

On 08/14/2018 11:01 AM, Vlad Lazar wrote:

On 13/08/18 15:00, Richard Sandiford wrote:

Vlad Lazar  writes:

diff --git a/gcc/expmed.h b/gcc/expmed.h
index
2890d9c9bbd034f01030dd551d544bf73e73b784..86a32a643fdd0fc9f396bd2c7904244bd484df16
100644
--- a/gcc/expmed.h
+++ b/gcc/expmed.h
@@ -702,6 +702,10 @@ extern rtx emit_store_flag (rtx, enum rtx_code,
rtx, rtx, machine_mode,
extern rtx emit_store_flag_force (rtx, enum rtx_code, rtx, rtx,
  machine_mode, int, int);
+extern void canonicalize_comparison (machine_mode, enum rtx_code
*, rtx *);
+
+extern enum rtx_code canonicalized_cmp_code (enum rtx_code);


It would probably be better to make the second function static (local
to expmed.c), since it's only used internally by canonicalize_comparison.
Switching the order of the two functions in expmed.c would avoid the need
for a forward declaration.


@@ -6153,6 +6156,97 @@ emit_store_flag_force (rtx target, enum
rtx_code code, rtx op0, rtx op1,
 return target;
}
+
+/* Choose the more appropiate immediate in comparisons.  The purpose
of this


Maybe worth saying "scalar integer comparisons", since the function
doesn't handle vector or floating-point comparisons.


+   is to end up with an immediate which can be loaded into a
register in fewer
+   moves, if possible.
+
+   For each integer comparison there exists an equivalent choice:
+ i)   a >  b or a >= b + 1
+ ii)  a <= b or a <  b + 1
+ iii) a >= b or a >  b - 1
+ iv)  a <  b or a <= b - 1
+
+   The function is called in the gimple expanders which handle
GIMPLE_ASSIGN
+   and GIMPLE_COND.  It assumes that the first operand of the
comparison is a
+   register and the second is a constant.


This last paragraph doesn't seem accurate any more.  Probably best to
drop it, since there's no real need to say who the callers are if we
describe the interface.


+   mode is the mode of the first operand.
+   code points to the comparison code.
+   imm points to the rtx containing the immediate.  */


GCC's convention is to put parameter names in caps in comments,
so "MODE is...", etc.

On the IMM line, it might be worth adding "*IMM must satisfy
CONST_SCALAR_INT_P on entry and continues to satisfy CONST_SCALAR_INT_P
on exit."


+void canonicalize_comparison (machine_mode mode, enum rtx_code
*code, rtx *imm)
+{
+  int to_add = 0;
+
+  if (!SCALAR_INT_MODE_P (mode))
+return;
+
+  /* Extract the immediate value from the rtx.  */
+  wide_int imm_val = wi::shwi (INTVAL (*imm), mode);


This should be:

rtx_mode_t imm_val (*imm, mode);

so that it copes with all CONST_SCALAR_INT_P, not just CONST_INT.


+  if (*code == GT || *code == GTU || *code == LE || *code == LEU)
+to_add = 1;
+
+  if (*code == GE || *code == GEU || *code == LT || *code == LTU)
+to_add = -1;


Might be better to have:

if (*code == GT || *code == GTU || *code == LE || *code == LEU)
  to_add = 1;
else if (*code == GE || *code == GEU || *code == LT || *code == LTU)
  to_add = -1;
else
  return;

so that we exit early for other comparisons, rather than doing wasted
work.


+  /* Check for overflow/underflow in the case of signed values and
+ wrapping around in the case of unsigned values.  If any occur
+ cancel the optimization.  */
+  wide_int max_val = wi::max_value (mode, SIGNED);
+  wide_int min_val = wi::min_value (mode, SIGNED);
+  if ((wi::cmps (imm_val, max_val) == 0 && to_add == 1)
+  || (wi::cmps (imm_val, min_val) == 0 && to_add == -1))
+return;


This needs to use the SIGNED/UNSIGNED choice appropriate for the
comparison (SIGNED for GT, UNSIGNED for GTU, etc.).  There doesn't
seem to be an existing function that says whether an rtx comparison
operation is signed or not (bit of a surprise), but there is
unsigned_condition, which returns the unsigned form of an rtx
comparison operator.  It might be worth adding something like:

/* Return true if integer comparison operator CODE interprets its
operands
   as unsigned.  */

inline bool
unsigned_condition_p (rtx_code code)
{
  return unsigned_condition (code) == code;
}

and using that to choose between SIGNED and UNSIGNED.

Rather than using wi::cmp, a more direct way of checking for overflow
is to change this:


+  /* Generate a mov instruction for both cases and see whether the
change
+ was profitable.  */
+  wide_int imm_modif = wi::add (imm_val, to_add);


to use the overflow form of wi::add, i.e.:

wide_int imm_modif = wi::add (imm_val, to_add, sign, &overflow);

and return if "overflow" is set.


+
+  rtx reg = gen_reg_rtx (mode);


gen_reg_rtx creates a new pseudo register that essentially stays
around until we've finished compiling the function.  It's better to use:

gen_rtx_REG (mode, LAST_VIRTUAL_REGISTER + 1);

for cost calculations, so that we don't waste pseudo register numbers.


+  rtx new_imm = GEN_INT (imm_modif.to_shwi ());


This should be:

  

Re: [RFC][PATCH][mid-end] Optimize immediate choice in comparisons.

[Jeff Law]

On 08/16/2018 10:46 AM, Vlad Lazar wrote:
>> Thanks.  I fixed up the ChangeLog entry and installed this on the trunk.
>>
>> Richard S -- thanks for working with Vlad to push this forward.
>>
>> jeff
>>
> Thanks for committing.  Sorry about the ChangeLog.
No worries.  Just trivial stuff that we deal with all the time.

If you're going to be contributing regularly you probably want to get
write access to the gcc repository.

https://sourceware.org/cgi-bin/pdw/ps_form.cgi

List me as the sponsor/approver.

jeff

Re: [PATCH] [C] Warn when calculating abs(unsigned_value)

[Joseph Myers]

On Wed, 15 Aug 2018, Martin Sebor wrote:

> Detecting some of these bugs without too much noise would require
> moving the warning out of the front-end and to some later point
> after VRP has run.

But you need the information about whether the conversion was explicit or 
implicit.  If someone does

abs ((int) unsigned_var)

it's presumably intentional and should not receive a warning.  If they do

abs (unsigned_var)

it's suspect.  If they do

abs (long_var)

it's entirely possible they know the long value is within range for int, 
but it came from some API that produces long.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH] Merge Ignore and Deprecated in .opt files.

[Joseph Myers]

On Thu, 16 Aug 2018, Martin Liška wrote:

> On 08/15/2018 06:38 PM, Joseph Myers wrote:
> > On Wed, 15 Aug 2018, Martin Liška wrote:
> > 
> >> Ok, so you have very similar opinion as Jakub. Thus I'm sending new 
> >> version that preserves status quo, it only does:
> > 
> > This is removing RejectNegative from some Deprecated options.  Won't that 
> > result in the -fno-* variants of those options starting to be accepted, 
> > when they never were accepted when the positive versions of the options 
> > did something useful?
> > 
> 
> That's not intended, I fixed that. It the patch acceptable in form in 
> which it is?

This version is OK.

-- 
Joseph S. Myers
jos...@codesourcery.com

[PATCH] Avoid deprecation warning with -Wsystem-headers

[Jonathan Wakely]

C++17 says to use std::uncaught_exceptions() here instead of
std::uncaught_exception() but since we only care whether the result is
non-zero (and we aren't planning to remove the deprecated version) we
can just keep using std::uncaught_exception() and suppress the warning.

* include/std/ostream (basic_ostream::sentry::~sentry): Suppress
deprecation warnings for using uncaught_exception().

Tested x86_64-linux, committed to trunk.


commit a6d329728aaf20d795ea3e406b2523e9d1a02afd
Author: Jonathan Wakely 
Date:   Thu Aug 16 17:36:44 2018 +0100

Avoid deprecation warning with -Wsystem-headers

C++17 says to use std::uncaught_exceptions() here instead of
std::uncaught_exception() but since we only care whether the result is
non-zero (and we aren't planning to remove the deprecated version) we
can just keep using std::uncaught_exception() and suppress the warning.

* include/std/ostream (basic_ostream::sentry::~sentry): Suppress
deprecation warnings for using uncaught_exception().

diff --git a/libstdc++-v3/include/std/ostream b/libstdc++-v3/include/std/ostream
index 448a9288188..2099294fd92 100644
--- a/libstdc++-v3/include/std/ostream
+++ b/libstdc++-v3/include/std/ostream
@@ -444,6 +444,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   explicit
   sentry(basic_ostream<_CharT, _Traits>& __os);
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
   /**
*  @brief  Possibly flushes the stream.
*
@@ -461,6 +463,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  _M_os.setstate(ios_base::badbit);
  }
   }
+#pragma GCC diagnostic pop
 
   /**
*  @brief  Quick status checking.

[committed] -Wmisleading-indentation: fix ICE in get_visual_column (PR c++/70693)

[David Malcolm]

PR c++/70693 reports a crash within -Wmisleading-indentation in
get_visual_column, reading past the end of a source line.

The issue occurs due to a stray carriage return aka '\r' aka ^M, occurring
towards the end of line 35 of attachment 38289 - but not at the end itself.

This carriage return confuses our line numbering: from that point in the
file, the lexer (and thus location_t values) use line numbers that are
one larger than those seen by input.c, "cat -n" and emacs.

This discrepancy between the lexer's line numbering and input.c's line
numbering leads to an out-of-range read in get_visual_column (trying to
read column 8, to locate the first non-whitespace on the line containing
"break;", but finding the next line, which is only 4 characters long).

This patch fixes the ICE by adding a range check to get_visual_column
before accessing the input.c line buffer.  This is arguably papering
over the root cause, but there are presumably other ways of triggering
such an out-of-range read by writing to the source file after the lexer
but before -Wmisleading-indentation, and we ought to not ICE in the
face of that.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
verified that "make selftest-valgrind" is clean.

Committed to trunk as r263595.

gcc/c-family/ChangeLog:
PR c++/70693
* c-common.c (selftest::c_family_tests): Call
selftest::c_indentation_c_tests.
* c-common.h (selftest::c_indentation_c_tests): New decl.
* c-indentation.c: Include "selftest.h".
(next_tab_stop): Add "tab_width" param, rather than accessing
cpp_opts.
(get_visual_column): Likewise.  Clarify comment.  Bulletproof
against reading past the end of the line.
(get_first_nws_vis_column): Add "tab_width" param.
(detect_intervening_unindent): Likewise.
(should_warn_for_misleading_indentation): Read tab width from
cpp_opts and pass around.
(selftest::test_next_tab_stop): New test.
(selftest::assert_get_visual_column_succeeds): New function.
(ASSERT_GET_VISUAL_COLUMN_SUCCEEDS): New macro.
(selftest::assert_get_visual_column_fails): New function.
(ASSERT_GET_VISUAL_COLUMN_FAILS): New macro.
(selftest::test_get_visual_column): New test.
(selftest::c_indentation_c_tests): New function.

gcc/testsuite/ChangeLog:
PR c++/70693
* c-c++-common/Wmisleading-indentation-pr70693.c: New test.
---
 gcc/c-family/c-common.c|   1 +
 gcc/c-family/c-common.h|   1 +
 gcc/c-family/c-indentation.c   | 192 +++--
 .../c-c++-common/Wmisleading-indentation-pr70693.c |  12 ++
 4 files changed, 192 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wmisleading-indentation-pr70693.c

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 55b2e50..95cff21 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -8370,6 +8370,7 @@ c_family_tests (void)
 {
   c_common_c_tests ();
   c_format_c_tests ();
+  c_indentation_c_tests ();
   c_pretty_print_c_tests ();
   c_spellcheck_cc_tests ();
 }
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 8a802bb..9b05e60 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1338,6 +1338,7 @@ namespace selftest {
   /* Declarations for specific families of tests within c-family,
  by source file, in alphabetical order.  */
   extern void c_format_c_tests (void);
+  extern void c_indentation_c_tests (void);
   extern void c_pretty_print_c_tests (void);
   extern void c_spellcheck_cc_tests (void);
 
diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c
index 44b1e1e..436d61b 100644
--- a/gcc/c-family/c-indentation.c
+++ b/gcc/c-family/c-indentation.c
@@ -23,15 +23,15 @@ along with GCC; see the file COPYING3.  If not see
 #include "tm.h"
 #include "c-common.h"
 #include "c-indentation.h"
+#include "selftest.h"
 
 extern cpp_options *cpp_opts;
 
 /* Round up VIS_COLUMN to nearest tab stop. */
 
 static unsigned int
-next_tab_stop (unsigned int vis_column)
+next_tab_stop (unsigned int vis_column, unsigned int tab_width)
 {
-  const unsigned int tab_width = cpp_opts->tabstop;
   vis_column = ((vis_column + tab_width) / tab_width) * tab_width;
   return vis_column;
 }
@@ -43,12 +43,13 @@ next_tab_stop (unsigned int vis_column)
Returns true if a conversion was possible, writing the result to OUT,
otherwise returns false.  If FIRST_NWS is not NULL, then write to it
the visual column corresponding to the first non-whitespace character
-   on the line.  */
+   on the line (up to or before EXPLOC).  */
 
 static bool
 get_visual_column (expanded_location exploc, location_t loc,
   unsigned int *out,
-  unsigned int *first_nws)
+  unsigned int *first_nws,
+  unsigned int tab_width)
 {
   /*

[PATCH 0/4] rs6000: length attribute

[Segher Boessenkool]

This series makes some changes to the length attribute.

Currently, the length attribute for branch instructions defaults to
what is needed for B-form instructions, that is, branches with an
immediate 16-bit displacement field.  There is only one such machine
insn, and only three such RTL patterns.

This series moves this treatment to those few patterns, and defaults
branch instructions to length 4 bytes (like all other instructions).

It then deletes many instances where we explicitly set the attribute
to the default value.

It also deletes a few related patterns that are not useful anymore.

Bootstrapped and tested on powerpc64-linux; will also test on
powerpc64le-linux before committing.


Segher


 gcc/config/rs6000/altivec.md |  81 ++--
 gcc/config/rs6000/darwin.md  |  45 +++--
 gcc/config/rs6000/dfp.md |   6 +-
 gcc/config/rs6000/htm.md |  36 +++
 gcc/config/rs6000/rs6000.md  | 223 +++
 gcc/config/rs6000/sync.md|   6 +-
 gcc/config/rs6000/vsx.md |   3 +-
 7 files changed, 134 insertions(+), 266 deletions(-)

-- 
1.8.3.1

[PATCH 1/4] rs6000: Change the length attribute default

[Segher Boessenkool]

This moves what is currently the default of the length attribute to
the only branch instruction patterns where it applies, namely, the
B-form instructions.  It was used for the "jump" instruction as well
before, but that is an I-form instruction and always has length 4.


2018-08-16  Segher Boessenkool  

* config/rs6000/rs6000.md (length): Always define as const_int 4.
(unnamed conditional branch define_insn): Set length to 4 or 8
depending on offset.
(_): Similar, for alternative 0.
(tf_): Ditto.

---
 gcc/config/rs6000/rs6000.md | 47 ++---
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index d70b01b..45e42ff 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -243,18 +243,8 @@ (define_attr "var_shift" "no,yes"
 ;; Is copying of this instruction disallowed?
 (define_attr "cannot_copy" "no,yes" (const_string "no"))
 
-;; Length (in bytes).
-; '(pc)' in the following doesn't include the instruction itself; it is
-; calculated as if the instruction had zero size.
-(define_attr "length" ""
-  (if_then_else (eq_attr "type" "branch")
-   (if_then_else (and (ge (minus (match_dup 0) (pc))
-  (const_int -32768))
-  (lt (minus (match_dup 0) (pc))
-  (const_int 32764)))
- (const_int 4)
- (const_int 8))
-   (const_int 4)))
+;; Length of the instruction (in bytes).
+(define_attr "length" "" (const_int 4))
 
 ;; Processor type -- this attribute must exactly match the processor_type
 ;; enumeration in rs6000-opts.h.
@@ -12346,7 +12336,14 @@ (define_insn ""
 {
   return output_cbranch (operands[1], "%l0", 0, insn);
 }
-  [(set_attr "type" "branch")])
+  [(set_attr "type" "branch")
+   (set (attr "length")
+   (if_then_else (and (ge (minus (match_dup 0) (pc))
+  (const_int -32768))
+  (lt (minus (match_dup 0) (pc))
+  (const_int 32764)))
+ (const_int 4)
+ (const_int 8)))])
 
 (define_insn ""
   [(set (pc)
@@ -12716,7 +12713,17 @@ (define_insn "_"
 return " $+8\;b %l0";
 }
   [(set_attr "type" "branch")
-   (set_attr "length" "*,16,20,20")])
+   (set (attr "length")
+   (cond [(eq (symbol_ref "which_alternative") (const_int 0))
+ (if_then_else (and (ge (minus (match_dup 0) (pc))
+(const_int -32768))
+(lt (minus (match_dup 0) (pc))
+(const_int 32764)))
+   (const_int 4)
+   (const_int 8))
+  (eq (symbol_ref "which_alternative") (const_int 1))
+ (const_int 16)]
+  (const_int 20)))])
 
 ;; Now the splitter if we could not allocate the CTR register
 (define_split
@@ -12795,7 +12802,17 @@ (define_insn "tf_"
 }
 }
   [(set_attr "type" "branch")
-   (set_attr "length" "*,16,20,20")])
+   (set (attr "length")
+   (cond [(eq (symbol_ref "which_alternative") (const_int 0))
+ (if_then_else (and (ge (minus (match_dup 0) (pc))
+(const_int -32768))
+(lt (minus (match_dup 0) (pc))
+(const_int 32764)))
+   (const_int 4)
+   (const_int 8))
+  (eq (symbol_ref "which_alternative") (const_int 1))
+ (const_int 16)]
+(const_int 20)))])
 
 ;; Now the splitter if we could not allocate the CTR register
 (define_split
-- 
1.8.3.1

[PATCH 2/4] rs6000: Remove "length 4" from branch insns

[Segher Boessenkool]

Now that it is the default for branch insns like for all other insns,
we don't need to set it explicitly so often anymore.


2018-08-16  Segher Boessenkool  

* config/rs6000/altivec.md: Don't set length attribute to the default
value, for branch instructions.
* config/rs6000/darwin.md: Ditto.
* config/rs6000/rs6000.md: Ditto.

---
 gcc/config/rs6000/altivec.md | 15 +++--
 gcc/config/rs6000/darwin.md  | 12 +++
 gcc/config/rs6000/rs6000.md  | 74 ++--
 3 files changed, 33 insertions(+), 68 deletions(-)

diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index 8ee42ae..1af9687 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -413,8 +413,7 @@ (define_insn "*save_world"
(use (match_operand:SI 1 "call_operand" "s"))])]
  "TARGET_MACHO && (DEFAULT_ABI == ABI_DARWIN) && TARGET_32BIT" 
  "bl %z1"
-  [(set_attr "type" "branch")
-   (set_attr "length" "4")])
+  [(set_attr "type" "branch")])
 
 (define_insn "*restore_world"
  [(match_parallel 0 "restore_world_operation"
@@ -441,8 +440,7 @@ (define_insn "*save_vregs__r11"
   (match_operand:V4SI 4 "altivec_register_operand" "v"))])]
   "TARGET_ALTIVEC"
   "bl %1"
-  [(set_attr "type" "branch")
-   (set_attr "length" "4")])
+  [(set_attr "type" "branch")])
 
 (define_insn "*save_vregs__r12"
   [(match_parallel 0 "any_parallel_operand"
@@ -455,8 +453,7 @@ (define_insn "*save_vregs__r12"
   (match_operand:V4SI 4 "altivec_register_operand" "v"))])]
   "TARGET_ALTIVEC"
   "bl %1"
-  [(set_attr "type" "branch")
-   (set_attr "length" "4")])
+  [(set_attr "type" "branch")])
 
 (define_insn "*restore_vregs__r11"
   [(match_parallel 0 "any_parallel_operand"
@@ -469,8 +466,7 @@ (define_insn "*restore_vregs__r11"
 (match_operand:P 4 "short_cint_operand" "I"])]
   "TARGET_ALTIVEC"
   "bl %1"
-  [(set_attr "type" "branch")
-   (set_attr "length" "4")])
+  [(set_attr "type" "branch")])
 
 (define_insn "*restore_vregs__r12"
   [(match_parallel 0 "any_parallel_operand"
@@ -483,8 +479,7 @@ (define_insn "*restore_vregs__r12"
 (match_operand:P 4 "short_cint_operand" "I"])]
   "TARGET_ALTIVEC"
   "bl %1"
-  [(set_attr "type" "branch")
-   (set_attr "length" "4")])
+  [(set_attr "type" "branch")])
 
 ;; Simple binary operations.
 
diff --git a/gcc/config/rs6000/darwin.md b/gcc/config/rs6000/darwin.md
index 48fd5b96..7c429a5 100644
--- a/gcc/config/rs6000/darwin.md
+++ b/gcc/config/rs6000/darwin.md
@@ -263,8 +263,7 @@ (define_insn "load_macho_picbase_si"
   return "bcl 20,31,%0\n%0:";
 }
   [(set_attr "type" "branch")
-   (set_attr "cannot_copy" "yes")
-   (set_attr "length" "4")])
+   (set_attr "cannot_copy" "yes")])
 
 (define_insn "load_macho_picbase_di"
   [(set (reg:DI LR_REGNO)
@@ -280,8 +279,7 @@ (define_insn "load_macho_picbase_di"
   return "bcl 20,31,%0\n%0:";
 }
   [(set_attr "type" "branch")
-   (set_attr "cannot_copy" "yes")
-   (set_attr "length" "4")])
+   (set_attr "cannot_copy" "yes")])
 
 (define_expand "macho_correct_pic"
   [(set (match_operand 0 "")
@@ -416,8 +414,7 @@ (define_insn "reload_macho_picbase_si"
 return "bcl 20,31,%0\n%0:";
 }
   [(set_attr "type" "branch")
-   (set_attr "cannot_copy" "yes")
-   (set_attr "length" "4")])
+   (set_attr "cannot_copy" "yes")])
 
 (define_insn "reload_macho_picbase_di"
   [(set (reg:DI LR_REGNO)
@@ -440,8 +437,7 @@ (define_insn "reload_macho_picbase_di"
 return "bcl 20,31,%0\n%0:";
 }
   [(set_attr "type" "branch")
-   (set_attr "cannot_copy" "yes")
-   (set_attr "length" "4")])
+   (set_attr "cannot_copy" "yes")])
 
 ;; We need to restore the PIC register, at the site of nonlocal label.
 
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 45e42ff..c066bae 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -9450,8 +9450,7 @@ (define_insn "*tls_gd_call_sysv"
 }
   return "bl %z1(%3@tlsgd)";
 }
-  [(set_attr "type" "branch")
-   (set_attr "length" "4")])
+  [(set_attr "type" "branch")])
 
 (define_insn_and_split "tls_ld_aix"
   [(set (match_operand:TLSmode 0 "gpc_reg_operand" "=b")
@@ -9584,8 +9583,7 @@ (define_insn "*tls_ld_call_sysv"
 }
   return "bl %z1(%&@tlsld)";
 }
-  [(set_attr "type" "branch")
-   (set_attr "length" "4")])
+  [(set_attr "type" "branch")])
 
 (define_insn "tls_dtprel_"
   [(set (match_operand:TLSmode 0 "gpc_reg_operand" "=r")
@@ -10028,8 +10026,7 @@ (define_insn "load_toc_v4_pic_si"
(unspec:SI [(const_int 0)] UNSPEC_TOC))]
   "DEFAULT_ABI == ABI_V4 && flag_pic == 1 && TARGET_32BIT"
   "bl _GLOBAL_OFFSET_TABLE_@local-4"
-  [(set_attr "type" "branch")
-   (set_attr "length" "4")])
+  [(set_attr "type" "branch")])
 
 (define_expand "load_toc_v4_PIC_1"
   [(parallel [(set (reg:SI LR_REGNO)
@@ -10047,7 +10044,6 @@ (define_insn "load_toc_v4_PIC_1_normal"
&& (flag_pic == 2 || (flag_pic && TARGET_SEC

[PATCH 4/4] rs6000: Delete old add+cmp patterns

[Segher Boessenkool]

There are some patterns that recognise the parallel of an add and a
compare, and split it back to the same two insns.  This apparently
helped RIOS machines before RTL scheduling existed?  Either way, it
isn't helpful anymore, and even hurts a tiny bit.  So, delete it.


2018-08-16  Segher Boessenkool  

* config/rs6000/rs6000.md (two unnamed define_insn and define_split):
Delete.

---
 gcc/config/rs6000/rs6000.md | 45 -
 1 file changed, 45 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 50c264f..c691952 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -11581,51 +11581,6 @@ (define_peephole2
   operands[10] = GEN_INT (sextc);
 })
 
-;; The following two insns don't exist as single insns, but if we provide
-;; them, we can swap an add and compare, which will enable us to overlap more
-;; of the required delay between a compare and branch.  We generate code for
-;; them by splitting.
-
-(define_insn ""
-  [(set (match_operand:CC 3 "cc_reg_operand" "=y")
-   (compare:CC (match_operand:SI 1 "gpc_reg_operand" "r")
-   (match_operand:SI 2 "short_cint_operand" "i")))
-   (set (match_operand:SI 0 "gpc_reg_operand" "=r")
-   (plus:SI (match_dup 1) (match_operand:SI 4 "short_cint_operand" "i")))]
-  ""
-  "#"
-  [(set_attr "length" "8")])
-
-(define_insn ""
-  [(set (match_operand:CCUNS 3 "cc_reg_operand" "=y")
-   (compare:CCUNS (match_operand:SI 1 "gpc_reg_operand" "r")
-  (match_operand:SI 2 "u_short_cint_operand" "i")))
-   (set (match_operand:SI 0 "gpc_reg_operand" "=r")
-   (plus:SI (match_dup 1) (match_operand:SI 4 "short_cint_operand" "i")))]
-  ""
-  "#"
-  [(set_attr "length" "8")])
-
-(define_split
-  [(set (match_operand:CC 3 "cc_reg_operand")
-   (compare:CC (match_operand:SI 1 "gpc_reg_operand")
-   (match_operand:SI 2 "short_cint_operand")))
-   (set (match_operand:SI 0 "gpc_reg_operand")
-   (plus:SI (match_dup 1) (match_operand:SI 4 "short_cint_operand")))]
-  ""
-  [(set (match_dup 3) (compare:CC (match_dup 1) (match_dup 2)))
-   (set (match_dup 0) (plus:SI (match_dup 1) (match_dup 4)))])
-
-(define_split
-  [(set (match_operand:CCUNS 3 "cc_reg_operand")
-   (compare:CCUNS (match_operand:SI 1 "gpc_reg_operand")
-  (match_operand:SI 2 "u_short_cint_operand")))
-   (set (match_operand:SI 0 "gpc_reg_operand")
-   (plus:SI (match_dup 1) (match_operand:SI 4 "short_cint_operand")))]
-  ""
-  [(set (match_dup 3) (compare:CCUNS (match_dup 1) (match_dup 2)))
-   (set (match_dup 0) (plus:SI (match_dup 1) (match_dup 4)))])
-
 ;; Only need to compare second words if first words equal
 (define_insn "*cmp_internal1"
   [(set (match_operand:CCFP 0 "cc_reg_operand" "=y")
-- 
1.8.3.1

[PATCH 3/4] rs6000: Remove "length 4" from other insns

[Segher Boessenkool]

There were many insns that set "length 4" explicitly while that does
not make anything clearer to the reader.  So, simplify the code.


2018-08-16  Segher Boessenkool  

* config/rs6000/altivec.md: Don't set length attribute to the default
value.
* config/rs6000/darwin.md: Ditto.
* config/rs6000/dfp.md: Ditto.
* config/rs6000/htm.md: Ditto.
* config/rs6000/rs6000.md: Ditto.
* config/rs6000/sync.md: Ditto.
* config/rs6000/vsx.md: Ditto.

---
 gcc/config/rs6000/altivec.md | 66 +++-
 gcc/config/rs6000/darwin.md  | 33 --
 gcc/config/rs6000/dfp.md |  6 ++--
 gcc/config/rs6000/htm.md | 36 
 gcc/config/rs6000/rs6000.md  | 57 +-
 gcc/config/rs6000/sync.md|  6 ++--
 gcc/config/rs6000/vsx.md |  3 +-
 7 files changed, 69 insertions(+), 138 deletions(-)

diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index 1af9687..3419e3a 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -2042,8 +2042,7 @@ (define_insn "altivec_vperm__direct"
   "@
vperm %0,%1,%2,%3
xxperm %x0,%x1,%x3"
-  [(set_attr "type" "vecperm")
-   (set_attr "length" "4")])
+  [(set_attr "type" "vecperm")])
 
 (define_insn "altivec_vperm_v8hiv16qi"
   [(set (match_operand:V16QI 0 "register_operand" "=v,?wo")
@@ -2055,8 +2054,7 @@ (define_insn "altivec_vperm_v8hiv16qi"
   "@
vperm %0,%1,%2,%3
xxperm %x0,%x1,%x3"
-  [(set_attr "type" "vecperm")
-   (set_attr "length" "4")])
+  [(set_attr "type" "vecperm")])
 
 (define_expand "altivec_vperm__uns"
   [(set (match_operand:VM 0 "register_operand")
@@ -2083,8 +2081,7 @@ (define_insn "*altivec_vperm__uns_internal"
   "@
vperm %0,%1,%2,%3
xxperm %x0,%x1,%x3"
-  [(set_attr "type" "vecperm")
-   (set_attr "length" "4")])
+  [(set_attr "type" "vecperm")])
 
 (define_expand "vec_permv16qi"
   [(set (match_operand:V16QI 0 "register_operand")
@@ -2110,8 +2107,7 @@ (define_insn "*altivec_vpermr__internal"
   "@
vpermr %0,%1,%2,%3
xxpermr %x0,%x1,%x3"
-  [(set_attr "type" "vecperm")
-   (set_attr "length" "4")])
+  [(set_attr "type" "vecperm")])
 
 (define_insn "altivec_vrfip"   ; ceil
   [(set (match_operand:V4SF 0 "register_operand" "=v")
@@ -3268,8 +3264,7 @@ (define_insn "vperm_v8hiv4si"
   "@
vperm %0,%1,%2,%3
xxperm %x0,%x1,%x3"
-  [(set_attr "type" "vecperm")
-   (set_attr "length" "4")])
+  [(set_attr "type" "vecperm")])
 
 (define_insn "vperm_v16qiv8hi"
   [(set (match_operand:V8HI 0 "register_operand" "=v,?wo")
@@ -3281,8 +3276,7 @@ (define_insn "vperm_v16qiv8hi"
   "@
vperm %0,%1,%2,%3
xxperm %x0,%x1,%x3"
-  [(set_attr "type" "vecperm")
-   (set_attr "length" "4")])
+  [(set_attr "type" "vecperm")])
 
 
 (define_expand "vec_unpacku_hi_v16qi"
@@ -3857,8 +3851,7 @@ (define_insn "*p8v_clz2"
(clz:VI2 (match_operand:VI2 1 "register_operand" "v")))]
   "TARGET_P8_VECTOR"
   "vclz %0,%1"
-  [(set_attr "length" "4")
-   (set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecsimple")])
 
 ;; Vector absolute difference unsigned
 (define_expand "vadu3"
@@ -3884,8 +3877,7 @@ (define_insn "*p9v_ctz2"
(ctz:VI2 (match_operand:VI2 1 "register_operand" "v")))]
   "TARGET_P9_VECTOR"
   "vctz %0,%1"
-  [(set_attr "length" "4")
-   (set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecsimple")])
 
 ;; Vector population count
 (define_insn "*p8v_popcount2"
@@ -3893,8 +3885,7 @@ (define_insn "*p8v_popcount2"
 (popcount:VI2 (match_operand:VI2 1 "register_operand" "v")))]
   "TARGET_P8_VECTOR"
   "vpopcnt %0,%1"
-  [(set_attr "length" "4")
-   (set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecsimple")])
 
 ;; Vector parity
 (define_insn "*p9v_parity2"
@@ -3902,8 +3893,7 @@ (define_insn "*p9v_parity2"
 (parity:VParity (match_operand:VParity 1 "register_operand" "v")))]
   "TARGET_P9_VECTOR"
   "vprtyb %0,%1"
-  [(set_attr "length" "4")
-   (set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecsimple")])
 
 ;; Vector Gather Bits by Bytes by Doubleword
 (define_insn "p8v_vgbbd"
@@ -3912,8 +3902,7 @@ (define_insn "p8v_vgbbd"
  UNSPEC_VGBBD))]
   "TARGET_P8_VECTOR"
   "vgbbd %0,%1"
-  [(set_attr "length" "4")
-   (set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecsimple")])
 
 
 ;; 128-bit binary integer arithmetic
@@ -3927,8 +3916,7 @@ (define_insn "altivec_vadduqm"
   (match_operand:V1TI 2 "register_operand" "v")))]
   "TARGET_VADDUQM"
   "vadduqm %0,%1,%2"
-  [(set_attr "length" "4")
-   (set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecsimple")])
 
 (define_insn "altivec_vaddcuq"
   [(set (match_operand:V1TI 0 "register_operand" "=v")
@@ -3937,8 +3925,7 @@ (define_insn "altivec_vaddcuq"
 UNSPEC_VADDCUQ))]
   "TARGET_VADDUQM"
   "vaddcuq %0,%1,%2"
-  [(set_attr "length" "4")
-   (set_attr "type" "vecsimple")])
+  [(set_at

[PATCH] Fix warning with -Wsign-compare -Wsystem-headers in __sph_legendre

[Jonathan Wakely]

Ed, I'm checking this in as it looks correct anyway, quite apart from
the fact it fixes a warning, but could you please double check it?

* include/tr1/legendre_function.tcc (__sph_legendre): Avoid warning
about signed/unsigned comparison.

Tested x86_64-linux, committed to trunk.


commit 67aca02fcfb8cde961480ebb1a2d19d1439d2e98
Author: Jonathan Wakely 
Date:   Thu Aug 16 17:57:58 2018 +0100

Fix warning with -Wsign-compare -Wsystem-headers

* include/tr1/legendre_function.tcc (__sph_legendre): Avoid warning
about signed/unsigned comparison.

diff --git a/libstdc++-v3/include/tr1/legendre_function.tcc 
b/libstdc++-v3/include/tr1/legendre_function.tcc
index 85ca7969f5b..e75414c5296 100644
--- a/libstdc++-v3/include/tr1/legendre_function.tcc
+++ b/libstdc++-v3/include/tr1/legendre_function.tcc
@@ -284,7 +284,7 @@ namespace tr1
   _Tp __y_lm = _Tp(0);
 
   // Compute Y_l^m, l > m+1, upward recursion on l.
-  for ( int __ll = __m + 2; __ll <= __l; ++__ll)
+  for (unsigned int __ll = __m + 2; __ll <= __l; ++__ll)
 {
   const _Tp __rat1 = _Tp(__ll - __m) / _Tp(__ll + __m);
   const _Tp __rat2 = _Tp(__ll - __m - 1) / _Tp(__ll + __m - 1);

Re: [PATCH] Add a character size parameter to c_strlen/get_range_strlen

[Jeff Law]

On 08/15/2018 12:51 PM, Bernd Edlinger wrote:
[ snip -- comment attribution is likely lost... ]


 2018-08-14  Bernd Edlinger  

 * builtins.c (c_strlen): Add new parameter eltsize.
 * builtins.h (c_strlen): Adjust prototype.
 * expr.c (string_constant): Add new parameter mem_size.
 * expr.h (string_constant): Adjust protoype.
 * gimple-fold.c (get_range_strlen): Add new parameter eltsize.
 * gimple-fold.h (get_range_strlen): Adjust prototype.
 * gimple-ssa-sprintf.c (get_string_length): Add new parameter eltsize.
 (format_string): Call get_string_length with eltsize.

 2018-08-14  Bernd Edlinger  

 * gcc.dg/strlenopt-49.c: Adjust test case.
 * gcc.dg/tree-ssa/builtin-sprintf-warn-18.c: Likewise.
>>>
>>> Your patch appears to pass in an ELTSIZE which is used to determine how
>>> to count.  This has some advantages in that the routine can be used to
>>> count bytes or other elements such as 2 or 4 byte wide characters.
[ ... ]
So yesterday I sent a message where I was totally offbase with the
intent of this patch.  After further discussions and re-review ISTM the
real intent here is to restore prior behavior of counting bytes to the
clients outside the sprintf pass.  Terribly sorry for that and the
conclusions I drew as a result.




>>
>> Parameterizing the function to return either the number of
>> bytes or the number of elements makes sense as an enhancement.
>> It makes less sense (and could be the source of bugs) to let
>> callers pass in anything else.  A boolean flag to toggle
>> between the two alternatives would make the API narrower
>> and safer to use.  It doesn't seem necessary to fix any
>> bugs.
No bugs that we're currently aware of. But ensuring that routines that
expect to be counting bytes are counting bytes is a good thing.  If some
pass (say forwprop) changed the char * argument to an array of something
other than chars then we'd likely end up giving a wrong result.

I don't mind making the interface narrower with a boolean or an enum to
specify how we count rather than having the caller pass in a type.


>>
>>> Your testsuite change appears to remove the xfails in strlenopt-49 which
>>> is usually OK.  So no concerns there.
So it turns out we don't really need any of the testsuite changes anyway
if we make this patch independent of the 86711/86714 patches.  And
breaking that dependency is IMHO a good thing.

[ Another snip. ]

> Anyway, I thought your patch would convert dir.specifier == 'S'
> into dir.modifier == FMT_LEN_l, since %S is like %ls.
> 
> the xfail will go away if I change this:
> 
> get_string_length (arg, dir.modifier == FMT_LEN_l ? 4 : 1);
> 
> to:

> get_string_length (arg, dir.specifier == 'S' dir.modifier == FMT_LEN_l ? 4 : 
> 1);
Presumably  "||" is missing here.

> 
> 
> That should be easy, I will change the patch to match your patch in that
> area.
> 
> 
> The other xfail with memcmp needs to be fixed separately.
> The issue is, callers of string_constant that do not pass the mem_size
> parameter may access up to TREE_STRING_LENGTH byte, therefore
> returning longer strings is not appropriate: the string constant
> contains 9 bytes, mind the terminating nul.
If we make the patch to add the size parameter to c_strlen independent
of the changes related to 86711/86714, then we don't need to xfail or
change the testsuite in any way shape or form.

My overall thinking is to carve out this patch so that it's independent
of anything else.  That's actually easy to do after addressing two very
minor issues.

We can then add Martin's patch to fix handling of wide chars in sprintf
(86853).

--

This overall plan addresses the need to be able to tell c_strlen how to
count and reverts its behavior for callers outside of the sprintf
warning pass to historical (pre-gcc7) behavior of counting bytes by
default.  It does this without losing any warnings and allow us to move
forward with Martin's more complete fix for 86853.

With those two patches out of the way we can then dig further into the
issues around 86711/86714.


jeff

Re: [PATCH] [C] Warn when calculating abs(unsigned_value)

[Martin Sebor]

On 08/16/2018 10:49 AM, Joseph Myers wrote:

On Wed, 15 Aug 2018, Martin Sebor wrote:


Detecting some of these bugs without too much noise would require
moving the warning out of the front-end and to some later point
after VRP has run.


But you need the information about whether the conversion was explicit or
implicit.  If someone does

abs ((int) unsigned_var)

it's presumably intentional and should not receive a warning.  If they do

abs (unsigned_var)

it's suspect.  If they do

abs (long_var)

it's entirely possible they know the long value is within range for int,
but it came from some API that produces long.


Right.  The warning would need to avoid triggering for arguments
in an unknown range, or in a range that straddles the boundary
between positive and negative values.

But the lack of a distinction between implicit and explicit casts
int the middle-end is a problem, not just here but in other contexts.
Maybe NOP_EXPR could be extended to capture it.  There should be
an unused bit there somewhere.  I suspect this would be far more
work than could be justified for a single warning, but it may be
an idea for a more general design enhancement.

Martin

Re: [PATCH] Add a character size parameter to c_strlen/get_range_strlen

[Bernd Edlinger]

On 08/16/18 19:35, Jeff Law wrote:
> On 08/15/2018 12:51 PM, Bernd Edlinger wrote:
> [ snip -- comment attribution is likely lost... ]
> 
>
> 2018-08-14  Bernd Edlinger  
>
>  * builtins.c (c_strlen): Add new parameter eltsize.
>  * builtins.h (c_strlen): Adjust prototype.
>  * expr.c (string_constant): Add new parameter mem_size.
>  * expr.h (string_constant): Adjust protoype.
>  * gimple-fold.c (get_range_strlen): Add new parameter eltsize.
>  * gimple-fold.h (get_range_strlen): Adjust prototype.
>  * gimple-ssa-sprintf.c (get_string_length): Add new parameter 
> eltsize.
>  (format_string): Call get_string_length with eltsize.
>
> 2018-08-14  Bernd Edlinger  
>
>  * gcc.dg/strlenopt-49.c: Adjust test case.
>  * gcc.dg/tree-ssa/builtin-sprintf-warn-18.c: Likewise.

 Your patch appears to pass in an ELTSIZE which is used to determine how
 to count.  This has some advantages in that the routine can be used to
 count bytes or other elements such as 2 or 4 byte wide characters.
> [ ... ]
> So yesterday I sent a message where I was totally offbase with the
> intent of this patch.  After further discussions and re-review ISTM the
> real intent here is to restore prior behavior of counting bytes to the
> clients outside the sprintf pass.  Terribly sorry for that and the
> conclusions I drew as a result.
> 
> 

no problem at all.

I really appreciate the time you take to review
this patch.

And I see how confusing this must be, but I think
some things went in a wrong direction for quite some time.

> 
> 
>>>
>>> Parameterizing the function to return either the number of
>>> bytes or the number of elements makes sense as an enhancement.
>>> It makes less sense (and could be the source of bugs) to let
>>> callers pass in anything else.  A boolean flag to toggle
>>> between the two alternatives would make the API narrower
>>> and safer to use.  It doesn't seem necessary to fix any
>>> bugs.
> No bugs that we're currently aware of. But ensuring that routines that
> expect to be counting bytes are counting bytes is a good thing.  If some
> pass (say forwprop) changed the char * argument to an array of something
> other than chars then we'd likely end up giving a wrong result.
> 
> I don't mind making the interface narrower with a boolean or an enum to
> specify how we count rather than having the caller pass in a type.
> 
> 

Yes, the caller _must_ know what to expect.
The function should not try to be smarter than the caller.

>>>
 Your testsuite change appears to remove the xfails in strlenopt-49 which
 is usually OK.  So no concerns there.
> So it turns out we don't really need any of the testsuite changes anyway
> if we make this patch independent of the 86711/86714 patches.  And
> breaking that dependency is IMHO a good thing.
> 
> [ Another snip. ]
> 
>> Anyway, I thought your patch would convert dir.specifier == 'S'
>> into dir.modifier == FMT_LEN_l, since %S is like %ls.
>>
>> the xfail will go away if I change this:
>>
>> get_string_length (arg, dir.modifier == FMT_LEN_l ? 4 : 1);
>>
>> to:
> 
>> get_string_length (arg, dir.specifier == 'S' dir.modifier == FMT_LEN_l ? 4 : 
>> 1);
> Presumably  "||" is missing here.
> 

Yes, and on another line, I confirmed that it works, and removes the xfail.


>>
>>
>> That should be easy, I will change the patch to match your patch in that
>> area.
>>
>>
>> The other xfail with memcmp needs to be fixed separately.
>> The issue is, callers of string_constant that do not pass the mem_size
>> parameter may access up to TREE_STRING_LENGTH byte, therefore
>> returning longer strings is not appropriate: the string constant
>> contains 9 bytes, mind the terminating nul.
> If we make the patch to add the size parameter to c_strlen independent
> of the changes related to 86711/86714, then we don't need to xfail or
> change the testsuite in any way shape or form.
> 
> My overall thinking is to carve out this patch so that it's independent
> of anything else.  That's actually easy to do after addressing two very
> minor issues.
> 

I can give it a try, but I think that 86711/86714 is only a manifestation
of the design issues, and it is probably impossible to fix the design
without fixing 86711/86714 at the same time.  It is likely not the only
one IMHO.

They are due to string_constant, c_strlen and c_getstr returning data that is
invalid in some corner cases, like over-length string constants.  These were
previously only happening rarely, but due to the recent enhancements, much more
corner cases were added, and some of them trigger those issues now.


> We can then add Martin's patch to fix handling of wide chars in sprintf
> (86853).
> 

Yes, I think that seems likely an independent issue.  If done properly it
will probably not even be a merge conflict.

> --
> 
> This overall plan addresses the need to be able to tell c_strlen how to
> count and rev

[PATCH, testsuite]: Loosen scan-assembler strings in gcc.target/i386/avx{,2}-cvt-2.c

[Uros Bizjak]

Hello!

These instructions can take memory operands and current scan-assembler
strings were too tight to accept them.

2018-08-16  Uros Bizjak  

PR testsuite/86745
* gcc.target/i386/avx-cvt-2.c: Loosen scan-assembler strings.
* gcc.target/i386/avx2-cvt-2.c: Ditto.

Tested on x86_64-linux-gnu {,-m32} and committed to mainline SVN.

Uros.
diff --git a/gcc/testsuite/gcc.target/i386/avx-cvt-2.c 
b/gcc/testsuite/gcc.target/i386/avx-cvt-2.c
index 1fbcf6e..672ce06 100644
--- a/gcc/testsuite/gcc.target/i386/avx-cvt-2.c
+++ b/gcc/testsuite/gcc.target/i386/avx-cvt-2.c
@@ -4,9 +4,9 @@
 #include "avx-cvt-1.c"
 
 /* { dg-final { scan-tree-dump-times "vectorized 1 loops in function" 6 "vect" 
} } */
-/* { dg-final { scan-assembler 
"vcvttpd2dq(y\[^\n\r\]*%xmm|\[^\n\r\]*xmm\[^\n\r\]*YMMWORD PTR)" } } */
-/* { dg-final { scan-assembler "vcvtdq2ps\[^\n\r\]*ymm" } } */
-/* { dg-final { scan-assembler 
"vcvtps2pd\[^\n\r\]*(%xmm\[^\n\r\]*%ymm|ymm\[^\n\r\]*xmm)" } } */
-/* { dg-final { scan-assembler "vcvttps2dq\[^\n\r\]*ymm" } } */
-/* { dg-final { scan-assembler 
"vcvtdq2pd\[^\n\r\]*(%xmm\[^\n\r\]*%ymm|ymm\[^\n\r\]*xmm)" } } */
-/* { dg-final { scan-assembler 
"vcvtpd2ps(y\[^\n\r\]*%xmm|\[^\n\r\]*xmm\[^\n\r\]*YMMWORD PTR)" } } */
+/* { dg-final { scan-assembler "vcvttpd2dq" } } */
+/* { dg-final { scan-assembler "vcvtdq2ps" } } */
+/* { dg-final { scan-assembler "vcvtps2pd" } } */
+/* { dg-final { scan-assembler "vcvttps2dq" } } */
+/* { dg-final { scan-assembler "vcvtdq2pd" } } */
+/* { dg-final { scan-assembler "vcvtpd2ps" } } */
diff --git a/gcc/testsuite/gcc.target/i386/avx2-cvt-2.c 
b/gcc/testsuite/gcc.target/i386/avx2-cvt-2.c
index d37809d..633a1cd 100644
--- a/gcc/testsuite/gcc.target/i386/avx2-cvt-2.c
+++ b/gcc/testsuite/gcc.target/i386/avx2-cvt-2.c
@@ -4,9 +4,9 @@
 #include "avx2-cvt-1.c"
 
 /* { dg-final { scan-tree-dump-times "vectorized 1 loops in function" 6 "vect" 
} } */
-/* { dg-final { scan-assembler 
"vcvttpd2dq(y\[^\n\r\]*%xmm|\[^\n\r\]*xmm\[^\n\r\]*YMMWORD PTR)" } } */
-/* { dg-final { scan-assembler "vcvtdq2ps\[^\n\r\]*ymm" } } */
-/* { dg-final { scan-assembler 
"vcvtps2pd\[^\n\r\]*(%xmm\[^\n\r\]*%ymm|ymm\[^\n\r\]*xmm)" } } */
-/* { dg-final { scan-assembler "vcvttps2dq\[^\n\r\]*ymm" } } */
-/* { dg-final { scan-assembler 
"vcvtdq2pd\[^\n\r\]*(%xmm\[^\n\r\]*%ymm|ymm\[^\n\r\]*xmm)" } } */
-/* { dg-final { scan-assembler 
"vcvtpd2ps(y\[^\n\r\]*%xmm|\[^\n\r\]*xmm\[^\n\r\]*YMMWORD PTR)" } } */
+/* { dg-final { scan-assembler "vcvttpd2dq" } } */
+/* { dg-final { scan-assembler "vcvtdq2ps" } } */
+/* { dg-final { scan-assembler "vcvtps2pd" } } */
+/* { dg-final { scan-assembler "vcvttps2dq" } } */
+/* { dg-final { scan-assembler "vcvtdq2pd" } } */
+/* { dg-final { scan-assembler "vcvtpd2ps" } } */

[PATCH] Macro definition parameter parsing

[Nathan Sidwell]

This next patch in the macro cleanup changes the internal interface to 
the parameter parsing logic.  Rather than pass a macro pointer in, we 
pass explicit variable pointers in.  This'll allow later creation of the 
macro object itself.


While there, I cleaned up the parsing logic for more consistent error 
messages -- hence the changes to the two testcases.


booted & tested on x86_64-linux.

nathan
--
Nathan Sidwell
2018-08-16  Nathan Sidwell  

	libcpp/
	* internal.h (_cpp_save_parameter): Take parmno, not macro.
	(_cpp_unsave_parameters): Declare.
	* macro.c (_cpp_save_parameter): Take parm number, not macro.
	Return true on success.
	(_cpp_unsave_parameters): New.
	(parse_params): Take parm_no and variadic pointers, not macro.
	Reimplement parsing logic.
	(create_iso_definition): Adjust parse_params changes.  Call
	_cpp_unsave_parameters here.
	(_cpp_create_definition): Don't unsave params here.
	* traditional.c (scan_parameters): Take n_param pointer, adjust.
	(_cpp_create_trad_definition): Ajust scan_parameters change.  Call
	_cpp_unsave_parameters.
	gcc/testsuite/
	* gcc.dg/cpp/macsyntx.c: Adjust expected errors.
	* gcc.dg/cpp/macsyntx2.c: likewise.

Index: libcpp/internal.h
===
--- libcpp/internal.h	(revision 263587)
+++ libcpp/internal.h	(working copy)
@@ -632,8 +632,9 @@ extern bool _cpp_create_definition (cpp_
 extern void _cpp_pop_context (cpp_reader *);
 extern void _cpp_push_text_context (cpp_reader *, cpp_hashnode *,
 const unsigned char *, size_t);
-extern bool _cpp_save_parameter (cpp_reader *, cpp_macro *, cpp_hashnode *,
+extern bool _cpp_save_parameter (cpp_reader *, unsigned, cpp_hashnode *,
  cpp_hashnode *);
+extern void _cpp_unsave_parameters (cpp_reader *, unsigned);
 extern bool _cpp_arguments_ok (cpp_reader *, cpp_macro *, const cpp_hashnode *,
 			   unsigned int);
 extern const unsigned char *_cpp_builtin_macro_text (cpp_reader *,
Index: libcpp/macro.c
===
--- libcpp/macro.c	(revision 263587)
+++ libcpp/macro.c	(working copy)
@@ -316,7 +316,7 @@ static cpp_token *alloc_expansion_token
 static cpp_token *lex_expansion_token (cpp_reader *, cpp_macro *);
 static bool warn_of_redefinition (cpp_reader *, cpp_hashnode *,
   const cpp_macro *);
-static bool parse_params (cpp_reader *, cpp_macro *);
+static bool parse_params (cpp_reader *, unsigned *, bool *);
 static void check_trad_stringification (cpp_reader *, const cpp_macro *,
 	const cpp_string *);
 static bool reached_end_of_context (cpp_context *);
@@ -3053,119 +3053,158 @@ _cpp_free_definition (cpp_hashnode *h)
 }
 
 /* Save parameter NODE (spelling SPELLING) to the parameter list of
-   macro MACRO.  Returns zero on success, nonzero if the parameter is
-   a duplicate.  */
+   macro MACRO.  Returns true on success, false on failure.   */
 bool
-_cpp_save_parameter (cpp_reader *pfile, cpp_macro *macro, cpp_hashnode *node,
+_cpp_save_parameter (cpp_reader *pfile, unsigned n, cpp_hashnode *node,
 		 cpp_hashnode *spelling)
 {
-  unsigned int len;
   /* Constraint 6.10.3.6 - duplicate parameter names.  */
   if (node->flags & NODE_MACRO_ARG)
 {
   cpp_error (pfile, CPP_DL_ERROR, "duplicate macro parameter \"%s\"",
 		 NODE_NAME (node));
-  return true;
+  return false;
 }
 
-  if (BUFF_ROOM (pfile->a_buff)
-  < (macro->paramc + 1) * sizeof (cpp_hashnode *))
-_cpp_extend_buff (pfile, &pfile->a_buff, sizeof (cpp_hashnode *));
-
-  ((cpp_hashnode **) BUFF_FRONT (pfile->a_buff))[macro->paramc++] = spelling;
-  node->flags |= NODE_MACRO_ARG;
-  len = macro->paramc * sizeof (struct macro_arg_saved_data);
+  unsigned len = (n + 1) * sizeof (struct macro_arg_saved_data);
   if (len > pfile->macro_buffer_len)
 {
-  pfile->macro_buffer = XRESIZEVEC (unsigned char, pfile->macro_buffer,
-len);
+  pfile->macro_buffer
+	= XRESIZEVEC (unsigned char, pfile->macro_buffer, len);
   pfile->macro_buffer_len = len;
 }
-  struct macro_arg_saved_data save;
-  save.value = node->value;
-  save.canonical_node = node;
-  ((struct macro_arg_saved_data *) pfile->macro_buffer)[macro->paramc - 1]
-= save;
   
-  node->value.arg_index  = macro->paramc;
-  return false;
+  macro_arg_saved_data *saved = (macro_arg_saved_data *)pfile->macro_buffer;
+  saved[n].canonical_node = node;
+  saved[n].value = node->value;
+
+  if (BUFF_ROOM (pfile->a_buff) < (n + 1) * sizeof (cpp_hashnode *))
+_cpp_extend_buff (pfile, &pfile->a_buff, sizeof (cpp_hashnode *));
+
+  ((cpp_hashnode **) BUFF_FRONT (pfile->a_buff))[n] = spelling;
+
+  /* Morph into a macro arg.  */
+  node->flags |= NODE_MACRO_ARG;
+  /* Index is 1 based.  */
+  node->value.arg_index = n + 1;
+
+  return true;
+}
+
+/* Restore the parameters to their previous state.  */
+void
+_cpp_unsave_parameters (cpp_reader *pfile, unsigned n)
+{
+  /* Clear the fast ar

[PATCH] RFC: reduce likelihood of fully-dynamic-string throwing on move

[Jonathan Wakely]

With --enable-fully-dynamic-string the COW basic_string move
constructor is noexcept(false), because it has to allocate a new empty
rep for the moved-from string.

If we did this, it would only throw when the string we're moving from
is "leaked" (that is, there are potentially pointers, references or
iterators into the string that prevent it being shared):

--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -3575,7 +3575,10 @@ _GLIBCXX_END_NAMESPACE_CXX11
#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
   __str._M_data(_S_empty_rep()._M_refdata());
#else
-   __str._M_data(_S_construct(size_type(), _CharT(), get_allocator()));
+   if (__str._M_is_leaked())
+ __str._M_data(_S_construct(size_type(), _CharT(), get_allocator()));
+   else
+ (void) _M_rep()->_M_refcopy();
#endif
  }

If the string is sharable then a move can just increase the refcount,
which won't throw.

This would break the guarantee that a moved-from string is empty, but
the standard doesn't guarantee that anyway, and we don't leave COW
strings empty after move assignment.

Thoughts?

Re: [PATCH, Darwin, drivers] Split DWARF is not supported on Darwin.

[Mike Stump]

On Aug 16, 2018, at 6:55 AM, Iain Sandoe  wrote:
> 
> The Darwin toolchains have a separate debug linker (dsymutil) so that the
> link-time penalty for debug data is not usually seen.  At present, it's not
> clear how we would support split DWARF on Darwin (or if it would bring
> any additional benefit over dsymutil).
> 
> This patch produces diagnostic output to note that it's not supported and
> disables it.  We also skip test cases that invoke -gsplit-dwarf.
> 
> NOTE that the tests failed because of PR81685 and this fix will need to
> be back-ported after the fix for 81685.
> 
> OK for trunk?

Ok.

> affected branches (7,8)?

Ok.

Re: [PATCH, RFC, rs6000, v2] folding of vec_splat

[Segher Boessenkool]

Hi Will,

On Thu, Aug 16, 2018 at 10:50:45AM -0500, Will Schmidt wrote:
> 2018-08-16  Will Schmidt  
> 
>   * config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add support for
> early gimple folding of vec_splat().

Continuation lines should be indented to the *, not to the text after it.

> + /* Only fold the vec_splat_*() if arg1 is a constant
> +5-bit unsigned literal.  */
> + if (TREE_CODE (arg1) != INTEGER_CST || TREE_INT_CST_LOW (arg1) > 0x1f)
> +   return false;

Does this need to check for negative as well?  So something with IN_RANGE
then.

> + /* force arg1 into range.  */
> + tree new_arg1 = build_int_cst (arg1_type,
> +TREE_INT_CST_LOW (arg1) % n_elts);

Or is the range check unnecessary completely, since you have this?

> + tree splat;
> + if (TREE_CODE (arg0) == VECTOR_CST)
> +   splat = VECTOR_CST_ELT (arg0, TREE_INT_CST_LOW (new_arg1));
> + else
> +   {
> + /* Determine (in bits) the length and start location of the
> +splat value for a call to the tree_vec_extract helper.  */
> + int tree_size_in_bits = TREE_INT_CST_LOW (size_in_bytes (arg0_type))
> + * BITS_PER_UNIT;
> + int splat_elem_size = tree_size_in_bits / n_elts;
> + int splat_start_bit = TREE_INT_CST_LOW (new_arg1) * splat_elem_size;
> + /* Do not attempt to early-fold if the size + specified offset into
> +the vector would touch outside of the source vector.  */
> + tree len = build_int_cst (bitsizetype, splat_elem_size);
> + tree start = build_int_cst (bitsizetype, splat_start_bit);
> + splat = tree_vec_extract (gsi, TREE_TYPE (lhs_type), arg0,
> +   len, start);
> + }

This closing brace should be indented two spaces more.

> -static inline tree
> +tree
>  tree_vec_extract (gimple_stmt_iterator *gsi, tree type,
> tree t, tree bitsize, tree bitpos)

It could use a comment, too (what the args are, etc.)

Other than those nits, looks fine to me.  Maybe Richard or Bill have
more comments?


Segher

Re: [PATCH] Add a character size parameter to c_strlen/get_range_strlen

[Jeff Law]

On 08/16/2018 12:47 PM, Bernd Edlinger wrote:

 Parameterizing the function to return either the number of
 bytes or the number of elements makes sense as an enhancement.
 It makes less sense (and could be the source of bugs) to let
 callers pass in anything else.  A boolean flag to toggle
 between the two alternatives would make the API narrower
 and safer to use.  It doesn't seem necessary to fix any
 bugs.
>> No bugs that we're currently aware of. But ensuring that routines that
>> expect to be counting bytes are counting bytes is a good thing.  If some
>> pass (say forwprop) changed the char * argument to an array of something
>> other than chars then we'd likely end up giving a wrong result.
>>
>> I don't mind making the interface narrower with a boolean or an enum to
>> specify how we count rather than having the caller pass in a type.
>>
>>
> 
> Yes, the caller _must_ know what to expect.
> The function should not try to be smarter than the caller.
So I have a tweaked implementation here which uses a boolean.  I
actually don't like it much.  I think it's largely because true/false is
just a bad match for the objects and what we're doing.

It's not like something is on or off -- we're selecting between sizes.

An enum would work, but it just seems like overkill.  So I'm leaning
back towards just having the caller pass it in as an integer.  We can
have c_strlen verify the argument for sanity.

>>
>>> get_string_length (arg, dir.specifier == 'S' dir.modifier == FMT_LEN_l ? 4 
>>> : 1);
>> Presumably  "||" is missing here.
>>
> 
> Yes, and on another line, I confirmed that it works, and removes the xfail.
Similarly.  I actually had all this working last night before sending
out the message.


> 
> 
>>>
>>>
>>> That should be easy, I will change the patch to match your patch in that
>>> area.
>>>
>>>
>>> The other xfail with memcmp needs to be fixed separately.
>>> The issue is, callers of string_constant that do not pass the mem_size
>>> parameter may access up to TREE_STRING_LENGTH byte, therefore
>>> returning longer strings is not appropriate: the string constant
>>> contains 9 bytes, mind the terminating nul.
>> If we make the patch to add the size parameter to c_strlen independent
>> of the changes related to 86711/86714, then we don't need to xfail or
>> change the testsuite in any way shape or form.
>>
>> My overall thinking is to carve out this patch so that it's independent
>> of anything else.  That's actually easy to do after addressing two very
>> minor issues.
>>
> 
> I can give it a try, but I think that 86711/86714 is only a manifestation
> of the design issues, and it is probably impossible to fix the design
> without fixing 86711/86714 at the same time.  It is likely not the only
> one IMHO.
You don't need to do the work -- I've already got all the bits here.  I
don't mind taking care of the final post/commit.


> 
> They are due to string_constant, c_strlen and c_getstr returning data that is
> invalid in some corner cases, like over-length string constants.  These were
> previously only happening rarely, but due to the recent enhancements, much 
> more
> corner cases were added, and some of them trigger those issues now.
Let's not go down this yet :-)


> 
> 
>> We can then add Martin's patch to fix handling of wide chars in sprintf
>> (86853).
>>
> 
> Yes, I think that seems likely an independent issue.  If done properly it
> will probably not even be a merge conflict.
It doesn't cause any conflicts.  I've already verified that.

> 
>> --
>>
>> This overall plan addresses the need to be able to tell c_strlen how to
>> count and reverts its behavior for callers outside of the sprintf
>> warning pass to historical (pre-gcc7) behavior of counting bytes by
>> default.  It does this without losing any warnings and allow us to move
>> forward with Martin's more complete fix for 86853.
>>
> 
> Yes, I have already started to write a similar patch on c_getstr, which
> seems to fix the other xfail.
> 
> The issue with c_getstr is similar, some call it with one parameter,
> those expect a zero terminated string, that needs to be valid.
> Others use a two parameter overload, which retuns the memory length,
> those use memcmp or memchr, and need no zero termination.
> 
> But I start to think that I should merge all three patches into one,
> to avoid unnecessary confusion.
Let's wait and please let's try to avoid mashing too many things
together.  I expect to start looking at the next round of stuff tomorrow
or Monday.

Jeff

Re: [PATCH] Add a character size parameter to c_strlen/get_range_strlen

[Bernd Edlinger]

On 08/16/18 23:27, Jeff Law wrote:
> On 08/16/2018 12:47 PM, Bernd Edlinger wrote:
> 
> Parameterizing the function to return either the number of
> bytes or the number of elements makes sense as an enhancement.
> It makes less sense (and could be the source of bugs) to let
> callers pass in anything else.  A boolean flag to toggle
> between the two alternatives would make the API narrower
> and safer to use.  It doesn't seem necessary to fix any
> bugs.
>>> No bugs that we're currently aware of. But ensuring that routines that
>>> expect to be counting bytes are counting bytes is a good thing.  If some
>>> pass (say forwprop) changed the char * argument to an array of something
>>> other than chars then we'd likely end up giving a wrong result.
>>>
>>> I don't mind making the interface narrower with a boolean or an enum to
>>> specify how we count rather than having the caller pass in a type.
>>>
>>>
>>
>> Yes, the caller _must_ know what to expect.
>> The function should not try to be smarter than the caller.
> So I have a tweaked implementation here which uses a boolean.  I
> actually don't like it much.  I think it's largely because true/false is
> just a bad match for the objects and what we're doing.
> 
> It's not like something is on or off -- we're selecting between sizes.
> 
> An enum would work, but it just seems like overkill.  So I'm leaning
> back towards just having the caller pass it in as an integer.  We can
> have c_strlen verify the argument for sanity.
> 
>>>
 get_string_length (arg, dir.specifier == 'S' dir.modifier == FMT_LEN_l ? 4 
 : 1);
>>> Presumably  "||" is missing here.
>>>
>>
>> Yes, and on another line, I confirmed that it works, and removes the xfail.
> Similarly.  I actually had all this working last night before sending
> out the message.
> 
> 
>>
>>


 That should be easy, I will change the patch to match your patch in that
 area.


 The other xfail with memcmp needs to be fixed separately.
 The issue is, callers of string_constant that do not pass the mem_size
 parameter may access up to TREE_STRING_LENGTH byte, therefore
 returning longer strings is not appropriate: the string constant
 contains 9 bytes, mind the terminating nul.
>>> If we make the patch to add the size parameter to c_strlen independent
>>> of the changes related to 86711/86714, then we don't need to xfail or
>>> change the testsuite in any way shape or form.
>>>
>>> My overall thinking is to carve out this patch so that it's independent
>>> of anything else.  That's actually easy to do after addressing two very
>>> minor issues.
>>>
>>
>> I can give it a try, but I think that 86711/86714 is only a manifestation
>> of the design issues, and it is probably impossible to fix the design
>> without fixing 86711/86714 at the same time.  It is likely not the only
>> one IMHO.
> You don't need to do the work -- I've already got all the bits here.  I
> don't mind taking care of the final post/commit.
> 
> 
>>
>> They are due to string_constant, c_strlen and c_getstr returning data that is
>> invalid in some corner cases, like over-length string constants.  These were
>> previously only happening rarely, but due to the recent enhancements, much 
>> more
>> corner cases were added, and some of them trigger those issues now.
> Let's not go down this yet :-)
> 
> 
>>
>>
>>> We can then add Martin's patch to fix handling of wide chars in sprintf
>>> (86853).
>>>
>>
>> Yes, I think that seems likely an independent issue.  If done properly it
>> will probably not even be a merge conflict.
> It doesn't cause any conflicts.  I've already verified that.
> 
>>
>>> --
>>>
>>> This overall plan addresses the need to be able to tell c_strlen how to
>>> count and reverts its behavior for callers outside of the sprintf
>>> warning pass to historical (pre-gcc7) behavior of counting bytes by
>>> default.  It does this without losing any warnings and allow us to move
>>> forward with Martin's more complete fix for 86853.
>>>
>>
>> Yes, I have already started to write a similar patch on c_getstr, which
>> seems to fix the other xfail.
>>
>> The issue with c_getstr is similar, some call it with one parameter,
>> those expect a zero terminated string, that needs to be valid.
>> Others use a two parameter overload, which retuns the memory length,
>> those use memcmp or memchr, and need no zero termination.
>>
>> But I start to think that I should merge all three patches into one,
>> to avoid unnecessary confusion.
> Let's wait and please let's try to avoid mashing too many things
> together.  I expect to start looking at the next round of stuff tomorrow
> or Monday.
> 

Don't worry, I did not mean to merge that with the varasm patch(es).

Those are currently broken by Martin's PR 71625 :-( because this does
create lots of non-zero terminated string constants.

Rather the opposite, strictly speaking the change in c_getstr was
in anticipation of the guarante

Re: [ARM/FDPIC v2 00/21] FDPIC ABI for ARM

[Christophe Lyon]

Ping?

Le mer. 1 août 2018 à 10:03, Christophe Lyon  a
écrit :

> Ping?
>
>
> On 13/07/2018 18:10, christophe.l...@st.com wrote:
> > From: Christophe Lyon 
> >
> > Hello,
> >
> > This patch series implements the GCC contribution of the FDPIC ABI for
> > ARM targets.
> >
> > This ABI enables to run Linux on ARM MMU-less cores and supports
> > shared libraries to reduce the memory footprint.
> >
> > Without MMU, text and data segments relative distances are different
> > from one process to another, hence the need for a dedicated FDPIC
> > register holding the start address of the data segment. One of the
> > side effects is that function pointers require two words to be
> > represented: the address of the code, and the data segment start
> > address. These two words are designated as "Function Descriptor",
> > hence the "FD PIC" name.
> >
> > On ARM, the FDPIC register is r9 [1], and the target name is
> > arm-uclinuxfdpiceabi. Note that arm-uclinux exists, but uses another
> > ABI and the BFLAT file format; it does not support code sharing.
> > The -mfdpic option is enabled by default, and -mno-fdpic should be
> > used to build the Linux kernel.
> >
> > This work was developed some time ago by STMicroelectronics, and was
> > presented during Linaro Connect SFO15 (September 2015). You can watch
> > the discussion and read the slides [2].
> > This presentation was related to the toolchain published on github [3],
> > which is based on binutils-2.22, gcc-4.7, uclibc-0.9.33.2, gdb-7.5.1
> > and qemu-2.3.0, and for which pre-built binaries are available [3].
> >
> > The ABI itself is described in details in [1].
> >
> > Our Linux kernel patches have been updated and committed by Nicolas
> > Pitre (Linaro) in July 2017. They are required so that the loader is
> > able to handle this new file type. Indeed, the ELF files are tagged
> > with ELFOSABI_ARM_FDPIC. This new tag has been allocated by ARM, as
> > well as the new relocations involved.
> >
> > The binutils and QEMU patch series have been merged recently. [4][5]
> >
> > To build such a toolchain, you'd also need to use my uClibc branch[6].
> > I have posted uclibc-ng patches for review [7]
> >
> > I am currently working on updating the patches for the remaining
> > toolchain components: uclibc and gdb.
> >
> > This series provides support for ARM v7 architecture and has been
> > tested on arm-linux-gnueabi without regression, as well as
> > arm-uclinuxfdpiceabi, using QEMU. arm-uclinuxfdpiceabi has more
> > failures than arm-linux-gnueabi, but is quite functional.
> >
> > Are the GCC patches OK for inclusion in master?
> >
> > Changes between v1 and v2:
> > - fix GNU coding style
> > - exit with an error for pre-Armv7
> > - use ACLE __ARM_ARCH and remove dead code for pre-Armv4
> > - remove unsupported attempts of pre-Armv7/thumb1 support
> > - add instructions in comments next to opcodes
> > - merge patches 11 and 13
> > - fixed protected visibility handling in patch 8
> > - merged legitimize_tls_address_fdpic and
> >legitimize_tls_address_not_fdpic as requested
> >
> > Thanks,
> >
> > Christophe.
> >
> >
> > [1] https://github.com/mickael-guene/fdpic_doc/blob/master/abi.txt
> > [2]
> http://connect.linaro.org/resource/sfo15/sfo15-406-arm-fdpic-toolset-kernel-libraries-for-cortex-m-cortex-r-mmuless-cores/
> > [3] https://github.com/mickael-guene/fdpic_manifest
> > [4]
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=f1ac0afe481e83c9a33f247b81fa7de789edc4d9
> > [5]
> https://git.qemu.org/?p=qemu.git;a=commit;h=e8fa72957419c11984608062c7dcb204a6003a06
> > [6]
> https://git.linaro.org/people/christophe.lyon/uclibc.git/log/?h=uClibc-0.9.33.2-fdpic-upstream
> > [7] https://mailman.uclibc-ng.org/pipermail/devel/2018-July/001705.html
> >
> > Christophe Lyon (21):
> >[ARM] FDPIC: Add -mfdpic option support
> >[ARM] FDPIC: Handle arm*-*-uclinuxfdpiceabi in configure scripts
> >[ARM] FDPIC: Force FDPIC related options unless -mno-fdpic is provided
> >[ARM] FDPIC: Add support for FDPIC for arm architecture
> >[ARM] FDPIC: Fix __do_global_dtors_aux and frame_dummy generation
> >[ARM] FDPIC: Add support for c++ exceptions
> >[ARM] FDPIC: Avoid saving/restoring r9 on stack since it is RO
> >[ARM] FDPIC: Ensure local/global binding for function descriptors
> >[ARM] FDPIC: Add support for taking address of nested function
> >[ARM] FDPIC: Implement TLS support.
> >[ARM] FDPIC: Add support to unwind FDPIC signal frame
> >[ARM] FDPIC: Restore r9 after we call __aeabi_read_tp
> >[ARM] FDPIC: Force LSB bit for PC in Cortex-M architecture
> >[ARM][testsuite] FDPIC: Skip unsupported tests
> >[ARM][testsuite] FDPIC: Adjust scan-assembler patterns.
> >[ARM][testsuite] FDPIC: Skip v8-m and v6-m tests that currently
> >  produce an ICE
> >[ARM][testsuite] FDPIC: Skip tests that don't work in PIC mode
> >[ARM][testsuite] FDPIC: Handle *-*-uclinux*
> >[ARM][testsuite] FDPIC: Enable

[committed] diagnostics: tweak to line-insertion fix-it hints with line-numbering

[David Malcolm]

This commit slightly tweaks line-insertion fix-it hints, so that
with line-numbering, rather than e.g.:

 99 |   x = a;
|+  break;
110 | case 'b':
| ^~~~

we fill the margin with "+":

 99 |   x = a;
+++ |+  break;
110 | case 'b':
| ^~~~

to emphasize that this is a suggested new line, rather than the user's
source.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

Committed to trunk as r263605.

gcc/ChangeLog:
* diagnostic-show-locus.c (layout::start_annotation_line): Add
"margin_char" parameter, defaulting to space.  Use it in place
of pp_space for the initial part of the margin.
(layout::print_leading_fixits): Use '+' when filling the margin
of line-insertion fix-it hints.

gcc/testsuite/ChangeLog:
* gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c
(test_fixit_insert_newline): Update expected output to show '+'
characters in margin of line-insertion fix-it hint.
---
 gcc/diagnostic-show-locus.c   | 8 
 .../gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c| 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index c9edaab..a759826 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -253,7 +253,7 @@ class layout
   void print_source_line (linenum_type row, const char *line, int line_width,
  line_bounds *lbounds_out);
   bool should_print_annotation_line_p (linenum_type row) const;
-  void start_annotation_line () const;
+  void start_annotation_line (char margin_char = ' ') const;
   void print_annotation_line (linenum_type row, const line_bounds lbounds);
   void print_any_labels (linenum_type row);
   void print_trailing_fixits (linenum_type row);
@@ -1330,12 +1330,12 @@ layout::should_print_annotation_line_p (linenum_type 
row) const
margin, which is empty for annotation lines.  Otherwise, do nothing.  */
 
 void
-layout::start_annotation_line () const
+layout::start_annotation_line (char margin_char) const
 {
   if (m_show_line_numbers_p)
 {
   for (int i = 0; i < m_linenum_width; i++)
-   pp_space (m_pp);
+   pp_character (m_pp, margin_char);
   pp_string (m_pp, " |");
 }
 }
@@ -1587,7 +1587,7 @@ layout::print_leading_fixits (linenum_type row)
 helps them stand out from each other, and from
 the surrounding text.  */
  m_colorizer.set_normal_text ();
- start_annotation_line ();
+ start_annotation_line ('+');
  pp_character (m_pp, '+');
  m_colorizer.set_fixit_insert ();
  /* Print all but the trailing newline of the fix-it hint.
diff --git 
a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c 
b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c
index 89213eb..f2bbc58 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw-line-numbers.c
@@ -111,7 +111,7 @@ void test_fixit_insert_newline (void)
   x = b;
 }
 /* { dg-begin-multiline-output "" }
-|+  break;
 |+  break;
 110 | case 'b':
 | ^~~~
{ dg-end-multiline-output "" } */
-- 
1.8.5.3

[committed] diagnostics: fix bad interaction between line spans and line numbers

[David Malcolm]

Without this patch, the "line span" markers and the line numbering
interacted badly, leading to stray copies of the line-span markers
appearing as prefixes on the first source line in a span:

missing-header-fixit-3.c: In function 'test':
missing-header-fixit-3.c:9:3: warning: implicit declaration of function 
'printf' [-Wimplicit-function-declaration]
9 |   printf ("%i of %i\n", i, j);
  |   ^~
missing-header-fixit-3.c:9:3: warning: incompatible implicit declaration of 
built-in function 'printf'
missing-header-fixit-3.c:9:3: note: include '' or provide a 
declaration of 'printf'
missing-header-fixit-3.c:1:1:
  |+#include 
missing-header-fixit-3.c:1:1:1 | /* Example of a fix-it hint that adds a 
#include directive,
missing-header-fixit-3.c:9:3:
missing-header-fixit-3.c:9:3:9 |   printf ("%i of %i\n", i, j);
  |   ^~

With this patch, we now correctly print:

missing-header-fixit-3.c: In function 'test':
missing-header-fixit-3.c:9:3: warning: implicit declaration of function 
'printf' [-Wimplicit-function-declaration]
9 |   printf ("%i of %i\n", i, j);
  |   ^~
missing-header-fixit-3.c:9:3: warning: incompatible implicit declaration of 
built-in function 'printf'
missing-header-fixit-3.c:9:3: note: include '' or provide a 
declaration of 'printf'
missing-header-fixit-3.c:1:1:
+ |+#include 
1 | /* Example of a fix-it hint that adds a #include directive,
missing-header-fixit-3.c:9:3:
9 |   printf ("%i of %i\n", i, j);
  |   ^~

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

Committed to trunk as r263606.

gcc/ChangeLog:
* diagnostic.c (default_diagnostic_start_span_fn): Call pp_string
to emit the span, rather than setting it as the prefix.

gcc/testsuite/ChangeLog:
* gcc.dg/missing-header-fixit-3.c: New test.
---
 gcc/diagnostic.c  |  6 +++---
 gcc/testsuite/gcc.dg/missing-header-fixit-3.c | 27 +++
 2 files changed, 30 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/missing-header-fixit-3.c

diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 59477ce..7e8bcf5 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -629,9 +629,9 @@ void
 default_diagnostic_start_span_fn (diagnostic_context *context,
  expanded_location exploc)
 {
-  pp_set_prefix (context->printer,
-diagnostic_get_location_text (context, exploc));
-  pp_string (context->printer, "");
+  char *text = diagnostic_get_location_text (context, exploc);
+  pp_string (context->printer, text);
+  free (text);
   pp_newline (context->printer);
 }
 
diff --git a/gcc/testsuite/gcc.dg/missing-header-fixit-3.c 
b/gcc/testsuite/gcc.dg/missing-header-fixit-3.c
new file mode 100644
index 000..8f2fb5b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/missing-header-fixit-3.c
@@ -0,0 +1,27 @@
+/* Example of a fix-it hint that adds a #include directive,
+   adding them to the top of the file, given that there is no
+   pre-existing #include.  */
+
+/* { dg-options "-fdiagnostics-show-caret -fdiagnostics-show-line-numbers" } */
+
+void test (int i, int j)
+{
+  printf ("%i of %i\n", i, j); /* { dg-warning "implicit declaration" } */
+  /* { dg-message "include '' or provide a declaration of 'printf'" 
"" { target *-*-* } .-1 } */
+#if 0
+/* { dg-begin-multiline-output "" }
+9 |   printf ("%i of %i\n", i, j);
+  |   ^~
+   { dg-end-multiline-output "" } */
+/* { dg-regexp ".*missing-header-fixit-3.c:1:1:" } */
+/* { dg-begin-multiline-output "" }
++ |+#include 
+1 | /* Example of a fix-it hint that adds a #include directive,
+   { dg-end-multiline-output "" } */
+/* { dg-regexp ".*missing-header-fixit-3.c:9:3:" } */
+/* { dg-begin-multiline-output "" }
+9 |   printf ("%i of %i\n", i, j);
+  |   ^~
+   { dg-end-multiline-output "" } */
+#endif
+}
-- 
1.8.5.3

Re: [PATCH] Add a character size parameter to c_strlen/get_range_strlen

[Jeff Law]

On 08/14/2018 04:25 AM, Bernd Edlinger wrote:
> 2018-08-14  Bernd Edlinger  
> 
>   * builtins.c (c_strlen): Add new parameter eltsize.
>   * builtins.h (c_strlen): Adjust prototype.
>   * expr.c (string_constant): Add new parameter mem_size.
>   * expr.h (string_constant): Adjust protoype.
>   * gimple-fold.c (get_range_strlen): Add new parameter eltsize.
>   * gimple-fold.h (get_range_strlen): Adjust prototype.
>   * gimple-ssa-sprintf.c (get_string_length): Add new parameter eltsize.
>   (format_string): Call get_string_length with eltsize.
So as I mentioned downthread, I made some minor tweaks.  Essentially
disentangling this from the prior patch to fix 86711/86714.  I don't
believe my changes affect this patch in any material way.

The key take-away from this patch is that callers of c_strlen now pass
to c_strlen how to count elements (1-byte, 2-byte or 4-byte chunks).
All the callers except the sprintf warning code count single bytes which
restores previous behavior.  The sprintf bits want to count element
sized chunks, which for wchars is 4 bytes (that count will then be
adjusted for multibyte-chars during analysis).

The patch requires no testsuite changes.

Installing on the trunk.

Jeff
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 6716aab..b1a79f3 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -568,41 +568,43 @@ string_length (const void *ptr, unsigned eltsize, 
unsigned maxelts)
accesses.  Note that this implies the result is not going to be emitted
into the instruction stream.
 
-   The value returned is of type `ssizetype'.
+   ELTSIZE is 1 for normal single byte character strings, and 2 or
+   4 for wide characer strings.  ELTSIZE is by default 1.
 
-   Unfortunately, string_constant can't access the values of const char
-   arrays with initializers, so neither can we do so here.  */
+   The value returned is of type `ssizetype'.  */
 
 tree
-c_strlen (tree src, int only_value)
+c_strlen (tree src, int only_value, unsigned eltsize)
 {
+  gcc_assert (eltsize == 1 || eltsize == 2 || eltsize == 4);
   STRIP_NOPS (src);
   if (TREE_CODE (src) == COND_EXPR
   && (only_value || !TREE_SIDE_EFFECTS (TREE_OPERAND (src, 0
 {
   tree len1, len2;
 
-  len1 = c_strlen (TREE_OPERAND (src, 1), only_value);
-  len2 = c_strlen (TREE_OPERAND (src, 2), only_value);
+  len1 = c_strlen (TREE_OPERAND (src, 1), only_value, eltsize);
+  len2 = c_strlen (TREE_OPERAND (src, 2), only_value, eltsize);
   if (tree_int_cst_equal (len1, len2))
return len1;
 }
 
   if (TREE_CODE (src) == COMPOUND_EXPR
   && (only_value || !TREE_SIDE_EFFECTS (TREE_OPERAND (src, 0
-return c_strlen (TREE_OPERAND (src, 1), only_value);
+return c_strlen (TREE_OPERAND (src, 1), only_value, eltsize);
 
   location_t loc = EXPR_LOC_OR_LOC (src, input_location);
 
   /* Offset from the beginning of the string in bytes.  */
   tree byteoff;
-  src = string_constant (src, &byteoff);
+  tree memsize;
+  src = string_constant (src, &byteoff, &memsize);
   if (src == 0)
 return NULL_TREE;
 
   /* Determine the size of the string element.  */
-  unsigned eltsize
-= tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (src;
+  if (eltsize != tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (src)
+return NULL_TREE;
 
   /* Set MAXELTS to sizeof (SRC) / sizeof (*SRC) - 1, the maximum possible
  length of SRC.  Prefer TYPE_SIZE() to TREE_STRING_LENGTH() if possible
@@ -613,14 +615,10 @@ c_strlen (tree src, int only_value)
   HOST_WIDE_INT strelts = TREE_STRING_LENGTH (src);
   strelts = strelts / eltsize - 1;
 
-  HOST_WIDE_INT maxelts = strelts;
-  tree type = TREE_TYPE (src);
-  if (tree size = TYPE_SIZE_UNIT (type))
-if (tree_fits_shwi_p (size))
-  {
-   maxelts = tree_to_uhwi (size);
-   maxelts = maxelts / eltsize - 1;
-  }
+  if (!tree_fits_uhwi_p (memsize))
+return NULL_TREE;
+
+  HOST_WIDE_INT maxelts = tree_to_uhwi (memsize) / eltsize - 1;
 
   /* PTR can point to the byte representation of any string type, including
  char* and wchar_t*.  */
@@ -628,19 +626,23 @@ c_strlen (tree src, int only_value)
 
   if (byteoff && TREE_CODE (byteoff) != INTEGER_CST)
 {
+  /* For empty strings the result should be zero.  */
+  if (maxelts == 0)
+   return ssize_int (0);
+
+  /* The code below works only for single byte character types.  */
+  if (eltsize != 1)
+   return NULL_TREE;
+
   /* If the string has an internal NUL character followed by any
 non-NUL characters (e.g., "foo\0bar"), we can't compute
 the offset to the following NUL if we don't know where to
 start searching for it.  */
   unsigned len = string_length (ptr, eltsize, strelts);
-  if (len < strelts)
-   {
- /* Return when an embedded null character is found.  */
- return NULL_TREE;
-   }
 
-  if (!maxelts)
-   return ssize_int (0);
+   

Re: [PATCH] assume sprintf formatting of wide characters may fail (PR 86853)

[Jeff Law]

On 08/09/2018 01:29 PM, Bernd Edlinger wrote:
>>> + bool one_2_one_ascii
>>> +   = (target_to_host_charmap[0] == 1 && target_to_host ('a') == 
>>> 97);
>> Hmm.  Is this really sufficient?I have nowhere near enough knowledge
>> of the potential target character sets to know if this is sufficiently
>> tight.
> 
> Shouldn't it be target_to_host (97) == 'a' ?
According to Martin it actually doesn't matter, it can be written either
way and the result is still testing what we need.

jeff

Re: [PATCH] Add a character size parameter to c_strlen/get_range_strlen

[Joseph Myers]

On Thu, 16 Aug 2018, Jeff Law wrote:

> restores previous behavior.  The sprintf bits want to count element
> sized chunks, which for wchars is 4 bytes (that count will then be

>/* Compute the range the argument's length can be in.  */
> -  fmtresult slen = get_string_length (arg);
> +  int count_by = dir.specifier == 'S' || dir.modifier == FMT_LEN_l ? 4 : 1;

I don't see how a hardcoded 4 is correct here.  Surely you need to example 
wchar_type_node to determine its actual size for this target.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH, RFC, rs6000, v2] folding of vec_splat

[Will Schmidt]

On Thu, 2018-08-16 at 15:51 -0500, Segher Boessenkool wrote:
> Hi Will,
> 
> On Thu, Aug 16, 2018 at 10:50:45AM -0500, Will Schmidt wrote:
> > 2018-08-16  Will Schmidt  
> > 
> > * config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add support for
> >   early gimple folding of vec_splat().
> 
> Continuation lines should be indented to the *, not to the text after it.
OK

> 
> > +   /* Only fold the vec_splat_*() if arg1 is a constant
> > +  5-bit unsigned literal.  */
> > +   if (TREE_CODE (arg1) != INTEGER_CST || TREE_INT_CST_LOW (arg1) > 0x1f)
> > + return false;
> 
> Does this need to check for negative as well?  So something with IN_RANGE
> then.

> 
> > +   /* force arg1 into range.  */
> > +   tree new_arg1 = build_int_cst (arg1_type,
> > +  TREE_INT_CST_LOW (arg1) % n_elts);
> 
> Or is the range check unnecessary completely, since you have this?

I can be convinced either way. :-)
I think i still want both.  The first rejects (from gimple-folding) any
values that are outside of the 5-bit range as specified by the function
definition.
The second (modulo) then maps any 'valid' values into what is reasonable
for the data type being splatted.

While trying to convince myself one way or another, I do notice that
today (pre-folding), i can get out-of-range errors such as 
  /tmp/ccP0s6iJ.s:781: Error: operand out of range (-1 is not between
  0 and 3)
while with folding enabled, and this modulo in place, we compile without
warning.  So there is a behavior change, for which I have mixed
feelings.  :-)


> > +   tree splat;
> > +   if (TREE_CODE (arg0) == VECTOR_CST)
> > + splat = VECTOR_CST_ELT (arg0, TREE_INT_CST_LOW (new_arg1));
> > +   else
> > + {
> > +   /* Determine (in bits) the length and start location of the
> > +  splat value for a call to the tree_vec_extract helper.  */
> > +   int tree_size_in_bits = TREE_INT_CST_LOW (size_in_bytes (arg0_type))
> > +   * BITS_PER_UNIT;
> > +   int splat_elem_size = tree_size_in_bits / n_elts;
> > +   int splat_start_bit = TREE_INT_CST_LOW (new_arg1) * splat_elem_size;
> > +   /* Do not attempt to early-fold if the size + specified offset into
> > +  the vector would touch outside of the source vector.  */
> > +   tree len = build_int_cst (bitsizetype, splat_elem_size);
> > +   tree start = build_int_cst (bitsizetype, splat_start_bit);
> > +   splat = tree_vec_extract (gsi, TREE_TYPE (lhs_type), arg0,
> > + len, start);
> > +   }
> 
> This closing brace should be indented two spaces more.
Ok

> > -static inline tree
> > +tree
> >  tree_vec_extract (gimple_stmt_iterator *gsi, tree type,
> >   tree t, tree bitsize, tree bitpos)
> 
> It could use a comment, too (what the args are, etc.)

I can definitely add some commentary to my call into this function.
At a glance, the functions nearby tree_vec_extract do have a couple
lines of description each, so I'll look and see if I can come up with
anything reasonable here.

> Other than those nits, looks fine to me.  Maybe Richard or Bill have
> more comments?

Thanks for the review. 
-Will

> 
> 
> Segher
> 

Re: [PATCH] Add a character size parameter to c_strlen/get_range_strlen

[Jeff Law]

On 08/16/2018 05:01 PM, Joseph Myers wrote:
> On Thu, 16 Aug 2018, Jeff Law wrote:
> 
>> restores previous behavior.  The sprintf bits want to count element
>> sized chunks, which for wchars is 4 bytes (that count will then be
> 
>>/* Compute the range the argument's length can be in.  */
>> -  fmtresult slen = get_string_length (arg);
>> +  int count_by = dir.specifier == 'S' || dir.modifier == FMT_LEN_l ? 4 : 1;
> 
> I don't see how a hardcoded 4 is correct here.  Surely you need to example 
> wchar_type_node to determine its actual size for this target.
We did kick this around a little.  IIRC Martin didn't think that it was
worth handling the 2 byte wchar case.

In theory something like WCHAR_TYPE_SIZE / BITS_PER_UNIT probably does
the trick.   I'm a bit leery of using that though.  We don't use it
anywhere else within GCC AFAICT.

JEff

C++ PATCH for c++/67012, c++/86942, detect invalid cases with function return type deduction

[Marek Polacek]

As I promised in ,
this patch fixes a couple of invalid cases we weren't detecting.  It's got
testcases from two PRs and another case I found out; they're intertwined so
I think it makes sense to fix them in one go.

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

2018-08-16  Marek Polacek  

PR c++/86942
PR c++/67012
* decl.c (grokdeclarator): Disallow functions with trailing return
type with decltype(auto) as its type.  Also check the function if
it's inner declarator doesn't exist.

* g++.dg/cpp0x/auto52.C: New test.
* g++.dg/cpp1y/auto-fn52.C: New test.
* g++.dg/cpp1y/auto-fn53.C: New test.
* g++.dg/cpp1y/auto-fn54.C: New test.

diff --git gcc/cp/decl.c gcc/cp/decl.c
index fa58bc4d2b3..8261f8e30e5 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -11238,7 +11238,10 @@ grokdeclarator (const cp_declarator *declarator,
 
/* Handle a late-specified return type.  */
tree late_return_type = declarator->u.function.late_return_type;
-   if (funcdecl_p)
+   if (funcdecl_p
+   /* This is the case e.g. for
+  using T = auto () -> int.  */
+   || inner_declarator == NULL)
  {
if (tree auto_node = type_uses_auto (type))
  {
@@ -11270,6 +11273,18 @@ grokdeclarator (const cp_declarator *declarator,
   name, type);
return error_mark_node;
  }
+   else if (is_auto (type)
+&& (TYPE_IDENTIFIER (type)
+== decltype_auto_identifier))
+ {
+   if (funcdecl_p)
+ error ("%qs function with trailing return type has "
+"% as its type rather than "
+"plain %", name);
+   else
+ error ("invalid use of %");
+   return error_mark_node;
+ }
tree tmpl = CLASS_PLACEHOLDER_TEMPLATE (auto_node);
if (!tmpl)
  if (tree late_auto = type_uses_auto (late_return_type))
diff --git gcc/testsuite/g++.dg/cpp0x/auto52.C 
gcc/testsuite/g++.dg/cpp0x/auto52.C
index e69de29bb2d..9bfe7c754b5 100644
--- gcc/testsuite/g++.dg/cpp0x/auto52.C
+++ gcc/testsuite/g++.dg/cpp0x/auto52.C
@@ -0,0 +1,6 @@
+// PR c++/86942
+// { dg-do compile { target c++11 } }
+
+using T = auto() -> int;
+using U = void() -> int; // { dg-error "function with trailing return type not 
declared with .auto." }
+using W = auto(); // { dg-error "invalid use of .auto." }
diff --git gcc/testsuite/g++.dg/cpp1y/auto-fn52.C 
gcc/testsuite/g++.dg/cpp1y/auto-fn52.C
index e69de29bb2d..e239bc27dc2 100644
--- gcc/testsuite/g++.dg/cpp1y/auto-fn52.C
+++ gcc/testsuite/g++.dg/cpp1y/auto-fn52.C
@@ -0,0 +1,4 @@
+// PR c++/67012
+// { dg-do compile { target c++14 } }
+
+decltype(auto) f() -> int; // { dg-error "function with trailing return type 
has" }
diff --git gcc/testsuite/g++.dg/cpp1y/auto-fn53.C 
gcc/testsuite/g++.dg/cpp1y/auto-fn53.C
index e69de29bb2d..720aeeb215d 100644
--- gcc/testsuite/g++.dg/cpp1y/auto-fn53.C
+++ gcc/testsuite/g++.dg/cpp1y/auto-fn53.C
@@ -0,0 +1,4 @@
+// PR c++/86942
+// { dg-do compile { target c++14 } }
+
+using T = decltype(auto) () -> int; // { dg-error "invalid use of" }
diff --git gcc/testsuite/g++.dg/cpp1y/auto-fn54.C 
gcc/testsuite/g++.dg/cpp1y/auto-fn54.C
index e69de29bb2d..f3391ddfd75 100644
--- gcc/testsuite/g++.dg/cpp1y/auto-fn54.C
+++ gcc/testsuite/g++.dg/cpp1y/auto-fn54.C
@@ -0,0 +1,3 @@
+// { dg-do compile { target c++14 } }
+
+using T = int () -> decltype(auto); // { dg-error "function with trailing 
return type not declared with .auto." }

Re: [PATCH] assume sprintf formatting of wide characters may fail (PR 86853)

[Jeff Law]

On 08/04/2018 12:46 PM, Martin Sebor wrote:
> The sprintf handling of wide characters neglects to consider
> that calling the function may fail due to a conversion error
> (when the wide character is invalid or not representable in
> the current locale).  The handling also misinterprets
> the POSIX %S wide string directive as a plain narrow %s and
> doesn't include %C (the POSIX equivalent of %lc).  The attached
> patch corrects these oversights by extending the data structures
> to indicate when a directive may fail, and extending the UNDER4K
> member of the format_result structure to also encode calls with
> such directives.
> 
> Tested on x86_64-linux.
> 
> Besides the trunk, since this bug can affect code correctness
> I'd like to backport this patch to both release branches (7
> and 8).
> 
> Martin
> 
> gcc-86853.diff
> 
> 
> PR tree-optimization/86853 - sprintf optimization for wide strings doesn't 
> account for conversion failure
> 
> gcc/ChangeLog:
> 
>   PR tree-optimization/86853
>   * gimple-ssa-sprintf.c (struct format_result): Rename member.
>   (struct fmtresult): Add member and initialize it in ctors.
>   (format_character): Handle %C.  Extend range to NUL.  Set MAYFAIL.
>   (format_string): Handle %S the same as %ls.  Set MAYFAIL.
>   (format_directive): Set POSUNDER4K when MAYFAIL is set.
>   (parse_directive): Handle %C same as %c.
>   (sprintf_dom_walker::compute_format_length): Adjust.
>   (is_call_safe): Adjust.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR tree-optimization/86853
>   * gcc.dg/tree-ssa/builtin-sprintf-10.c: New test.
>   * gcc.dg/tree-ssa/builtin-sprintf-11.c: New test.
>   * gcc.dg/tree-ssa/builtin-sprintf-warn-18.c: Adjust.
I fixed two "empty" mis-spellings in gimple-ssa-sprintf.c and installed
this patch.

Let's give it a few days of soak time, then backporting is fine.

Jeff

Re: [PATCH] v2: Formatted printing for dump_* in the middle-end

[Jeff Law]

On 08/02/2018 11:54 AM, David Malcolm wrote:
> On Tue, 2018-07-31 at 19:56 +, Joseph Myers wrote:
>> On Tue, 31 Jul 2018, David Malcolm wrote:
>>
>>> I didn't exhaustively check every callsite to the changed calls;
>>> I'm
>>> assuming that -Wformat during bootstrap has effectively checked
>>> that
>>> for me.  Though now I think about it, I note that we use
>>> HOST_WIDE_INT_PRINT_DEC in many places: is this guaranteed to be a
>>> valid input to pp_format on all of our configurations?
>> HOST_WIDE_INT_PRINT_DEC should not be considered safe with pp_format 
>> (although since r197049 may have effectively stopped using %I64 on
>> MinGW 
>> hosts, I'm not sure if there are current cases where it won't
>> work).  
>> Rather, it is the job of pp_format to map the 'w' length specifier
>> to 
>> HOST_WIDE_INT_PRINT_DEC etc.
>>
>> I think it clearly makes for cleaner code to limit use of 
>> HOST_WIDE_INT_PRINT_* to as few places as possible and to prefer use
>> of 
>> internal printf-like functions that accept formats such as %wd where 
>> possible.
> Thanks.
> 
> I grepped the tree for every use of HOST_WIDE_INT_PRINT* and found
> all that were within dump_printf[_loc] calls.  All such uses were
> within files of the form "gcc/tree-vect*.c".
> 
> Here's an updated version of the patch (v2) which fixes those
> callsites to use "%wd" or "%wu"; the dumpfile.c changes are as
> per v1.
> 
> Changed in v2:
> * rebased to after r263239 (which also touched c-format.c/h)
> * avoid HOST_WIDE_INT_PRINT* within dump_printf* calls (in
>   gcc/tree-vect*.c)
> 
> I didn't add test coverage for %wd and %wu, as pretty-print.c already
> has selftest coverage for these.
> 
> Richard's review of the v1 patch was:
> "The patch is OK if C family maintainers agree on their parts."
> so I'm looking for review of:
> (a) the c-format.c changes (Joseph?), and
> (b) the tree-vect*.c changes (Richard?  Joseph?)
> 
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> 
> Is this patch OK for trunk?
> 
> Thanks
> Dave
> 
> 
> Blurb from v1, for reference:
> 
> This patch converts dump_print and dump_printf_loc from using
> printf (and thus ATTRIBUTE_PRINTF) to using a new pretty-printer
> based on pp_format, which supports formatting middle-end types.
> 
> In particular, the following codes are implemented (in addition
> to the standard pretty_printer ones):
> 
>%E: gimple *:
>Equivalent to: dump_gimple_expr (MSG_*, TDF_SLIM, stmt, 0)
>%G: gimple *:
>Equivalent to: dump_gimple_stmt (MSG_*, TDF_SLIM, stmt, 0)
>%T: tree:
>Equivalent to: dump_generic_expr (MSG_*, arg, TDF_SLIM).
> 
> Hence it becomes possible to convert e.g.:
> 
>   if (dump_enabled_p ())
> {
>   dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>"not vectorized: different sized vector "
>"types in statement, ");
>   dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM, vectype);
>   dump_printf (MSG_MISSED_OPTIMIZATION, " and ");
>   dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM, nunits_vectype);
>   dump_printf (MSG_MISSED_OPTIMIZATION, "\n");
> }
> 
> into a one-liner:
> 
>   if (dump_enabled_p ())
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>  "not vectorized: different sized vector "
>  "types in statement, %T and %T\n",
>  vectype, nunits_vectype);
> 
> Unlike regular pretty-printers, this one captures optinfo_item
> instances for the formatted chunks as appropriate, so that when
> written out to a JSON optimization record, the relevant parts of
> the message are labelled by type, and by source location (so that
> e.g. %G is entirely equivalent to using dump_gimple_stmt).
> 
> dump_printf and dump_printf_loc become marked with
> ATTRIBUTE_GCC_DUMP_PRINTF, which the patch also implements.
> 
> gcc/c-family/ChangeLog:
>   * c-format.c (enum format_type): Add gcc_dump_printf_format_type.
>   (gcc_dump_printf_length_specs): New.
>   (gcc_dump_printf_flag_pairs): New.
>   (gcc_dump_printf_flag_specs): New.
>   (gcc_dump_printf_char_table): New.
>   (format_types_orig): Add entry for "gcc_dump_printf".
>   (init_dynamic_diag_info): Set up length_char_specs and
>   conversion_specs for gcc_dump_printf_format_type.
>   (handle_format_attribute): Handle gcc_dump_printf_format_type.
> 
> gcc/ChangeLog:
>   * dump-context.h: Include "dumpfile.h".
>   (dump_context::dump_printf_va): Convert final param from va_list
>   to va_list *.  Convert from ATTRIBUTE_PRINTF to
>   ATTRIBUTE_GCC_DUMP_PRINTF.
>   (dump_context::dump_printf_loc_va): Likewise.
>   * dumpfile.c: Include "stringpool.h".
>   (make_item_for_dump_printf_va): Delete.
>   (make_item_for_dump_printf): Delete.
>   (class dump_pretty_printer): New class.
>   (dump_pretty_printer::dump_pretty_printer): New ctor.
>   (dump_pretty_

[PATCH] DWARF: add DW_AT_count to zero-length arrays

[Omar Sandoval]

Hi,

This fixes the issue that it is impossible to distinguish a zero-length array
type from a flexible array type given the DWARF produced by GCC (which I
reported here [1]). We do so by adding a DW_AT_count attribute with a value of
zero only for zero-length arrays (this is what clang does in this case, too).

1: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86985

The reproducer from the PR now produces the expected output:

$ ~/gcc-build/gcc/xgcc -B ~/gcc-build/gcc -g -c zero_length.c
$ ~/gcc-build/gcc/xgcc -B ~/gcc-build/gcc -g -c flexible.c
$ gdb -batch -ex 'ptype baz' zero_length.o
type = struct {
int foo;
int bar[0];
}
$ gdb -batch -ex 'ptype baz' flexible.o
type = struct {
int foo;
int bar[];
}

This was bootstrapped and tested on x86_64-pc-linux-gnu.

I don't have commit rights (first time contributor), so if this change is okay
could it please be applied?

Thanks!

2018-08-16  Omar Sandoval  

* dwarf2out.c (is_c_family): New.
(add_subscript_info): Add DW_AT_count of 0 for C zero-length arrays.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 5a74131d332..b638942c156 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -3671,6 +3671,7 @@ static const char *get_AT_string (dw_die_ref, enum 
dwarf_attribute);
 static int get_AT_flag (dw_die_ref, enum dwarf_attribute);
 static unsigned get_AT_unsigned (dw_die_ref, enum dwarf_attribute);
 static inline dw_die_ref get_AT_ref (dw_die_ref, enum dwarf_attribute);
+static bool is_c_family (void);
 static bool is_cxx (void);
 static bool is_cxx (const_tree);
 static bool is_fortran (void);
@@ -5434,6 +5435,21 @@ get_AT_file (dw_die_ref die, enum dwarf_attribute 
attr_kind)
   return a ? AT_file (a) : NULL;
 }
 
+/* Return TRUE if the language is C or C++.  */
+
+static inline bool
+is_c_family (void)
+{
+  unsigned int lang = get_AT_unsigned (comp_unit_die (), DW_AT_language);
+
+  return (lang == DW_LANG_C || lang == DW_LANG_C89 || lang == DW_LANG_C99
+ || lang == DW_LANG_C11 || lang == DW_LANG_C_plus_plus
+ || lang == DW_LANG_ObjC_plus_plus || lang == DW_LANG_C_plus_plus_11
+ || lang == DW_LANG_C_plus_plus_14);
+
+
+}
+
 /* Return TRUE if the language is C++.  */
 
 static inline bool
@@ -20918,12 +20934,24 @@ add_subscript_info (dw_die_ref type_die, tree type, 
bool collapse_p)
   dimension arr(N:*)
 Since the debugger is definitely going to need to know N
 to produce useful results, go ahead and output the lower
-bound solo, and hope the debugger can cope.  */
+bound solo, and hope the debugger can cope.
+
+For C and C++, if upper is NULL, this may be a zero-length array
+or a flexible array; we'd like to be able to distinguish between
+the two.  Set DW_AT_count to 0 for the former.  TYPE_SIZE is NULL
+for the latter.  */
 
  if (!get_AT (subrange_die, DW_AT_lower_bound))
add_bound_info (subrange_die, DW_AT_lower_bound, lower, NULL);
- if (upper && !get_AT (subrange_die, DW_AT_upper_bound))
-   add_bound_info (subrange_die, DW_AT_upper_bound, upper, NULL);
+ if (!get_AT (subrange_die, DW_AT_upper_bound)
+ && !get_AT (subrange_die, DW_AT_count))
+   {
+ if (upper)
+   add_bound_info (subrange_die, DW_AT_upper_bound, upper, NULL);
+ else if (is_c_family () && TYPE_SIZE (type))
+   add_bound_info (subrange_die, DW_AT_count,
+   build_int_cst (TREE_TYPE (lower), 0), NULL);
+   }
}
 
   /* Otherwise we have an array type with an unspecified length.  The

Re: [PATCH] Check the STRING_CSTs in varasm.c

[Jeff Law]

On 08/05/2018 04:28 AM, Bernd Edlinger wrote:
> Hi,
> 
> I would like to do a minor tweak to the patch.
> While staring at the other patch I realized that I should
> better pass size and not thissize to the check
> function, instead of making use of how thissize is
> computed using MIN above.  This means that this condition
> 
> +  if ((unsigned)len != size && (unsigned)len != size + elts)
> +return false;
> 
> should better and more readable be:
> 
> +  if (size < (unsigned)len && size != len - elts)
> +return false;
> 
> Furthermore I wanted to have the check function static,
> which I missed to do previously.
> 
> For completeness, these are again the precondition patches
> for this patch:
> 
> [PATCH] Handle overlength strings in the C FE
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00142.html
I believe Joseph ack'd this already:

https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00655.html


> 
> [PATCH] Make GO string literals properly NUL terminated
> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01931.html
Ian didn't object, deferring to the larger scoped review.  My
understanding is this should be a nop for Go.  Given it takes us closer
to have a canonical form, I'll go ahead and ACK for the trunk.


> 
> [PATCH] [Ada] Make middle-end string literals NUL terminated
> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01929.html
This is OK.


> 
> [PATCH] Create internally nul terminated string literals in fortan FE
> https://gcc.gnu.org/ml/fortran/2018-08/msg0.html
This is OK too.  THough it is unclear why we're not fixing
gfc_build_string_const vs introducing gfc_build_hollerith_string_const.
Are hollerith strings naturally not terminated in the fortran front-end
and thus need special handling, while other calls to
gfc_build_string_const are always with naturally nul terminated strings?




> 
> 
> Bootstrapped, as usual.
> Is it OK for trunk?
So as a toplevel comment.While there may be some interaction with
Martin's code to detect and warn for unterminated strings passed to
strlen and other cases, I think the overall goal of canonicalizing our
internal representation of STRING_CST along with a verification step is
a good thing and it may make some of Martin's work easier.

At this point are the prereqs of the varasm verification patch all ACK'd?

Jeff

Re: [PATCH, Darwin] Move fixed _Unwind_find_Enclosing_func to a crt.

[Jeff Law]

On 08/15/2018 12:50 PM, Iain Sandoe wrote:
> Hi,
> 
> This is a pre-cursor to other tidy-ups aimed at being able to dispense with 
> the “ext” library that we introduced (a lng time ago) to deal with the 
> differences between Darwin’s installed libgcc_s and the current compiler.
> 
> Since the installed version of _Unwind_find_Enclosing_func was broken, we 
> built a small shim function to allow Java to work on Darwin10.
> 
> We have been carrying this around in the darwin-specific unwinder code ever 
> since.
> 
> This patch splits the patch function out into a shim crt that is included 
> only when needed (for x86 and ppc via Rosetta on Darwin10).
> 
> There is no need to treat Darwin12+ differently in this and that allows us to 
> simplify the header there.
> 
> OK for trunk?
> Iain
> 
> 2018-08-15  Iain Sandoe 
> 
> gcc/
>   * config/darwin10.h (LINK_GCC_C_SEQUENCE_SPEC): Adjust to use the
>   Darwin10-specific unwinder-shim.
>   * config/darwin12.h (LINK_GCC_C_SEQUENCE_SPEC): Remove.
>   * config/rs6000/darwin.h (DARWIN_CRT1_SPEC, DARWIN_DYLIB1_SPEC): 
>   New to cater for Darwin10 Rosetta.
> 
> libgcc/
>   * config/unwind-dw2-fde-darwin.c  
> (_darwin10_Unwind_FindEnclosingFunction):
>   move from here ...
>   * config/darwin10-unwind-find-enc-func.c: … to here.
>   * config/t-darwin: Build Darwin10 unwinder shim crt.
>   * libgcc/config.host: Add the Darwin10 unwinder shim.
Given you've probably got a better understanding of Darwin's needs than
anyone here, this seems quite reasonable.  OK for the trunk unless
someone objects in the next 24hrs.

jeff

Re: [PATCH] Make strlen range computations more conservative

[Jeff Law]

On 08/09/2018 12:27 AM, Richard Biener wrote:
> On August 9, 2018 7:26:19 AM GMT+02:00, Jeff Law  wrote:
>> On 07/24/2018 05:18 PM, Bernd Edlinger wrote:
>>> On 07/24/18 23:46, Jeff Law wrote:
 On 07/24/2018 01:59 AM, Bernd Edlinger wrote:
> Hi!
>
> This patch makes strlen range computations more conservative.
>
> Firstly if there is a visible type cast from type A to B before
>> passing
> then value to strlen, don't expect the type layout of B to restrict
>> the
> possible return value range of strlen.
 Why do you think this is the right thing to do?  ie, is there
>> language
 in the standards that makes you think the code as it stands today is
 incorrect from a conformance standpoint?  Is there a significant
>> body of
 code that is affected in an adverse way by the current code?  If so,
 what code?


>>>
>>> I think if you have an object, of an effective type A say char[100],
>> then
>>> you can cast the address of A to B, say typedef char (*B)[2] for
>> instance
>>> and then to const char *, say for use in strlen.  I may be wrong, but
>> I think
>>> that we should at least try not to pick up char[2] from B, but
>> instead
>>> use A for strlen ranges, or leave this range open.  Currently the
>> range
>>> info for strlen is [0..1] in this case, even if we see the type cast
>>> in the generic tree.
>> Coming back to this... I'd like to hope we can depend on the type of
>> the
>> strlen argument.  Obviously if it's a char *, we know nothing.  But if
>> it's an ARRAY_TYPE it'd be advantageous if we could use the array
>> bounds
>> to bound the length.
> 
>  But the FE made it char * and only because pointer type conversions are 
> useless we may see sth else. So we cannot use the type of the argument. 
I must have missed something along the line -- I thought I saw array
types passed directly without converting to a char *.  But as I noted
later, we still have things like forwprop that may convert pointer
arithmetic/access to arrays.  So I think we have to give up on using the
types for anything that affects code generation.  I think that's
consistent with your and Jakub's position.

I think we do want to use types to drive warnings though.  They help us
eliminate meaningful numbers of false positives.  There may be oddball
missed warnings or false positives because of the actions of forwprop,
but we should see far fewer issues if we rely on types for warnings.


> 
>>
>> *But* we have code which will turn pointer access into array indexing.
>> tree-ssa-forwprop.c can do that, there may be others.  So if we
>> originally had a char * argument to strlen, but forwprop changed it
>> into
>> a char[N] type, then have we just broken things?  I'd totally forgotten
>> about this behavior from forwprop.  PR 46393 contains sample code where
>> this happens.
> 
> If we really still have this it must be very constrained. Because we used to 
> have sorts of wrong code issues with this and data dependence analysis. 
Yea, it's still in there.  I verified it during the gcc-8 cycle since
the BZ is a regression.   The general forms of the transformation is
dicussed in a comment at the top of the file.   Given all that we have
discussed in this thread this code could well be a ticking time bomb for
wrong code issues.

What's scary to me is git blame says I wrote that code.  I certainly
remember writing bits of tree-ssa-forwprop, but don't recall writing
this specific transformation.


> 
>> I also thought we had code to recover array indexing from pointer
>> arithmetic in the C/C++ front-end, but I can't seem to find it tonight.
>> But it would raise similar concerns.
> 
> I've removed that. What I did at some point is avoid decaying too much array 
> to pointer conversions in the FEs to preserve array refs instead of trying to 
> reconstruct them late. 
THat explains why I couldn't find it :-)


Jeff

Re: PING [PATCH] warn for strlen of arrays with missing nul (PR 86552, 86711, 86714) )

[Jeff Law]

On 08/01/2018 08:44 PM, Martin Sebor wrote:
> Since the foundation of the patch is detecting and avoiding
> the overly aggressive folding of unterminated char arrays,
> besides issuing a warning for such arguments to strlen,
> the patch also fixes pr86711 - wrong folding of memchr, and
> pr86714 - tree-ssa-forwprop.c confused by too long initializer.
> 
> The substance of the attached updated patch is unchanged,
> I have just added test cases for the two additional bugs.
> 
Just to be absolutely sure.  The patch attached to this message is
superceded by the later 6 part patchkit that fixes 86714, 86711 and 86552.

I'm just trying to make sure I'm looking at the right kits from both you
and Bernd for the next step.

Thanks,
Jeff

Re: [PATCH] DWARF: add DW_AT_count to zero-length arrays

[Andrew Pinski]

On Thu, Aug 16, 2018 at 9:29 PM Omar Sandoval  wrote:
>
> Hi,
>
> This fixes the issue that it is impossible to distinguish a zero-length array
> type from a flexible array type given the DWARF produced by GCC (which I
> reported here [1]). We do so by adding a DW_AT_count attribute with a value of
> zero only for zero-length arrays (this is what clang does in this case, too).
>
> 1: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86985
>
> The reproducer from the PR now produces the expected output:
>
> $ ~/gcc-build/gcc/xgcc -B ~/gcc-build/gcc -g -c zero_length.c
> $ ~/gcc-build/gcc/xgcc -B ~/gcc-build/gcc -g -c flexible.c
> $ gdb -batch -ex 'ptype baz' zero_length.o
> type = struct {
> int foo;
> int bar[0];
> }
> $ gdb -batch -ex 'ptype baz' flexible.o
> type = struct {
> int foo;
> int bar[];
> }
>
> This was bootstrapped and tested on x86_64-pc-linux-gnu.
>
> I don't have commit rights (first time contributor), so if this change is okay
> could it please be applied?

I don't think is really required.  Zero-sized arrays are a GCC
extension which was introduced before flexible array types were part
of C.  They should be interchangable in all places.  Can you give an
example of where they are not?

A comment about the patch below.

>
> Thanks!
>
> 2018-08-16  Omar Sandoval  
>
> * dwarf2out.c (is_c_family): New.
> (add_subscript_info): Add DW_AT_count of 0 for C zero-length arrays.
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index 5a74131d332..b638942c156 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -3671,6 +3671,7 @@ static const char *get_AT_string (dw_die_ref, enum 
> dwarf_attribute);
>  static int get_AT_flag (dw_die_ref, enum dwarf_attribute);
>  static unsigned get_AT_unsigned (dw_die_ref, enum dwarf_attribute);
>  static inline dw_die_ref get_AT_ref (dw_die_ref, enum dwarf_attribute);
> +static bool is_c_family (void);
>  static bool is_cxx (void);
>  static bool is_cxx (const_tree);
>  static bool is_fortran (void);
> @@ -5434,6 +5435,21 @@ get_AT_file (dw_die_ref die, enum dwarf_attribute 
> attr_kind)
>return a ? AT_file (a) : NULL;
>  }
>
> +/* Return TRUE if the language is C or C++.  */
> +
> +static inline bool
> +is_c_family (void)
> +{
> +  unsigned int lang = get_AT_unsigned (comp_unit_die (), DW_AT_language);
> +
> +  return (lang == DW_LANG_C || lang == DW_LANG_C89 || lang == DW_LANG_C99
> + || lang == DW_LANG_C11 || lang == DW_LANG_C_plus_plus
> + || lang == DW_LANG_ObjC_plus_plus || lang == DW_LANG_C_plus_plus_11
> + || lang == DW_LANG_C_plus_plus_14);
> +
> +
> +}

I think you should just "is_cxx () || is_c ()" and factor out the
C/Objective-C parts (C++ is already done).
This is will make it easier to maintain so if c++17 or c++20 comes
along, only one place needs to be changed.

Thanks,
Andrew


> +
>  /* Return TRUE if the language is C++.  */
>
>  static inline bool
> @@ -20918,12 +20934,24 @@ add_subscript_info (dw_die_ref type_die, tree type, 
> bool collapse_p)
>dimension arr(N:*)
>  Since the debugger is definitely going to need to know N
>  to produce useful results, go ahead and output the lower
> -bound solo, and hope the debugger can cope.  */
> +bound solo, and hope the debugger can cope.
> +
> +For C and C++, if upper is NULL, this may be a zero-length array
> +or a flexible array; we'd like to be able to distinguish between
> +the two.  Set DW_AT_count to 0 for the former.  TYPE_SIZE is NULL
> +for the latter.  */
>
>   if (!get_AT (subrange_die, DW_AT_lower_bound))
> add_bound_info (subrange_die, DW_AT_lower_bound, lower, NULL);
> - if (upper && !get_AT (subrange_die, DW_AT_upper_bound))
> -   add_bound_info (subrange_die, DW_AT_upper_bound, upper, NULL);
> + if (!get_AT (subrange_die, DW_AT_upper_bound)
> + && !get_AT (subrange_die, DW_AT_count))
> +   {
> + if (upper)
> +   add_bound_info (subrange_die, DW_AT_upper_bound, upper, NULL);
> + else if (is_c_family () && TYPE_SIZE (type))
> +   add_bound_info (subrange_die, DW_AT_count,
> +   build_int_cst (TREE_TYPE (lower), 0), NULL);
> +   }
> }
>
>/* Otherwise we have an array type with an unspecified length.  The

Re: [2/5] C-SKY port v3: Backend implementation

[Jeff Law]

On 08/05/2018 10:34 PM, Sandra Loosemore wrote:
> 
> csky-gcc-2.log
> 
> 
> 2018-08-05  Jojo  
>   Huibin Wang  
>   Sandra Loosemore  
>   Chung-Lin Tang  
> 
>   C-SKY port: Backend implementation
> 
>   gcc/
>   * config/csky/*: New.
>   * common/config/csky/*: New.
> 
I did another once-over and I think this is fine for the trunk.  Make
sure to add a MAINTAINERS entry.

Jeff

Re: [PATCH] DWARF: add DW_AT_count to zero-length arrays

[Omar Sandoval]

On Thu, Aug 16, 2018 at 10:27:48PM -0700, Andrew Pinski wrote:
> On Thu, Aug 16, 2018 at 9:29 PM Omar Sandoval  wrote:
> >
> > Hi,
> >
> > This fixes the issue that it is impossible to distinguish a zero-length 
> > array
> > type from a flexible array type given the DWARF produced by GCC (which I
> > reported here [1]). We do so by adding a DW_AT_count attribute with a value 
> > of
> > zero only for zero-length arrays (this is what clang does in this case, 
> > too).
> >
> > 1: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86985
> >
> > The reproducer from the PR now produces the expected output:
> >
> > $ ~/gcc-build/gcc/xgcc -B ~/gcc-build/gcc -g -c zero_length.c
> > $ ~/gcc-build/gcc/xgcc -B ~/gcc-build/gcc -g -c flexible.c
> > $ gdb -batch -ex 'ptype baz' zero_length.o
> > type = struct {
> > int foo;
> > int bar[0];
> > }
> > $ gdb -batch -ex 'ptype baz' flexible.o
> > type = struct {
> > int foo;
> > int bar[];
> > }
> >
> > This was bootstrapped and tested on x86_64-pc-linux-gnu.
> >
> > I don't have commit rights (first time contributor), so if this change is 
> > okay
> > could it please be applied?
> 
> I don't think is really required.  Zero-sized arrays are a GCC
> extension which was introduced before flexible array types were part
> of C.  They should be interchangable in all places.  Can you give an
> example of where they are not?

See https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html. All of the
following cases are only valid for zero-length arrays and not flexible
arrays:

sizeof(int [0]);

struct foo {
int bar[0];
};

struct foo {
int bar[0];
int baz;
};

union foo {
int bar[0];
int baz;
};

int arr[2][0];

The following is only valid for flexible arrays and not zero-length
arrays:

struct {
int foo;
int bar[];
} baz = {1, {2, 3}};

> A comment about the patch below.
> 
> >
> > Thanks!
> >
> > 2018-08-16  Omar Sandoval  
> >
> > * dwarf2out.c (is_c_family): New.
> > (add_subscript_info): Add DW_AT_count of 0 for C zero-length arrays.
> >
> > diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> > index 5a74131d332..b638942c156 100644
> > --- a/gcc/dwarf2out.c
> > +++ b/gcc/dwarf2out.c
> > @@ -3671,6 +3671,7 @@ static const char *get_AT_string (dw_die_ref, enum 
> > dwarf_attribute);
> >  static int get_AT_flag (dw_die_ref, enum dwarf_attribute);
> >  static unsigned get_AT_unsigned (dw_die_ref, enum dwarf_attribute);
> >  static inline dw_die_ref get_AT_ref (dw_die_ref, enum dwarf_attribute);
> > +static bool is_c_family (void);
> >  static bool is_cxx (void);
> >  static bool is_cxx (const_tree);
> >  static bool is_fortran (void);
> > @@ -5434,6 +5435,21 @@ get_AT_file (dw_die_ref die, enum dwarf_attribute 
> > attr_kind)
> >return a ? AT_file (a) : NULL;
> >  }
> >
> > +/* Return TRUE if the language is C or C++.  */
> > +
> > +static inline bool
> > +is_c_family (void)
> > +{
> > +  unsigned int lang = get_AT_unsigned (comp_unit_die (), DW_AT_language);
> > +
> > +  return (lang == DW_LANG_C || lang == DW_LANG_C89 || lang == DW_LANG_C99
> > + || lang == DW_LANG_C11 || lang == DW_LANG_C_plus_plus
> > + || lang == DW_LANG_ObjC_plus_plus || lang == 
> > DW_LANG_C_plus_plus_11
> > + || lang == DW_LANG_C_plus_plus_14);
> > +
> > +
> > +}
> 
> I think you should just "is_cxx () || is_c ()" and factor out the
> C/Objective-C parts (C++ is already done).
> This is will make it easier to maintain so if c++17 or c++20 comes
> along, only one place needs to be changed.

Okay, will do.

Thanks for taking a look!

> > +
> >  /* Return TRUE if the language is C++.  */
> >
> >  static inline bool
> > @@ -20918,12 +20934,24 @@ add_subscript_info (dw_die_ref type_die, tree 
> > type, bool collapse_p)
> >dimension arr(N:*)
> >  Since the debugger is definitely going to need to know N
> >  to produce useful results, go ahead and output the lower
> > -bound solo, and hope the debugger can cope.  */
> > +bound solo, and hope the debugger can cope.
> > +
> > +For C and C++, if upper is NULL, this may be a zero-length 
> > array
> > +or a flexible array; we'd like to be able to distinguish 
> > between
> > +the two.  Set DW_AT_count to 0 for the former.  TYPE_SIZE is 
> > NULL
> > +for the latter.  */
> >
> >   if (!get_AT (subrange_die, DW_AT_lower_bound))
> > add_bound_info (subrange_die, DW_AT_lower_bound, lower, NULL);
> > - if (upper && !get_AT (subrange_die, DW_AT_upper_bound))
> > -   add_bound_info (subrange_die, DW_AT_upper_bound, upper, NULL);
> > + if (!get_AT (subrange_die, DW_AT_upper_bound)
> > + && !get_AT (subrange_die, DW_AT_count))
> > +   {
> > + if (upper)
> > +   add_bound_info (subrange_die, DW_AT_upper_bound, upper, 
> > NULL);
> > + else if (is_c_family () && TYP