[PATCH][OBVIOUS] Add IntegerRange for Wcatch-value= option.

[Martin Liška]

Hi.

This is small documentation fix, I'm going to install the patch
as obvious.

Now we display:

$  ./xg++ -B. --help=c++  | grep catch-value=
  -Wcatch-value   Warn about catch handlers of non-reference type.  
Same as -Wcatch-value=.
  -Wcatch-value=<0,3> Warn about catch handlers of non-reference type.

Thanks,
Martin

gcc/c-family/ChangeLog:

2018-02-21  Martin Liska  

* c.opt (Wcatch-value=): Add IntegerRange.
---
 gcc/c-family/c.opt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 7fb386d456d..421b1464545 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -397,7 +397,7 @@ C++ ObjC++ Warning Alias(Wcatch-value=, 1, 0)
 Warn about catch handlers of non-reference type.
 
 Wcatch-value=
-C++ ObjC++ Var(warn_catch_value) Warning Joined RejectNegative UInteger LangEnabledBy(C++ ObjC++,Wall, 1, 0)
+C++ ObjC++ Var(warn_catch_value) Warning Joined RejectNegative UInteger LangEnabledBy(C++ ObjC++,Wall, 1, 0) IntegerRange(0, 3)
 Warn about catch handlers of non-reference type.
 
 Wchar-subscripts

Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478)

[Jakub Jelinek]

On Tue, Feb 20, 2018 at 04:59:12PM -0700, Martin Sebor wrote:
> > It would help if you explained why you think it is a good idea
> > ignoring the other phi arguments if you have one (or more) where you can
> > determine length.
> 
> It's a heuristic that was meant just for the -Wformat-overflow
> warning.  When making decisions that affect code generation it's
> obviously not correct to ignore the possibility that unknown
> arguments may be shorter than the minimum or longer than
> the maximum.  The fuzzy argument was meant to differentiate
> between two got but I forgot about it when I added the fix
> for PR 83671.

get_range_strlen (the 2 argument one) is right now called:
3 times from builtins.c for -Wstringop-overflow
once from gimple-ssa-sprintf.c for -Wformat-overflow
once from tree-ssa-strlen.c for -Wstringop-truncation
once from gimple-fold.c for gimple_fold_builtin_strlen value ranges

So, which of these do you want the heuristics for?  All 3 warnings,
just one of them, something else?  In the 2 patches I've posted last
all the 3 different warnings are treated the same (fuzzy == 2).

Looking at strlenopt-40.c testcase, in the test you clearly rely on
fuzzy argument being set for the value ranges case
(gimple_fold_builtin_strlen), otherwise many of the
subtests would fail.

Jakub

[PATCH] Add "native" as a valid option value for -march= on aarch64 (PR driver/83193).

[Martin Liška]

Hi.

This is equivalent patch for aarch64 target. I've tested that both as cross
compiler and native one.

Ready for trunk?
Thanks,
Martin
>From 4fbe17099f8618ddd6a4de2d269ecb6f99625927 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Tue, 20 Feb 2018 14:14:25 +0100
Subject: [PATCH 2/3] Add "native" as a valid option value for -march= on
 aarch64 (PR driver/83193).

gcc/ChangeLog:

2018-02-20  Martin Liska  

	PR driver/83193
	* config/aarch64/aarch64.c (aarch64_print_hint_for_core_or_arch):
	Add "native" as a possible value.
	* config/aarch64/aarch64.h (HAVE_LOCAL_CPU_DETECT):  Define
	the macro when native cpu detection is available.
---
 gcc/config/aarch64/aarch64.c | 7 +++
 gcc/config/aarch64/aarch64.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e1fb87f047f..33c90ef02dc 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10628,6 +10628,13 @@ aarch64_print_hint_for_core_or_arch (const char *str, bool arch)
   const struct processor *entry = arch ? all_architectures : all_cores;
   for (; entry->name != NULL; entry++)
 candidates.safe_push (entry->name);
+
+#ifdef HAVE_LOCAL_CPU_DETECT
+  /* Add also "native" as possible value.  */
+  if (arch)
+candidates.safe_push ("native");
+#endif
+
   char *s;
   const char *hint = candidates_list_and_hint (str, s, candidates);
   if (hint)
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index e3c52f63683..976f9afae54 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -1002,6 +1002,7 @@ extern const char *aarch64_rewrite_mcpu (int argc, const char **argv);
 
 #if defined(__aarch64__)
 extern const char *host_detect_local_cpu (int argc, const char **argv);
+#define HAVE_LOCAL_CPU_DETECT
 # define EXTRA_SPEC_FUNCTIONS		\
   { "local_cpu_detect", host_detect_local_cpu },			\
   MCPU_TO_MARCH_SPEC_FUNCTIONS
-- 
2.16.1

Add "native" as a valid option value for -march= on arm (PR driver/83193).

[Martin Liška]

Hi.

This is equivalent patch for ARM target.

Ready for trunk?
Thanks,
Martin
>From 656a883bc5239439ba80743f15a8df704501ee71 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Tue, 20 Feb 2018 14:09:22 +0100
Subject: [PATCH 1/3] Add "native" as a valid option value for -march= on arm
 (PR driver/83193).

gcc/ChangeLog:

2018-02-20  Martin Liska  

	PR driver/83193
	* common/config/arm/arm-common.c (arm_print_hint_for_arch_option):
	Add "native" as a possible value.
	* config/arm/arm.h (HAVE_LOCAL_CPU_DETECT): Define the macro
	when native cpu detection is available.
---
 gcc/common/config/arm/arm-common.c | 6 ++
 gcc/config/arm/arm.h   | 1 +
 2 files changed, 7 insertions(+)

diff --git a/gcc/common/config/arm/arm-common.c b/gcc/common/config/arm/arm-common.c
index fc585e0b0ee..50f0bad3e36 100644
--- a/gcc/common/config/arm/arm-common.c
+++ b/gcc/common/config/arm/arm-common.c
@@ -353,6 +353,12 @@ arm_print_hint_for_arch_option (const char *target,
   auto_vec candidates;
   for (; list->common.name != NULL; list++)
 candidates.safe_push (list->common.name);
+
+#ifdef HAVE_LOCAL_CPU_DETECT
+  /* Add also "native" as possible value.  */
+  candidates.safe_push ("native");
+#endif
+
   char *s;
   const char *hint = candidates_list_and_hint (target, s, candidates);
   if (hint)
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 6f3c4f461b9..bbf3937a592 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -2194,6 +2194,7 @@ extern const char *arm_target_thumb_only (int argc, const char **argv);
an ARM chip.  */
 #if defined(__arm__)
 extern const char *host_detect_local_cpu (int argc, const char **argv);
+#define HAVE_LOCAL_CPU_DETECT
 # define MCPU_MTUNE_NATIVE_FUNCTIONS			\
   { "local_cpu_detect", host_detect_local_cpu },
 # define MCPU_MTUNE_NATIVE_SPECS\
-- 
2.16.1

Re: [PATCH] Add "native" as a valid option value for -march= on i386 (PR driver/83193).

[Martin Liška]

On 02/20/2018 05:26 PM, Martin Liška wrote:
> On 02/20/2018 05:19 PM, Richard Sandiford wrote:
>> Jakub Jelinek  writes:
>>> On Tue, Feb 20, 2018 at 03:58:07PM +, Richard Sandiford wrote:
 Martin Liška  writes:
> Hi.
>
> Following patch adds "native" as a possible option for -march value on
> i386 target.  I have similar patches for other targets. Would it be
> possible to install the patch in current stage?

 [...]

> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index d54e7301e84..361d4df2663 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -4193,6 +4193,9 @@ ix86_option_override_internal (bool main_args_p,
>   || ((processor_alias_table[i].flags & PTA_64BIT) != 0)))
> candidates.safe_push (processor_alias_table[i].name);
>  
> +  /* Add also "native" as possible value.  */
> +  candidates.safe_push ("native");
> +
>char *s;
>const char *hint
>   = candidates_list_and_hint (opts->x_ix86_arch_string, s, candidates);

 We should probably only do this if the driver supports -march=native.
 I think for x86 that means HAVE_LOCAL_CPU_DETECT.
>>>
>>> Isn't the option available always?  It just doesn't do anything if
>>> /* If we are compiling with GCC where %EBX register is fixed, then the
>>>driver will just ignore -march and -mtune "native" target and will leave
>>>to the newly built compiler to generate code for its default target.  */
>>
>> It's only available for x86 hosts:
>>
>> /* -march=native handling only makes sense with compiler running on
>>an x86 or x86_64 chip.  If changing this condition, also change
>>the condition in driver-i386.c.  */
>> #if defined(__i386__) || defined(__x86_64__)
>> /* In driver-i386.c.  */
>> extern const char *host_detect_local_cpu (int argc, const char **argv);
>> #define EXTRA_SPEC_FUNCTIONS \
>>   { "local_cpu_detect", host_detect_local_cpu },
>> #define HAVE_LOCAL_CPU_DETECT
>> #endif
>>
>> Non-native hosts are obviously a niche case for x86, but it still
>> seems better to be consistent.
>>
>> Richard
>>
> 
> So would it enough to wrap 'candidates.safe_push ("native");' by
> #ifdef HAVE_LOCAL_CPU_DETECT
> ?
> 
> Thanks,
> Martin
> 

There's updated version of the patch I've just tested.

Martin
>From 8f1783a9017ec06c578fd644e46168ec5763d5ca Mon Sep 17 00:00:00 2001
From: marxin 
Date: Tue, 20 Feb 2018 14:21:05 +0100
Subject: [PATCH 3/3] Add "native" as a valid option value for -march= on i386
 (PR driver/83193).

gcc/ChangeLog:

2018-02-20  Martin Liska  

	PR driver/83193
	* config/i386/i386.c (ix86_option_override_internal):
	Add "native" as a possible value.
---
 gcc/config/i386/i386.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index d54e7301e84..9f2c5218ae5 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -4193,6 +4193,11 @@ ix86_option_override_internal (bool main_args_p,
 		|| ((processor_alias_table[i].flags & PTA_64BIT) != 0)))
 	  candidates.safe_push (processor_alias_table[i].name);
 
+#ifdef HAVE_LOCAL_CPU_DETECT
+  /* Add also "native" as possible value.  */
+  candidates.safe_push ("native");
+#endif
+
   char *s;
   const char *hint
 	= candidates_list_and_hint (opts->x_ix86_arch_string, s, candidates);
-- 
2.16.1

Re: [PATCH][i386] Adjust vec_construct cost for AVX256/512, penaltize elementwise load vectorization

[Kirill Yukhin]

Hello Richard,
On 14 фев 11:26, Richard Biener wrote:
> 
> The following tries to account for the fact that when constructing
> AVX256 or AVX512 vectors from elements we can only use insertps to
> insert into the low 128bits of a vector but have to use
> vinserti128 or vinserti64x4 to build larger AVX256/512 vectors.
> Those operations also have higher latency (Agner documents
> 3 cycles for Broadwell for reg-reg vinserti128 while insertps has
> one cycle latency).  Agner doesn't have tables for AVX512 yet but
> I guess the story is similar for vinserti64x4.
> 
> Latency is similar for FP adds so I re-used ix86_cost->addss for
> this cost.
> 
> This works towards fixing the referenced PRs below where we end
> up vectorizing a lot of loads via elementwise construction, mostly
> "enabled" by the new support for alias versioning for variable
> strides.  Here, analyzed for PR84037, the large number of scalar
> loads and vector builds before any meaningful computation means
> the CPU is bottlenecked with AGU and load ops and doesn't get
> any meaningful work done thus the vectorization should end up
> being not profitable (with some more massaging in the vectorizer
> and using SLP which reduces the number of loads a lot I only
> can get into same-speed as not vectorized territory).
> 
> So the real fix for those issues is to account for those
> microarchitectural issues in the backend costing.  I've decided
> to plumb this onto the vector construction op if that happens
> to be fed by loads, scaling this cost by the number of
> vector elements (overall latency should grow with the number
> of dependences).
> 
> Bootstrap/regtest running on x86_64-unknown-linux-gnu.
> 
> I've benchmarked this on Haswell with SPEC CPU 2006 and a three-run
> reveals that it doesn't regress any benchmark off-noise but improves
> 416.gamess by 7%, 465.tonto by 6% and 481.wrf by 2%.  It also fixes
> the Polyhedron capacita regression (which is what I "tuned" the
> factoring with).  I've mentioned the bugs refering any of the above
> affected benchmarks in the ChangeLog but it still has to be verified
> if the bugs are fully fixed (84037 is).
> 
> Ok for trunk?
Your patch is OK for trunk.

--
Thanks, K

> 
> Any confirmation of the microarchitectural bottleneck in, say,
> Capacita from people with access to cycle-accurate simulators
> are welcome ;)  Performance counters only help so much (not much...),
> so my guesses are based on Agner and finger-counting.
> 
> Thanks,
> Richard.
> 
> 2018-02-13  Richard Biener  
> 
>   PR tree-optimization/84037
>   PR tree-optimization/84016
>   PR target/82862
>   * config/i386/i386.c (ix86_builtin_vectorization_cost):
>   Adjust vec_construct for the fact we need additional higher latency
>   128bit inserts for AVX256 and AVX512 vector builds.
>   (ix86_add_stmt_cost): Scale vector construction cost for
>   elementwise loads.
> 
> Index: gcc/config/i386/i386.c
> ===
> --- gcc/config/i386/i386.c(revision 257620)
> +++ gcc/config/i386/i386.c(working copy)
> @@ -45904,7 +45904,18 @@ ix86_builtin_vectorization_cost (enum ve
> ix86_cost->sse_op, true);
>  
>case vec_construct:
> - return ix86_vec_cost (mode, ix86_cost->sse_op, false);
> + {
> +   /* N element inserts.  */
> +   int cost = ix86_vec_cost (mode, ix86_cost->sse_op, false);
> +   /* One vinserti128 for combining two SSE vectors for AVX256.  */
> +   if (GET_MODE_BITSIZE (mode) == 256)
> + cost += ix86_vec_cost (mode, ix86_cost->addss, true);
> +   /* One vinserti64x4 and two vinserti128 for combining SSE
> +  and AVX256 vectors to AVX512.  */
> +   else if (GET_MODE_BITSIZE (mode) == 512)
> + cost += 3 * ix86_vec_cost (mode, ix86_cost->addss, true);
> +   return cost;
> + }
>  
>default:
>  gcc_unreachable ();
> @@ -50243,6 +50254,18 @@ ix86_add_stmt_cost (void *data, int coun
> break;
>   }
>  }
> +  /* If we do elementwise loads into a vector then we are bound by
> + latency and execution resources for the many scalar loads
> + (AGU and load ports).  Try to account for this by scaling the
> + construction cost by the number of elements involved.  */
> +  if (kind == vec_construct
> +  && stmt_info
> +  && stmt_info->type == load_vec_info_type
> +  && stmt_info->memory_access_type == VMAT_ELEMENTWISE)
> +{
> +  stmt_cost = ix86_builtin_vectorization_cost (kind, vectype, misalign);
> +  stmt_cost *= TYPE_VECTOR_SUBPARTS (vectype);
> +}
>if (stmt_cost == -1)
>  stmt_cost = ix86_builtin_vectorization_cost (kind, vectype, misalign);
>  

Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478, take 2 and 3)

[Jeff Law]

On 02/20/2018 02:34 PM, Jakub Jelinek wrote:
> On Tue, Feb 20, 2018 at 01:13:13PM -0700, Martin Sebor wrote:
>> A safer and even more conservative alternative that should be
>> equivalent to your approach while avoiding the sprintf regressions
>> is to add another mode to the function and have it clear *minlen
>> as an option.  This lets the strlen code obtain the conservative
>> lower bound without compromising the sprintf warnings.
> 
> I fail to see what it would be good for to set *MINLEN to zero and
> *MAXLEN to all ones for the non-warning use cases, we simply don't know
> anything about it, both NULL_TREEs i.e. returning false is better.  I'm
> offering two alternate patches which use
> fuzzy == 0 for the previous !fuzzy, fuzzy == 1 for conservatively correct
> code that assumes strlen can't cross field/variable boundaries in
> compliant programs and fuzzy == 2 which does that + whatever the warning
> code wants.  Additionally, I've rewritten the COND_EXPR handling, so that
> it matches exactly the PHI handling.
> 
> The first patch doesn't change the 2 argument get_range_strlen and changes
> gimple_fold_builtin_strlen to use the 6 argument one, the second patch
> changes also the 2 argument get_range_strlen similarly to what you've done
> in your patch.
> 
> Tested on x86_64-linux and i686-linux, ok for trunk if it passes
> bootstrap/regtest?  Which one?
Just to be clear, the encapsulation proposal is a gcc-9 thingie.  I
don't think we need that to move forward.

jeff

Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478, take 2 and 3)

[Jeff Law]

On 02/20/2018 02:34 PM, Jakub Jelinek wrote:
> On Tue, Feb 20, 2018 at 01:13:13PM -0700, Martin Sebor wrote:
>> A safer and even more conservative alternative that should be
>> equivalent to your approach while avoiding the sprintf regressions
>> is to add another mode to the function and have it clear *minlen
>> as an option.  This lets the strlen code obtain the conservative
>> lower bound without compromising the sprintf warnings.
> 
> I fail to see what it would be good for to set *MINLEN to zero and
> *MAXLEN to all ones for the non-warning use cases, we simply don't know
> anything about it, both NULL_TREEs i.e. returning false is better.  I'm
> offering two alternate patches which use
> fuzzy == 0 for the previous !fuzzy, fuzzy == 1 for conservatively correct
> code that assumes strlen can't cross field/variable boundaries in
> compliant programs and fuzzy == 2 which does that + whatever the warning
> code wants.  Additionally, I've rewritten the COND_EXPR handling, so that
> it matches exactly the PHI handling.
> 
> The first patch doesn't change the 2 argument get_range_strlen and changes
> gimple_fold_builtin_strlen to use the 6 argument one, the second patch
> changes also the 2 argument get_range_strlen similarly to what you've done
> in your patch.
> 
> Tested on x86_64-linux and i686-linux, ok for trunk if it passes
> bootstrap/regtest?  Which one?
I'd lean towards the second -- essentially trying to keep the 6 operand
version for internal (recursive) use only.

In  my mind I'd like to encapsulate the 6 operand version so that it
can't be directly called and the only public interface is the 3 operand
version using strict/no-strict.

Let's go with that.  We can try to clean this up further during gcc-9.

Jeff

Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478)

[Jeff Law]

On 02/20/2018 12:03 PM, Martin Sebor wrote:
>> The thing that isn't handled conservatively is PHIs and COND_EXPR.
>> The current code, if we can't figure one of the args out, for PHIs in
>> fuzzy mode increases the *maxval value to +INF, but doesn't touch
>> *minval, for COND_EXPR doesn't adjust the *minval/*maxval at all and just
>> returns false.  Unfortunately, changing that breaks
> 
> It sounds like not setting *minlen is the problem then.  Attached
> is a slightly smaller patch that fixes the bug with no testsuite
> regressions on x86_64-linux.  How does it look to you?
What I don't like here is that we ultimately continue to use the two
operand get_range_strlen from the folder.  Meaning we're asking for
fuzzy results in a code generation path.

I'd lean more towards a solution that always gives conservatively
correct results in the codegen path while allowing fuzzy on the warning
paths.

Jeff

Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478)

[Jeff Law]

On 02/20/2018 01:13 PM, Martin Sebor wrote:
> On 02/20/2018 12:03 PM, Martin Sebor wrote:
>> On 02/20/2018 10:17 AM, Jakub Jelinek wrote:
>>> Hi!
>>>
>>> The following testcase distilled from pdftex is miscompiled on i?86,
>>> because gimple_fold_builtin_strlen sets incorrect value range on
>>> strlen call on SSA_NAME with def_stmt of PHI <"mu", something> where
>>> we can't determine anything about string length of something, so the
>>> right value range is [0, maximum_possible_strlen], but we actually used
>>> [2, INF].
>>>
>>> get_range_strlen has many modes, for get_maxval_strlen we check return
>>> value of get_range_strlen, don't set fuzzy and everything seems correct.
>>> Otherwise we have the fuzzy mode which is used in 2 arg get_range_strlen
>>> overload, which is used for warnings and recently also for
>>> gimple_fold_builtin_strlen which sets value ranges from it.  Unlike
>>> warnings, which can perhaps afford some heuristics and guessing, for
>>> gimple_fold_builtin_strlen we need to be 100% conservative.
>>>
>>> The thing that isn't handled conservatively is PHIs and COND_EXPR.
>>> The current code, if we can't figure one of the args out, for PHIs in
>>> fuzzy mode increases the *maxval value to +INF, but doesn't touch
>>> *minval, for COND_EXPR doesn't adjust the *minval/*maxval at all and
>>> just
>>> returns false.  Unfortunately, changing that breaks
>>
>> It sounds like not setting *minlen is the problem then.  Attached
>> is a slightly smaller patch that fixes the bug with no testsuite
>> regressions on x86_64-linux.  How does it look to you?
> 
> A safer and even more conservative alternative that should be
> equivalent to your approach while avoiding the sprintf regressions
> is to add another mode to the function and have it clear *minlen
> as an option.  This lets the strlen code obtain the conservative
> lower bound without compromising the sprintf warnings.
Adding another mode to this function seems wrong to me -- it's already a
bit of a tangled mess to figure out.  ISTM that either we allow fuzzy
results or not.  For warnings, fuzzy rules. For code generation fuzzy is
strictly not allowed.

Is there some reason that will not work?

jeff

Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478)

[Jeff Law]

On 02/20/2018 05:14 PM, Jakub Jelinek wrote:
> On Tue, Feb 20, 2018 at 04:59:12PM -0700, Martin Sebor wrote:
>>> It would help if you explained why you think it is a good idea
>>> ignoring the other phi arguments if you have one (or more) where you can
>>> determine length.
>>
>> It's a heuristic that was meant just for the -Wformat-overflow
>> warning.  When making decisions that affect code generation it's
>> obviously not correct to ignore the possibility that unknown
>> arguments may be shorter than the minimum or longer than
>> the maximum.  The fuzzy argument was meant to differentiate
>> between two got but I forgot about it when I added the fix
>> for PR 83671.
> 
> get_range_strlen (the 2 argument one) is right now called:
> 3 times from builtins.c for -Wstringop-overflow
> once from gimple-ssa-sprintf.c for -Wformat-overflow
> once from tree-ssa-strlen.c for -Wstringop-truncation
> once from gimple-fold.c for gimple_fold_builtin_strlen value ranges
Presumably it's the last one that's causing problems and were we should
focus our effort.


> 
> So, which of these do you want the heuristics for?  All 3 warnings,
> just one of them, something else?  In the 2 patches I've posted last
> all the 3 different warnings are treated the same (fuzzy == 2).
> 
> Looking at strlenopt-40.c testcase, in the test you clearly rely on
> fuzzy argument being set for the value ranges case
> (gimple_fold_builtin_strlen), otherwise many of the
> subtests would fail.
Which tests specifically?  I did a real quick scan and didn't see
anything in there that depended on incorrect behavior for the call from
gimple_fold_builtin_strlen.  BUt it was a quick scan and I could have
missed something.

jeff

Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478)

[Jeff Law]

On 02/20/2018 04:59 PM, Martin Sebor wrote:
>> It would help if you explained why you think it is a good idea
>> ignoring the other phi arguments if you have one (or more) where you can
>> determine length.
> 
> It's a heuristic that was meant just for the -Wformat-overflow
> warning.  When making decisions that affect code generation it's
> obviously not correct to ignore the possibility that unknown
> arguments may be shorter than the minimum or longer than
> the maximum.  The fuzzy argument was meant to differentiate
> between two got but I forgot about it when I added the fix
> for PR 83671.
> 
> For GCC 8 I don't have a preference for how to fix this as long
> as it doesn't regress the warning tests.
> 
> I think the ultimate solution (for GCC 9) may be to either
> disable the heuristic for code generation purposes (e.g., via
> another argument/flag) or provide a pointer argument to indicate
> to the caller that the minimum is based on known strings, and that
> the real minimum may be zero.
I'm still getting refamiliar with this code.  But one thing that jumps
out immediately is how much this reminds me of the discussion we had
around 77608 -- where I argued that returning something that was not
conservatively correct was just asking for long term problems.

I realize we're talking about different routines, but the concerns are
the same -- when we return something that is not conservatively correct
it's easy for someone to mistakenly use those results for code
generation purposes.

The fuzzy stuff is in there to reduce the false positive rates and we're
not *supposed* to be using fuzzy results for code generation purposes,
but as I argued in 77608, it's easy to miss.

I'll reiterate my desire to make this kind of mistake harder to make.

Jeff

Re: [PR middle-end/82123] 00/06 Use EVRP range data in sprintf warnings

[Jeff Law]

On 02/20/2018 05:00 PM, Joseph Myers wrote:
> Does this help with any of the cases in bug 80776 that weren't already 
> fixed, or are those distinct despite looking similar?
> 
I don't think so.

THe __builtin_unreachable markers are removed by vrp1 -- well before the
sprintf warning code gets run.

So the sprintf warning code never gets to exploit the properties implied
by the __builtin_unreachable calls.


It doesn't look like VRP records the narrowed ranges implied by the
__builtin_unreachable calls.

After ASSERT_EXPR insertion we have:

;;   basic block 6, loop depth 0, count 1072883002 (estimated locally),
maybe hot
;;prev block 5, next block 1, flags: (NEW, REACHABLE, VISITED)
;;pred:   4 [100.0% (guessed)]  count:1072883003 (estimated
locally) (FALSE_VALUE,EXECUTABLE)
  i_7 = ASSERT_EXPR ;
  __builtin___sprintf_chk (, 1, 7, "%d", i_7);
  return;

ANd the ranges computed by VRP:


i.0_1: [0, 99]
i_4: [0, +INF]
i_6: [0, +INF]  EQUIVALENCES: { i_4 } (1 elements)
i_7: [0, 99]  EQUIVALENCES: { i_4 i_6 } (2 elements)

So VRP does identify the narrow range for i_7.

But then we remove the ASSERT_EXPRs and we're left with:


  i_4 = somerandom ();
  i.0_1 = (unsigned int) i_4;
  __builtin___sprintf_chk (, 1, 7, "%d", i_4);
  return;

Subsequent EVRP analysis will start with the range of i_4 as a seed.
BUt there's nothing to further narrow that range.

If ASSERT_EXPR removal could be taught to use i_7 I suspect the right
things would "just happen".  I haven't thought at all about what might
be required to have VRP do-the-right-thing.  Given the overall desire to
drop ASSERT_EXPRs and the range propagation step in VRP in favor of EVRP
style analysis I doubt anyone is likely to spend much time on fixing
this in the old style VRP analysis.

jeff

Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478)

[Martin Sebor]

On 02/20/2018 05:14 PM, Jakub Jelinek wrote:

On Tue, Feb 20, 2018 at 04:59:12PM -0700, Martin Sebor wrote:

It would help if you explained why you think it is a good idea
ignoring the other phi arguments if you have one (or more) where you can
determine length.


It's a heuristic that was meant just for the -Wformat-overflow
warning.  When making decisions that affect code generation it's
obviously not correct to ignore the possibility that unknown
arguments may be shorter than the minimum or longer than
the maximum.  The fuzzy argument was meant to differentiate
between two got but I forgot about it when I added the fix
for PR 83671.


get_range_strlen (the 2 argument one) is right now called:
3 times from builtins.c for -Wstringop-overflow
once from gimple-ssa-sprintf.c for -Wformat-overflow
once from tree-ssa-strlen.c for -Wstringop-truncation


Right.  The warnings depend on the heuristic and on using
a non-zero lower bound when some of the strings is unknown
or has unknown length but other don't.


once from gimple-fold.c for gimple_fold_builtin_strlen value ranges


Right.  This depends on it too, except it needs to avoid
using a nonzero lower bound when any of the strings is
unknown/has unknown length.  Let's fix that.



So, which of these do you want the heuristics for?  All 3 warnings,
just one of them, something else?  In the 2 patches I've posted last
all the 3 different warnings are treated the same (fuzzy == 2).


All of them.  The bug is in how strlen is folded.  There is
nothing wrong with the warnings, so fix the bug and leave
the warnings alone.  My second patch did that and it passed
all tests.  (I forgot to include the change to
gimple-ssa-sprintf.c but it was the same as the one in
tree-ssa-strlen.c:  call get_range_strlen with true as
the third argument. Attached is the same patch with that
bit added.)


Looking at strlenopt-40.c testcase, in the test you clearly rely on
fuzzy argument being set for the value ranges case
(gimple_fold_builtin_strlen), otherwise many of the
subtests would fail.


Right, that needs to continue to work.  It does with my patch.
If your solution regresses any of the tests then use the second
patch I posted.

What am I missing?

Martin
PR tree-optimization/84478 - [8 Regression] pdftex miscompilation on i386

gcc/ChangeLog:

	PR tree-optimization/84478
	* gimple-fold.h (get_range_strlen): Add argument.
	* gimple-fold.c (get_range_strlen): Set *MINLEN to zero.
	(get_range_strlen): Reset range on failure.
	* gimple-ssa-sprintf.c (get_string_length): Add third argument
	to the call to get_range_strlen.
	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Same.

gcc/testsuite/ChangeLog:

	PR tree-optimization/84478
	* gcc.c-torture/execute/pr84478.c: New test.

Index: gcc/gimple-fold.c
===
--- gcc/gimple-fold.c	(revision 257859)
+++ gcc/gimple-fold.c	(working copy)
@@ -1290,6 +1290,9 @@ gimple_fold_builtin_memset (gimple_stmt_iterator *
When FUZZY is set and the length of a string cannot be determined,
the function instead considers as the maximum possible length the
size of a character array it may refer to.
+   When STRICT_MIN is set the function will clear MINMAXLEN[0] if
+   the length of any of the referenced strings cannot be determined
+   (and thus can be zero).
Set *FLEXP to true if the range of the string lengths has been
obtained from the upper bound of an array at the end of a struct.
Such an array may hold a string that's longer than its upper bound
@@ -1297,7 +1300,7 @@ gimple_fold_builtin_memset (gimple_stmt_iterator *
 
 static bool
 get_range_strlen (tree arg, tree length[2], bitmap *visited, int type,
-		  bool fuzzy, bool *flexp)
+		  bool fuzzy, bool strict_min, bool *flexp)
 {
   tree var, val = NULL_TREE;
   gimple *def_stmt;
@@ -1320,7 +1323,8 @@ get_range_strlen (tree arg, tree length[2], bitmap
 	  if (TREE_CODE (aop0) == INDIRECT_REF
 		  && TREE_CODE (TREE_OPERAND (aop0, 0)) == SSA_NAME)
 		return get_range_strlen (TREE_OPERAND (aop0, 0),
-	 length, visited, type, fuzzy, flexp);
+	 length, visited, type, fuzzy,
+	 strict_min, flexp);
 	}
 	  else if (TREE_CODE (TREE_OPERAND (op, 0)) == COMPONENT_REF && fuzzy)
 	{
@@ -1354,7 +1358,7 @@ get_range_strlen (tree arg, tree length[2], bitmap
 	{
 	  if (TREE_CODE (arg) == ADDR_EXPR)
 	return get_range_strlen (TREE_OPERAND (arg, 0), length,
- visited, type, fuzzy, flexp);
+ visited, type, fuzzy, strict_min, flexp);
 
 	  if (TREE_CODE (arg) == ARRAY_REF)
 	{
@@ -1497,14 +1501,17 @@ get_range_strlen (tree arg, tree length[2], bitmap
 || gimple_assign_unary_nop_p (def_stmt))
   {
 tree rhs = gimple_assign_rhs1 (def_stmt);
-	return get_range_strlen (rhs, length, visited, type, fuzzy, flexp);
+	return get_range_strlen (rhs, length, visited, type, fuzzy,
+ strict_min, flexp);
   }
 	else if 

[PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478, take 2 and 3)

[Jakub Jelinek]

On Tue, Feb 20, 2018 at 01:13:13PM -0700, Martin Sebor wrote:
> A safer and even more conservative alternative that should be
> equivalent to your approach while avoiding the sprintf regressions
> is to add another mode to the function and have it clear *minlen
> as an option.  This lets the strlen code obtain the conservative
> lower bound without compromising the sprintf warnings.

I fail to see what it would be good for to set *MINLEN to zero and
*MAXLEN to all ones for the non-warning use cases, we simply don't know
anything about it, both NULL_TREEs i.e. returning false is better.  I'm
offering two alternate patches which use
fuzzy == 0 for the previous !fuzzy, fuzzy == 1 for conservatively correct
code that assumes strlen can't cross field/variable boundaries in
compliant programs and fuzzy == 2 which does that + whatever the warning
code wants.  Additionally, I've rewritten the COND_EXPR handling, so that
it matches exactly the PHI handling.

The first patch doesn't change the 2 argument get_range_strlen and changes
gimple_fold_builtin_strlen to use the 6 argument one, the second patch
changes also the 2 argument get_range_strlen similarly to what you've done
in your patch.

Tested on x86_64-linux and i686-linux, ok for trunk if it passes
bootstrap/regtest?  Which one?

Jakub
2018-02-20  Jakub Jelinek  
Martin Sebor  

PR tree-optimization/84478
* gimple-fold.c (get_range_strlen): Make minlen const and assume it
can't be NULL.  Change FUZZY from bool to int, for 1 add PHI/COND_EXPR
support which is conservatively correct, for 2 only stay conservative
for maxlen.  Formatting and comment capitalization fixes.  Add warning
that the 2 argument get_range_strlen is only usable for warnings, adjust
6 arg get_range_strlen caller and clear minmaxlen[0] and [1] if it
returned false.
(get_maxval_strlen): Adjust 6 arg get_range_strlen caller.
(gimple_fold_builtin_strlen): Use the 6 arg get_range_strlen overload
rather than 2 arg, use it only if it returns true and flexarray is
false, pass 3 as type to it.

* gcc.c-torture/execute/pr84478.c: New test.

--- gcc/gimple-fold.c.jj2018-02-19 19:57:03.424279589 +0100
+++ gcc/gimple-fold.c   2018-02-20 22:03:47.595265756 +0100
@@ -1283,13 +1283,16 @@ gimple_fold_builtin_memset (gimple_stmt_
value of ARG in LENGTH[0] and LENGTH[1], respectively.
If ARG is an SSA name variable, follow its use-def chains.  When
TYPE == 0, if LENGTH[1] is not equal to the length we determine or
-   if we are unable to determine the length or value, return False.
+   if we are unable to determine the length or value, return false.
VISITED is a bitmap of visited variables.
TYPE is 0 if string length should be obtained, 1 for maximum string
length and 2 for maximum value ARG can have.
-   When FUZZY is set and the length of a string cannot be determined,
+   When FUZZY is non-zero and the length of a string cannot be determined,
the function instead considers as the maximum possible length the
-   size of a character array it may refer to.
+   size of a character array it may refer to.  If FUZZY is 2, it will handle
+   PHIs and COND_EXPRs optimistically, if we can determine string length
+   minimum and maximum, it will use the minimum from the ones where it
+   can be determined.
Set *FLEXP to true if the range of the string lengths has been
obtained from the upper bound of an array at the end of a struct.
Such an array may hold a string that's longer than its upper bound
@@ -1297,14 +1300,13 @@ gimple_fold_builtin_memset (gimple_stmt_
 
 static bool
 get_range_strlen (tree arg, tree length[2], bitmap *visited, int type,
- bool fuzzy, bool *flexp)
+ int fuzzy, bool *flexp)
 {
   tree var, val = NULL_TREE;
   gimple *def_stmt;
 
-  /* The minimum and maximum length.  The MAXLEN pointer stays unchanged
- but MINLEN may be cleared during the execution of the function.  */
-  tree *minlen = length;
+  /* The minimum and maximum length.  */
+  tree *const minlen = length;
   tree *const maxlen = length + 1;
 
   if (TREE_CODE (arg) != SSA_NAME)
@@ -1445,12 +1447,11 @@ get_range_strlen (tree arg, tree length[
   if (!val)
return false;
 
-  if (minlen
- && (!*minlen
- || (type > 0
- && TREE_CODE (*minlen) == INTEGER_CST
- && TREE_CODE (val) == INTEGER_CST
- && tree_int_cst_lt (val, *minlen
+  if (!*minlen
+ || (type > 0
+ && TREE_CODE (*minlen) == INTEGER_CST
+ && TREE_CODE (val) == INTEGER_CST
+ && tree_int_cst_lt (val, *minlen)))
*minlen = val;
 
   if (*maxlen)
@@ -1501,20 +1502,26 @@ get_range_strlen (tree arg, tree length[
   }
else if (gimple_assign_rhs_code (def_stmt) == COND_EXPR)
  

Re: RFA: Sanitize deprecation messages (PR 84195)

[Martin Sebor]

On 02/20/2018 05:48 AM, Nick Clifton wrote:

Hi Martin,



Since the class manages a resource it should ideally make sure it
doesn't try to release the same resource multiple times.  I.e., its
copy constructor and assignment operators should either "do the right
thing" (whatever you think that is) or be made inaccessible (or declared 
deleted in C++ 11).

For example:

  {
escaped_string a;
a.escape ("foo\nbar");

escaped_string b (a);
// b destroys its m_str
// double free in a's destructor here
  }


I am not sure that this can happen.  First of the escaped_string
class does not have constructor that accepts a char* argument.
(Maybe in C++ this is done automatically ?  My C++-fu is very weak).


It doesn't have a converting ctor and the above example doesn't
use one.  It uses the implicitly defined copy constructor to
create a copy of A in B.



Secondly the m_owned private field is only set to true when
the m_str field is set to a string allocated by the particular
instance of the class, and memory is only freed by the destructor
if m_owned is true.


Right, and implicitly-defined copy constructor copies all the
fields so both the original and the copy wind up pointing to
the same allocated chunk and both with m_owned set to true.

The same thing happens when an object of the calss is assigned
to another, thanks to the implicitly-defined copy assignment
operator.


So even this should work:

  {
escaped_string a,b;

a.escape ("foo\nbar");
b.escape ((const char *) a);
  }


Yes, this works but this wouldn't:

  {
escaped_string a, b;

a.escape ("foo\nbar");
b = a;
  }   // double free in a's dtor here

What also wouldn't work right is the converse:

  {
escaped_string a, b;

a.escape ("foo\nbar");
a = b;   // a.m_str leaks here (it's reset by the assignment)
  }

Your patch works correctly because it doesn't use the copy
constructor or the copy assignment operator.  So this is
only a hypothetical concern, if a change in the code led
to one of the copy functions being called.

Hope this makes sense.

Martin



The destructor for B will not free any memory, even though its
m_str field has been set to the same string as A, because its
m_owned field will be set to FALSE.

Cheers
  Nick

Re: [PR middle-end/82123] 00/06 Use EVRP range data in sprintf warnings

[Joseph Myers]

Does this help with any of the cases in bug 80776 that weren't already 
fixed, or are those distinct despite looking similar?

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

Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478)

[Martin Sebor]

It would help if you explained why you think it is a good idea
ignoring the other phi arguments if you have one (or more) where you can
determine length.


It's a heuristic that was meant just for the -Wformat-overflow
warning.  When making decisions that affect code generation it's
obviously not correct to ignore the possibility that unknown
arguments may be shorter than the minimum or longer than
the maximum.  The fuzzy argument was meant to differentiate
between two got but I forgot about it when I added the fix
for PR 83671.

For GCC 8 I don't have a preference for how to fix this as long
as it doesn't regress the warning tests.

I think the ultimate solution (for GCC 9) may be to either
disable the heuristic for code generation purposes (e.g., via
another argument/flag) or provide a pointer argument to indicate
to the caller that the minimum is based on known strings, and that
the real minimum may be zero.

Martin

Re: plugin-api.h patch to add a new interface for linker plugins

[Cary Coutant]

> Ping.  Is this alright to apply now or should I wait for Stage 1?
>
> * plugin-api.h (ld_plugin_get_wrap_symbols): New
>   plugin interface.

I'd say go ahead and apply the patch in binutils, and wait for Stage 1
to sync back to GCC, unless someone there OKs it sooner.

Nick, is that OK?

-cary

Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478, take 2 and 3)

[Martin Sebor]

On 02/20/2018 02:34 PM, Jakub Jelinek wrote:

On Tue, Feb 20, 2018 at 01:13:13PM -0700, Martin Sebor wrote:

A safer and even more conservative alternative that should be
equivalent to your approach while avoiding the sprintf regressions
is to add another mode to the function and have it clear *minlen
as an option.  This lets the strlen code obtain the conservative
lower bound without compromising the sprintf warnings.


I fail to see what it would be good for to set *MINLEN to zero and
*MAXLEN to all ones for the non-warning use cases, we simply don't know
anything about it, both NULL_TREEs i.e. returning false is better.  I'm
offering two alternate patches which use
fuzzy == 0 for the previous !fuzzy, fuzzy == 1 for conservatively correct
code that assumes strlen can't cross field/variable boundaries in
compliant programs and fuzzy == 2 which does that + whatever the warning
code wants.  Additionally, I've rewritten the COND_EXPR handling, so that
it matches exactly the PHI handling.

The first patch doesn't change the 2 argument get_range_strlen and changes
gimple_fold_builtin_strlen to use the 6 argument one, the second patch
changes also the 2 argument get_range_strlen similarly to what you've done
in your patch.

Tested on x86_64-linux and i686-linux, ok for trunk if it passes
bootstrap/regtest?  Which one?


I don't have a preference as long as it doesn't introduce any
test suite regressions.  That's all I was trying to do in my
approach (besides fixing the bug).

Martin

[committed] Add testcase for PR c++/84488

[Jakub Jelinek]

Hi!

This testcase got fixed in r257155 aka PR83942, but unlike
the PR83942 testcase which regressed during 8.x stage1 this one used
to fail before too, so I think it is worth adding another testcase for it.

Tested on x86_64-linux, committed to trunk as obvious.

2018-02-20  Jakub Jelinek  

PR c++/84488
* g++.dg/warn/Wunused-var-30.C: New test.

--- gcc/testsuite/g++.dg/warn/Wunused-var-30.C.jj   2018-02-20 
17:53:26.539480455 +0100
+++ gcc/testsuite/g++.dg/warn/Wunused-var-30.C  2018-02-20 17:53:18.419481675 
+0100
@@ -0,0 +1,11 @@
+// PR c++/84488
+// { dg-do compile }
+// { dg-options "-Wunused-but-set-variable" }
+
+int
+foo ()
+{
+  enum E { A, B, C, D };
+  double r = 1.0;  // { dg-bogus "set but not used" }
+  return static_cast(r);
+}

Jakub

Re: [PATCH v2] RISC-V: Support for FreeBSD

[Jim Wilson]

On Sat, Feb 17, 2018 at 7:08 PM, Kito Cheng  wrote:
> Hi all:
>
> This patch is version 2 of FreeBSD supporting for RISC-V, Ruslan
> (RISC-V FreeBSD maintainer) and me has been tested on FreeBSD 12
> for building kernel and whole user space programs/libraries again :)

This looks OK.

> Copyright part, Ruslan has sent to fsf-reco...@gnu.org but no echo,
> I've forward that to ass...@gnu.org again.

I saw the email.  I checked the FSF copyright list and see that it
hasn't been recorded yet.  The FSF copyright clerk sometimes takes a
few weeks to respond.  Hopefully this will be competed on the FSF side
soon.

Jim

Re: [PATCH] libgcc: xtensa: fix build with -mtext-section-literals

[Max Filippov]

On Wed, Feb 7, 2018 at 2:10 PM, Max Filippov  wrote:
> On Thu, Feb 1, 2018 at 9:12 AM, Max Filippov  wrote:
>> On Wed, Jan 31, 2018 at 11:17 AM, Max Filippov  wrote:
>>> On Wed, Jan 31, 2018 at 11:11 AM, augustine.sterl...@gmail.com 
>>>  wrote:
 On Tue, Jan 30, 2018 at 8:02 PM, Max Filippov  wrote:
>
> libgcc/
> 2018-01-31  Max Filippov  
>
> * config/xtensa/ieee754-df.S (__adddf3_aux): Add
> .literal_position directive.
> * config/xtensa/ieee754-sf.S (__addsf3_aux): Likewise.

 This is fine, but when did it stop working, and why isn't there a test
 that would catch it?
>>>
>>> I broke it with the recent softfloat NaN fix. Haven't noticed that because
>>> I usually build without any flags or with -mauto-litpools.
>>> Let me add the test, thanks for the suggestion.
>>
>> I've taken a look at gcc testsuites and I don't see how to add a test that 
>> would
>> check that libgcc builds with different compiler options. Any suggestions?
>
> If there are no suggestions I will check in the fix as is and will add 
> building
> with -mtext-section-literals to my testing scripts.

I've checked the fix into the trunk, gcc-6 and gcc-7 branches.

-- 
Thanks.
-- Max

Re: Extend aligned_membuf<> usage

[François Dumont]

On 20/02/2018 20:59, Jonathan Wakely wrote:

On 8 February 2018 at 06:10, François Dumont wrote:

On 06/02/2018 20:16, François Dumont wrote:

On 05/02/2018 18:16, Jonathan Wakely wrote:

Wouldn't it make more sense to simply make __aligned_buffer identical
to __aligned_membuf for the versioned-namespace? Then at least the
conditional code is only in one place.


Yes, __aligned_buffer is indeed not used, we could do that. I'll propose a
patch to do so.


So here it is, ok to commit ?

OK, because it only affects the gnu-versioned-namespace mode.


Ok, done now.

Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478)

[Jakub Jelinek]

On Tue, Feb 20, 2018 at 12:03:26PM -0700, Martin Sebor wrote:
> PR tree-optimization/84478 - [8 Regression] pdftex miscompilation on i386
> 
> gcc/ChangeLog:
> 
>   PR tree-optimization/84478
>   * gimple-fold.c (get_range_strlen): Set *MINLEN to zero.
>   (get_range_strlen): Reset range on failure.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR tree-optimization/84478
>   * gcc.c-torture/execute/pr84478.c: New test.
> 
> Index: gcc/gimple-fold.c
> ===
> --- gcc/gimple-fold.c (revision 257796)
> +++ gcc/gimple-fold.c (working copy)
> @@ -1369,7 +1369,10 @@ get_range_strlen (tree arg, tree length[2], bitmap
> tree eltype = TREE_TYPE (type);
> if (TREE_CODE (type) != ARRAY_TYPE
> || !INTEGRAL_TYPE_P (eltype))
> - return false;
> + {
> +   *minlen = ssize_int (0);
> +   return false;
> + }

This is just one of the 13 spots where we return false, so this doesn't look
safe or sufficient to me, even when you actually honor the return value in
2 argument get_range_strlen.  You'd really need to do

  {
if (fuzzy)
- *maxlen = build_all_ones_cst (size_type_node);
+ {
+   *minlen = size_int (0);
+   *maxlen = build_all_ones_cst (size_type_node);
+ }
else
  return false;
  }

or just drop that if (fuzzy) stuff from there, but that breaks the warning
tests.  It would help if you explained why you think it is a good idea
ignoring the other phi arguments if you have one (or more) where you can
determine length.

One variation of my patch could be instead of adding type 3 change
fuzzy from bool to int, and use fuzzy == 1 for the strlen value ranges and
fuzzy == 2 for the warning code (i.e. 2 operand get_range_strlen).

Note, my patch passed regtest on both x86_64-linux and i686-linux.

Jakub

Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478)

[Martin Sebor]

On 02/20/2018 12:03 PM, Martin Sebor wrote:

On 02/20/2018 10:17 AM, Jakub Jelinek wrote:

Hi!

The following testcase distilled from pdftex is miscompiled on i?86,
because gimple_fold_builtin_strlen sets incorrect value range on
strlen call on SSA_NAME with def_stmt of PHI <"mu", something> where
we can't determine anything about string length of something, so the
right value range is [0, maximum_possible_strlen], but we actually used
[2, INF].

get_range_strlen has many modes, for get_maxval_strlen we check return
value of get_range_strlen, don't set fuzzy and everything seems correct.
Otherwise we have the fuzzy mode which is used in 2 arg get_range_strlen
overload, which is used for warnings and recently also for
gimple_fold_builtin_strlen which sets value ranges from it.  Unlike
warnings, which can perhaps afford some heuristics and guessing, for
gimple_fold_builtin_strlen we need to be 100% conservative.

The thing that isn't handled conservatively is PHIs and COND_EXPR.
The current code, if we can't figure one of the args out, for PHIs in
fuzzy mode increases the *maxval value to +INF, but doesn't touch
*minval, for COND_EXPR doesn't adjust the *minval/*maxval at all and just
returns false.  Unfortunately, changing that breaks


It sounds like not setting *minlen is the problem then.  Attached
is a slightly smaller patch that fixes the bug with no testsuite
regressions on x86_64-linux.  How does it look to you?


A safer and even more conservative alternative that should be
equivalent to your approach while avoiding the sprintf regressions
is to add another mode to the function and have it clear *minlen
as an option.  This lets the strlen code obtain the conservative
lower bound without compromising the sprintf warnings.

Martin
PR tree-optimization/84478 - [8 Regression] pdftex miscompilation on i386

gcc/ChangeLog:

	PR tree-optimization/84478
	* gimple-fold.h (get_range_strlen): Add argument.
	* gimple-fold.c (get_range_strlen): Set *MINLEN to zero.
	(get_range_strlen): Reset range on failure.
	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Add third argument
	to the call to get_range_strlen.

gcc/testsuite/ChangeLog:

	PR tree-optimization/84478
	* gcc.c-torture/execute/pr84478.c: New test.

Index: gcc/gimple-fold.c
===
--- gcc/gimple-fold.c	(revision 257850)
+++ gcc/gimple-fold.c	(working copy)
@@ -1290,6 +1290,9 @@ gimple_fold_builtin_memset (gimple_stmt_iterator *
When FUZZY is set and the length of a string cannot be determined,
the function instead considers as the maximum possible length the
size of a character array it may refer to.
+   When STRICT_MIN is set the function will clear MINMAXLEN[0] if
+   the length of any of the referenced strings cannot be determined
+   (and thus can be zero).
Set *FLEXP to true if the range of the string lengths has been
obtained from the upper bound of an array at the end of a struct.
Such an array may hold a string that's longer than its upper bound
@@ -1297,7 +1300,7 @@ gimple_fold_builtin_memset (gimple_stmt_iterator *
 
 static bool
 get_range_strlen (tree arg, tree length[2], bitmap *visited, int type,
-		  bool fuzzy, bool *flexp)
+		  bool fuzzy, bool strict_min, bool *flexp)
 {
   tree var, val = NULL_TREE;
   gimple *def_stmt;
@@ -1320,7 +1323,8 @@ get_range_strlen (tree arg, tree length[2], bitmap
 	  if (TREE_CODE (aop0) == INDIRECT_REF
 		  && TREE_CODE (TREE_OPERAND (aop0, 0)) == SSA_NAME)
 		return get_range_strlen (TREE_OPERAND (aop0, 0),
-	 length, visited, type, fuzzy, flexp);
+	 length, visited, type, fuzzy,
+	 strict_min, flexp);
 	}
 	  else if (TREE_CODE (TREE_OPERAND (op, 0)) == COMPONENT_REF && fuzzy)
 	{
@@ -1354,7 +1358,7 @@ get_range_strlen (tree arg, tree length[2], bitmap
 	{
 	  if (TREE_CODE (arg) == ADDR_EXPR)
 	return get_range_strlen (TREE_OPERAND (arg, 0), length,
- visited, type, fuzzy, flexp);
+ visited, type, fuzzy, strict_min, flexp);
 
 	  if (TREE_CODE (arg) == ARRAY_REF)
 	{
@@ -1497,14 +1501,17 @@ get_range_strlen (tree arg, tree length[2], bitmap
 || gimple_assign_unary_nop_p (def_stmt))
   {
 tree rhs = gimple_assign_rhs1 (def_stmt);
-	return get_range_strlen (rhs, length, visited, type, fuzzy, flexp);
+	return get_range_strlen (rhs, length, visited, type, fuzzy,
+ strict_min, flexp);
   }
 	else if (gimple_assign_rhs_code (def_stmt) == COND_EXPR)
 	  {
 	tree op2 = gimple_assign_rhs2 (def_stmt);
 	tree op3 = gimple_assign_rhs3 (def_stmt);
-	return get_range_strlen (op2, length, visited, type, fuzzy, flexp)
-	  && get_range_strlen (op3, length, visited, type, fuzzy, flexp);
+	return (get_range_strlen (op2, length, visited, type, fuzzy,
+  strict_min, flexp)
+		&& get_range_strlen (op3, length, visited, type, fuzzy,
+	 strict_min, flexp));
 	  }
 return false;
 
@@ -1527,10 

Re: Extend aligned_membuf<> usage

[Jonathan Wakely]

On 8 February 2018 at 06:10, François Dumont wrote:
> On 06/02/2018 20:16, François Dumont wrote:
>>
>> On 05/02/2018 18:16, Jonathan Wakely wrote:
>>>
>>> Wouldn't it make more sense to simply make __aligned_buffer identical
>>> to __aligned_membuf for the versioned-namespace? Then at least the
>>> conditional code is only in one place.
>>>
>> Yes, __aligned_buffer is indeed not used, we could do that. I'll propose a
>> patch to do so.
>>
> So here it is, ok to commit ?

OK, because it only affects the gnu-versioned-namespace mode.

Re: [patch, fortran] Fix character length in constructors

[Thomas Koenig]

Am 20.02.2018 um 20:10 schrieb Janne Blomqvist:

Shouldn't the second one be "stop 2"?


Corrected, r257859.

Regards

Thomas

Re: [RFC] Tree Loop Unroller Pass

[Andrew Pinski]

On Mon, Feb 12, 2018 at 3:55 PM, Kugan Vivekanandarajah
 wrote:
> Implements tree loop unroller using the infrastructure provided.
>
> gcc/ChangeLog:
>
> 2018-02-12  Kugan Vivekanandarajah  
>
> * Makefile.in (OBJS): Add tree-ssa-loop-unroll.o.
> * common.opt (ftree-loop-unroll): New option.
> * passes.def: Add pass_tree_loop_uroll
> * timevar.def (TV_TREE_LOOP_UNROLL): Add.
> * tree-pass.h (make_pass_tree_loop_unroll): Declare.
> * tree-ssa-loop-unroll.c: New file.


I think it was decided to name new gimple passes as gimple-* rather
than tree-ssa-*.
The option should most likely just take over -funrol-loops, etc.

Shouldn't:
+  if (targetm.hw_max_mem_read_streams
+  && (max_load_streams = targetm.hw_max_mem_read_streams ()) > 0)
+{
+  load_streams = count_mem_load_streams (loop, max_load_streams);
+  if (load_streams > 0)
+ {
+   signed t = 1 << (floor_log2 (max_load_streams / load_streams));
+   if (t < n_unroll)
+ n_unroll = t;
+ }
+}

this be a target hook instead of doing inline here.  It seems too
target specific notion of what a stream is.  Even the notion of a
stream here seems micro-arch specific and not generic enough.


A few more comments about the pass.
You don't check can_duplicate_loop_p on the loop.
You use optimize_function_for_size_p on the whole function instead of
checking if the loop is cold (by checking optimize_loop_for_size_p).

In my version of gimple loop unrolling, I had to add
lpt_decision.already_unrolled and mark the loop as already unrolled so
the rtl loop unroller would not happen again.
-fopt-report does not report when the unrolling has happened unlike
the RTL version.

Thanks,
Andrew

Re: [PATCH] RL78 one_cmplhi2 improvement

[DJ Delorie]

Const type promotion is the bane of embedded developers...

One thing to try is to use (subreg:QI in a define_expand, so that
there's a one_cmplhi2 pattern that expands to two QImode insns that
operate on HImode input/outputs via SUBREGs.

I don't have high hopes of gcc optimizing this properly in all cases,
but it's worth trying.

If it doesn't work out, consider this patch approved, though.

Thanks!

Re: Fix find_widening_optab_handler_and_mode assertion (PR 84406)

[Jeff Law]

On 02/20/2018 10:18 AM, Richard Sandiford wrote:
> r254302 tried to clean up find_widening_optab_handler_and_mode
> so that it really did only handle widening ops rather than
> simple single-mode ones.  But this PR shows that I'd fluffed
> the MODE_PARTIAL_INT handling.  It turns out that all four
> combinations of:
> 
>   {MODE_INT,MODE_PARTIAL_INT}->{MODE_INT,MODE_PARTIAL_INT}
> 
> are possible, and GET_MODE_WIDER_MODE for a MODE_PARTIAL_INT
> returns a MODE_INT (which makes sense in retrospect).
> 
> No test case since this is a build failure on affected targets.
> 
> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu.
> Also tested by Jozef on msp430-elf (thanks).  OK to install?
> 
> Richard
> 
> 
> 2018-02-20  Richard Sandiford  
> 
> gcc/
>   PR middle-end/84406
>   * optabs-query.c (find_widening_optab_handler_and_mode): If from_mode
>   is a scalar_int_mode, assert that to_mode is a scalar_int_mode with
>   greater precision.  If to_mode is a MODE_PARTIAL_INT, stop the
>   search at the associated MODE_INT.
THanks.  I went ahead and installed this.  My tester (which has a msp430
target) will pick this up on its next run.

jeff

Re: [patch, fortran] Fix character length in constructors

[Janne Blomqvist]

On Tue, Feb 20, 2018 at 8:59 PM, Thomas Koenig  wrote:
> Am 20.02.2018 um 08:51 schrieb Janne Blomqvist:
>
>> It's better to use gfc_mpz_get_hwi() instead of mpz_get_si().
>> mpz_get_si() returns a long, which is a 32-bit type on win64.
>>
>> Otherwise Ok with the spelling fix suggested by Steve.
>
>
> Committed as r257856.
>
> Thanks to you and Steve for the review and the heads_up about
> gfc_mpz_get_hwi.
>
> I also changed the "call abort" to "STOP 1" at the last second
> (old habits are hard to break, it seems :-)

Shouldn't the second one be "stop 2"?


-- 
Janne Blomqvist

Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478)

[Martin Sebor]

On 02/20/2018 10:17 AM, Jakub Jelinek wrote:

Hi!

The following testcase distilled from pdftex is miscompiled on i?86,
because gimple_fold_builtin_strlen sets incorrect value range on
strlen call on SSA_NAME with def_stmt of PHI <"mu", something> where
we can't determine anything about string length of something, so the
right value range is [0, maximum_possible_strlen], but we actually used
[2, INF].

get_range_strlen has many modes, for get_maxval_strlen we check return
value of get_range_strlen, don't set fuzzy and everything seems correct.
Otherwise we have the fuzzy mode which is used in 2 arg get_range_strlen
overload, which is used for warnings and recently also for
gimple_fold_builtin_strlen which sets value ranges from it.  Unlike
warnings, which can perhaps afford some heuristics and guessing, for
gimple_fold_builtin_strlen we need to be 100% conservative.

The thing that isn't handled conservatively is PHIs and COND_EXPR.
The current code, if we can't figure one of the args out, for PHIs in
fuzzy mode increases the *maxval value to +INF, but doesn't touch
*minval, for COND_EXPR doesn't adjust the *minval/*maxval at all and just
returns false.  Unfortunately, changing that breaks


It sounds like not setting *minlen is the problem then.  Attached
is a slightly smaller patch that fixes the bug with no testsuite
regressions on x86_64-linux.  How does it look to you?

Martin


FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 165)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 166)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 167)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 168)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 309)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 310)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 311)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 312)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for excess errors)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 62)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 63)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 402)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 403)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 430)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 431)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 458)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 459)
so this patch instead stops using the 2 argument get_range_strlen
in gimple_fold_builtin_strlen and makes sure we handle that case
conservatively, i.e. PHI of something known + unknown is unknown, likewise
COND_EXPR.

Bootstrapped on x86_64-linux and i686-linux, regtest pending, ok for trunk
if it passes regtest on both?

For stage1 I guess Martin can tweak the warning code paths and if they will
become the same as the conservatively correct ones, what is in this patch
can be adjusted again.

2018-02-20  Jakub Jelinek  

PR tree-optimization/84478
* gimple-fold.c (get_range_strlen): Make minlen const and assume it
can't be NULL.  Add type 3 support which is conservatively correct
in PHIs.  Formatting and comment capitalization fixes.  Add warning
that the 2 argument get_range_strlen is only usable for warnings.
(gimple_fold_builtin_strlen): Use the 6 arg get_range_strlen overload
rather than 2 arg, use it only if it returns true and flexarray is
false, pass 3 as type to it.

* gcc.c-torture/execute/pr84478.c: New test.

--- gcc/gimple-fold.c.jj2018-02-19 19:57:03.424279589 +0100
+++ gcc/gimple-fold.c   2018-02-20 16:21:34.583020305 +0100
@@ -1283,10 +1283,11 @@ gimple_fold_builtin_memset (gimple_stmt_
value of ARG in LENGTH[0] and LENGTH[1], respectively.
If ARG is an SSA name variable, follow its use-def chains.  When
TYPE == 0, if LENGTH[1] is not equal to the length we determine or
-   if we are unable to determine the length or value, return False.
+   if we are unable to determine the length or value, return false.
VISITED is a bitmap of visited variables.
TYPE is 0 if string length should be obtained, 1 for maximum string
-   length and 2 for maximum value ARG can have.
+   length and 2 for maximum value ARG can have, 3 is like 1, but provide
+   conservatively correct rather than optimistic answer.
When FUZZY is set and the length of a string cannot be determined,
the function instead considers as the maximum possible length the
size of a character array it may refer to.
@@ -1302,9 +1303,8 @@ get_range_strlen (tree arg, tree length[
   tree var, val = NULL_TREE;
   gimple *def_stmt;

-  /* 

[PATCH][committed][PR middle-end/82123] 06/06 Use EVRP range data in another query

[Jeff Law]

The final patch, and the one that actually fixes 82123/81592.  This
replaces the final query of global range information with a query into
the EVRP range analyzer and adds the new tests.

Bootstrapped and regression tested on x86_64-linux-gnu.

Jeff
* gimple-ssa-sprintf.c (format_integer): Query EVRP range analyzer
for range data rather than using global data.
 
* gcc.dg/pr81592.c: New test.
* gcc.dg/pr82123.c: New test.

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index b3ffaec..1189d9f 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -1451,12 +1451,13 @@ format_integer (const directive , tree arg, 
vr_values *vr_values)
 {
   /* Try to determine the range of values of the integer argument
 (range information is not available for pointers).  */
-  wide_int min, max;
-  enum value_range_type range_type = get_range_info (arg, , );
-  if (range_type == VR_RANGE)
+  value_range *vr = vr_values->get_value_range (arg);
+  if (vr->type == VR_RANGE
+ && TREE_CODE (vr->min) == INTEGER_CST
+ && TREE_CODE (vr->max) == INTEGER_CST)
{
- argmin = wide_int_to_tree (argtype, min);
- argmax = wide_int_to_tree (argtype, max);
+ argmin = vr->min;
+ argmax = vr->max;
 
  /* Set KNOWNRANGE if the argument is in a known subrange
 of the directive's type and neither width nor precision
@@ -1469,11 +1470,12 @@ format_integer (const directive , tree arg, 
vr_values *vr_values)
  res.argmin = argmin;
  res.argmax = argmax;
}
-  else if (range_type == VR_ANTI_RANGE)
+  else if (vr->type == VR_ANTI_RANGE)
{
  /* Handle anti-ranges if/when bug 71690 is resolved.  */
}
-  else if (range_type == VR_VARYING)
+  else if (vr->type == VR_VARYING
+  || vr->type == VR_UNDEFINED)
{
  /* The argument here may be the result of promoting the actual
 argument to int.  Try to determine the type of the actual
commit d7ab4f0e1f049f2d9bce8c5f579ce3c837bcc29c
Author: Jeff Law 
Date:   Tue Feb 20 13:38:48 2018 -0500

Tests

diff --git a/gcc/testsuite/gcc.dg/pr81592.c b/gcc/testsuite/gcc.dg/pr81592.c
new file mode 100644
index 000..a37703a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr81592.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall -fno-strict-overflow  -Wstrict-overflow=2 
-fsanitize=signed-integer-overflow" } */
+
+#include 
+
+int proc_keys_show(long expiry, long now)
+{
+   unsigned long timo;
+   char xbuf[4];
+
+   if (now < expiry) {
+   timo = expiry - now;
+   if (timo < 60)
+   sprintf(xbuf, "%lus", timo);
+   }
+
+   return 0;
+}
+
+
diff --git a/gcc/testsuite/gcc.dg/pr82123.c b/gcc/testsuite/gcc.dg/pr82123.c
new file mode 100644
index 000..34109f1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr82123.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wformat-overflow=1" } */
+
+void acpi_gpiochip_request_interrupt(unsigned short s)
+{
+char name[3];
+   unsigned int pin = s;
+
+   if (pin <= 255)
+   __builtin_sprintf(name, "%02X", pin);
+}
+

Re: [patch, fortran] Fix character length in constructors

[Thomas Koenig]

Am 20.02.2018 um 08:51 schrieb Janne Blomqvist:


It's better to use gfc_mpz_get_hwi() instead of mpz_get_si().
mpz_get_si() returns a long, which is a 32-bit type on win64.

Otherwise Ok with the spelling fix suggested by Steve.


Committed as r257856.

Thanks to you and Steve for the review and the heads_up about
gfc_mpz_get_hwi.

I also changed the "call abort" to "STOP 1" at the last second
(old habits are hard to break, it seems :-)

Regards

Thomas

[PATCH][committed][PR tree-optimization/82123] 05/06 Use EVRP range data in get_int_range.

[Jeff Law]

This is the 5th patch in the series.  It updates another global range
query to instead use the EVRP range data.  Bootstrapped and regression
tested on x86_64-linux-gnu.

Jeff
* gimple-ssa-sprintf.c (get_int_range): Query EVRP range analyzer for
range data rather than using global data.

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 54c9132..b3ffaec 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -1149,9 +1149,10 @@ get_int_range (tree arg, HOST_WIDE_INT *pmin, 
HOST_WIDE_INT *pmax,
  && TYPE_PRECISION (argtype) <= TYPE_PRECISION (type))
{
  /* Try to determine the range of values of the integer argument.  */
- wide_int min, max;
- enum value_range_type range_type = get_range_info (arg, , );
- if (range_type == VR_RANGE)
+ value_range *vr = vr_values->get_value_range (arg);
+ if (vr->type == VR_RANGE
+ && TREE_CODE (vr->min) == INTEGER_CST
+ && TREE_CODE (vr->max) == INTEGER_CST)
{
  HOST_WIDE_INT type_min
= (TYPE_UNSIGNED (argtype)
@@ -1160,8 +1161,8 @@ get_int_range (tree arg, HOST_WIDE_INT *pmin, 
HOST_WIDE_INT *pmax,
 
  HOST_WIDE_INT type_max = tree_to_uhwi (TYPE_MAX_VALUE (argtype));
 
- *pmin = min.to_shwi ();
- *pmax = max.to_shwi ();
+ *pmin = TREE_INT_CST_LOW (vr->min);
+ *pmax = TREE_INT_CST_LOW (vr->max);
 
  if (*pmin < *pmax)
{

[PATCH][committed][PR tree-optimization/82123] 04/06 Make vr_values available where needed

[Jeff Law]

This is the 4th patch in the series.  It doesn't change the analysis in
any way shape or form.  It just arranges to pass around the range data
to the points where we're going to need it.

Bootstrapped and regression tested on x86_64-linux-gnu.

Jeff
commit e2b702e7bcb00c2197776691735ff8a599dce4e1
Author: Jeff Law 
Date:   Mon Feb 19 17:53:40 2018 -0500

Push vr-values to where it needs to be

* gimple-ssa-sprintf.c (get_int_range): Accept vr_values parameter
pass it to children as needed.
(struct directive::fmtresult): Similarly.
(struct directive::set_width): Similarly.
(struct directive::set_precision): Similarly.
(format_integer, format_directive, parse_directive): Similarly.
(format_none): Accept unnamed vr_values parameter.
(format_percent, format_floating, format_character): Similarly.
(format_string, format_plain): Similarly.

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 4b2de6d..54c9132 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -771,7 +771,8 @@ fmtresult::type_max_digits (tree type, int base)
 }
 
 static bool
-get_int_range (tree, HOST_WIDE_INT *, HOST_WIDE_INT *, bool, HOST_WIDE_INT);
+get_int_range (tree, HOST_WIDE_INT *, HOST_WIDE_INT *, bool, HOST_WIDE_INT,
+  class vr_values *vr_values);
 
 /* Description of a format directive.  A directive is either a plain
string or a conversion specification that starts with '%'.  */
@@ -806,7 +807,7 @@ struct directive
 
   /* Format conversion function that given a directive and an argument
  returns the formatting result.  */
-  fmtresult (*fmtfunc) (const directive &, tree);
+  fmtresult (*fmtfunc) (const directive &, tree, vr_values *);
 
   /* Return True when a the format flag CHR has been used.  */
   bool get_flag (char chr) const
@@ -843,9 +844,9 @@ struct directive
  or 0, whichever is greater.  For a non-constant ARG in some range
  set width to its range adjusting each bound to -1 if it's less.
  For an indeterminate ARG set width to [0, INT_MAX].  */
-  void set_width (tree arg)
+  void set_width (tree arg, vr_values *vr_values)
   {
-get_int_range (arg, width, width + 1, true, 0);
+get_int_range (arg, width, width + 1, true, 0, vr_values);
   }
 
   /* Set both bounds of the precision range to VAL.  */
@@ -859,9 +860,9 @@ struct directive
  or -1 whichever is greater.  For a non-constant ARG in some range
  set precision to its range adjusting each bound to -1 if it's less.
  For an indeterminate ARG set precision to [-1, INT_MAX].  */
-  void set_precision (tree arg)
+  void set_precision (tree arg, vr_values *vr_values)
   {
-get_int_range (arg, prec, prec + 1, false, -1);
+get_int_range (arg, prec, prec + 1, false, -1, vr_values);
   }
 
   /* Return true if both width and precision are known to be
@@ -1042,7 +1043,7 @@ struct sprintf_dom_walker::call_info
 /* Return the result of formatting a no-op directive (such as '%n').  */
 
 static fmtresult
-format_none (const directive &, tree)
+format_none (const directive &, tree, vr_values *)
 {
   fmtresult res (0);
   return res;
@@ -1051,7 +1052,7 @@ format_none (const directive &, tree)
 /* Return the result of formatting the '%%' directive.  */
 
 static fmtresult
-format_percent (const directive &, tree)
+format_percent (const directive &, tree, vr_values *)
 {
   fmtresult res (1);
   return res;
@@ -1108,7 +1109,8 @@ build_intmax_type_nodes (tree *pintmax, tree *puintmax)
 
 static bool
 get_int_range (tree arg, HOST_WIDE_INT *pmin, HOST_WIDE_INT *pmax,
-  bool absolute, HOST_WIDE_INT negbound)
+  bool absolute, HOST_WIDE_INT negbound,
+  class vr_values *vr_values)
 {
   /* The type of the result.  */
   const_tree type = integer_type_node;
@@ -1179,7 +1181,8 @@ get_int_range (tree arg, HOST_WIDE_INT *pmin, 
HOST_WIDE_INT *pmax,
   /* Handle an argument with an unknown range as if none had been
 provided.  */
   if (unknown)
-   return get_int_range (NULL_TREE, pmin, pmax, absolute, negbound);
+   return get_int_range (NULL_TREE, pmin, pmax, absolute,
+ negbound, vr_values);
 }
 
   /* Adjust each bound as specified by ABSOLUTE and NEGBOUND.  */
@@ -1264,7 +1267,7 @@ adjust_range_for_overflow (tree dirtype, tree *argmin, 
tree *argmax)
used when the directive argument or its value isn't known.  */
 
 static fmtresult
-format_integer (const directive , tree arg)
+format_integer (const directive , tree arg, vr_values *vr_values)
 {
   tree intmax_type_node;
   tree uintmax_type_node;
@@ -1482,7 +1485,7 @@ format_integer (const directive , tree arg)
  if (code == INTEGER_CST)
{
  arg = gimple_assign_rhs1 (def);
- return format_integer (dir, arg);
+ return format_integer (dir, arg, vr_values);
 

[PATCH][committed][PR tree-optimization/82123] 03/06 Query EVRP range data in sprintf warning pass

[Jeff Law]

This is the 3rd patch in the series.  It updates one of the 3 range
queries to start using the EVRP computed range data.  This one is
trivial as it's occurring in a member function where we have direct
access to the range analyzer data.

Bootstrapped and regression tested on x86_64-linux-gnu.

Jeff
* gimple-ssa-sprintf.c (sprintf_dom_walker::handle_gimple_call): Query
the EVRP range analyzer for range data rather than using global data.


diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 545f833..4b2de6d 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -3903,16 +3903,13 @@ sprintf_dom_walker::handle_gimple_call 
(gimple_stmt_iterator *gsi)
  /* Try to determine the range of values of the argument
 and use the greater of the two at level 1 and the smaller
 of them at level 2.  */
- wide_int min, max;
- enum value_range_type range_type
-   = get_range_info (size, , );
- if (range_type == VR_RANGE)
-   {
- dstsize
-   = (warn_level < 2
-  ? wi::fits_uhwi_p (max) ? max.to_uhwi () : max.to_shwi ()
-  : wi::fits_uhwi_p (min) ? min.to_uhwi () : min.to_shwi ());
-   }
+ value_range *vr = evrp_range_analyzer.get_value_range (size);
+ if (vr->type == VR_RANGE
+ && TREE_CODE (vr->min) == INTEGER_CST
+ && TREE_CODE (vr->max) == INTEGER_CST)
+   dstsize = (warn_level < 2
+  ? TREE_INT_CST_LOW (vr->max)
+  : TREE_INT_CST_LOW (vr->min));
 
  /* The destination size is not constant.  If the function is
 bounded (e.g., snprintf) a lower bound of zero doesn't

[PATCH][committed][PR tree-optimization/82123] 02/06 Perform EVRP analysis in sprintf warning pass

[Jeff Law]

This is the second patch in the series to fix 82123/81592.  It generates
range information within the sprintf warning pass using the EVRP
analyzer.  We don't use the range information in this patch.

This twiddles one test -- adding the calls into the analyzer from the
sprintf pass causes us to record a range for an object that didn't have
one before and compromises the test.  I just turn off VRP  which is
sufficient to obscure things so that we don't remove the
__builtin_unreachable.


Bootstrapped and regression tested on x86_64-linux-gnu.

Jeff
* gimple-ssa-sprintf.c: Include alloc-pool.h, vr-values.h and 
gimple-ssa-evrp-analyze.h
(class sprintf_dom_walker): Add after_dom_children member function.
Add evrp_range_analyzer member.
(sprintf_dom_walker::before_dom_children): Call into the EVRP
range analyzer as needed.
(sprintf_dom_walker::after_dom_children): New member function.

* gcc.dg/builtin-unreachable-6.c: Turn off VRP.

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index a2dd545..545f833 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -80,6 +80,9 @@ along with GCC; see the file COPYING3.  If not see
 #include "substring-locations.h"
 #include "diagnostic.h"
 #include "domwalk.h"
+#include "alloc-pool.h"
+#include "vr-values.h"
+#include "gimple-ssa-evrp-analyze.h"
 
 /* The likely worst case value of MB_LEN_MAX for the target, large enough
for UTF-8.  Ideally, this would be obtained by a target hook if it were
@@ -121,10 +124,12 @@ class sprintf_dom_walker : public dom_walker
   ~sprintf_dom_walker () {}
 
   edge before_dom_children (basic_block) FINAL OVERRIDE;
+  void after_dom_children (basic_block) FINAL OVERRIDE;
   bool handle_gimple_call (gimple_stmt_iterator *);
 
   struct call_info;
   bool compute_format_length (call_info &, format_result *);
+  class evrp_range_analyzer evrp_range_analyzer;
 };
 
 class pass_sprintf_length : public gimple_opt_pass
@@ -3456,7 +3461,7 @@ parse_directive (sprintf_dom_walker::call_info ,
 
 bool
 sprintf_dom_walker::compute_format_length (call_info ,
-   format_result *res)
+  format_result *res)
 {
   if (dump_file)
 {
@@ -4012,11 +4017,15 @@ sprintf_dom_walker::handle_gimple_call 
(gimple_stmt_iterator *gsi)
 edge
 sprintf_dom_walker::before_dom_children (basic_block bb)
 {
+  evrp_range_analyzer.enter (bb);
   for (gimple_stmt_iterator si = gsi_start_bb (bb); !gsi_end_p (si); )
 {
   /* Iterate over statements, looking for function calls.  */
   gimple *stmt = gsi_stmt (si);
 
+  /* First record ranges generated by this statement.  */
+  evrp_range_analyzer.record_ranges_from_stmt (stmt, false);
+
   if (is_gimple_call (stmt) && handle_gimple_call ())
/* If handle_gimple_call returns true, the iterator is
   already pointing to the next statement.  */
@@ -4027,6 +4036,12 @@ sprintf_dom_walker::before_dom_children (basic_block bb)
   return NULL;
 }
 
+void
+sprintf_dom_walker::after_dom_children (basic_block bb)
+{
+  evrp_range_analyzer.leave (bb);
+}
+
 /* Execute the pass for function FUN.  */
 
 unsigned int
diff --git a/gcc/testsuite/gcc.dg/builtin-unreachable-6.c 
b/gcc/testsuite/gcc.dg/builtin-unreachable-6.c
index 1915dd1..b0504be 100644
--- a/gcc/testsuite/gcc.dg/builtin-unreachable-6.c
+++ b/gcc/testsuite/gcc.dg/builtin-unreachable-6.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-fab1 -fno-tree-dominator-opts" } */
+/* { dg-options "-O2 -fdump-tree-fab1 -fno-tree-dominator-opts -fno-tree-vrp" 
} */
 
 void
 foo (int b, int c)

[PATCH][committed][PR tree-optimization/82123] 01/06 Do nothing in EVRP analyzer is not optimizing

[Jeff Law]

This is part #1 of the patches to fix 81592/82123.  The changes aren't
particularly large or invasive, but I already had them broken down
internally into distinct chunks, so I'm going to send them out that way.

This patch allows the EVRP range analyzer to be safely called even when
not optimizing.  It does no analysis in that case.   This prevents
problems if we were to ask for sprintf warnings but not have the
optimizer enabled.

Bootstrapped and regression tested on x86_64-linux-gnu.

Jeff
* gimple-ssa-evrp-analyze.c (evrp_range_analyzer::enter): Do nothing
if not optimizing.
(evrp_range_analyzer::record_ranges_from_stmt): Likewise.
(evrp_range_analyzer::pop_to_marker): Likewise.

diff --git a/gcc/gimple-ssa-evrp-analyze.c b/gcc/gimple-ssa-evrp-analyze.c
index 2eb2769..b9dcf90 100644
--- a/gcc/gimple-ssa-evrp-analyze.c
+++ b/gcc/gimple-ssa-evrp-analyze.c
@@ -69,6 +69,8 @@ evrp_range_analyzer::push_marker ()
 void
 evrp_range_analyzer::enter (basic_block bb)
 {
+  if (!optimize)
+return;
   push_marker ();
   record_ranges_from_incoming_edge (bb);
   record_ranges_from_phis (bb);
@@ -279,6 +281,9 @@ evrp_range_analyzer::record_ranges_from_stmt (gimple *stmt, 
bool temporary)
 {
   tree output = NULL_TREE;
 
+  if (!optimize)
+return;
+
   if (dyn_cast  (stmt))
 ;
   else if (stmt_interesting_for_vrp (stmt))
@@ -390,6 +395,8 @@ evrp_range_analyzer::pop_to_marker (void)
 void
 evrp_range_analyzer::leave (basic_block bb ATTRIBUTE_UNUSED)
 {
+  if (!optimize)
+return;
   pop_to_marker ();
 }
 

[PR middle-end/82123] 00/06 Use EVRP range data in sprintf warnings

[Jeff Law]

So I'm finally getting back to this.

To recap, the issue here is the sprintf pass is querying global range
information.  As a result the range for the key object is not narrowed
by the conditionals leading to the sprintf call and we get a false
positive (pr81592 and pr82123).

The plan for the last few months was to use the embeddable range
analyzer to get the narrowed range.  I'd dropped in a bit of
infrastructure to do that a while back, but got side tracked by spectre
and meltdown before I could push it to completion.

This patch completes the work.  It ties the range analyzer into the
dominator walk (which was trivial) and arranges to query the range
analyzer rather than the global data.

The patch is bigger than one might expect primarily because the points
where we want to issue the queries are in free functions rather than in
member functions.

Rather than go through a round of refactoring to bring those free
functions into the class hierarchy, I just passed around the range data.
 It's the same trivial change in a bunch of places to pass vr_values
down.  So it looks big, but is dead simple in reality.

We query the data in 3 places in the fairly obvious way.  My initial
patch actually created a get_range_info member function within vr_values
that had the same signature as the free function get_range_info  in
tree-ssanames.c

That works, but ultimately I decided it was more confusing than using
the existing get_value_range member function.  So this version uses the
existing get_value_range member function.

The fix is broken into a half-dozen patches in my local tree.  I didn't
see any value in squashing them together -- so I'm posting them as a 6
series kit, even though they're pretty simple.

Bootstrapped and regression tested on x86_64-linux-gnu.  Verified it
fixes both 81592 and 82123.

Jeff

Re: [Fortran, PATCH, coarray, v1] Extend caf_*_by_ref () API by a type specifier

[Damian Rouson]

Hi Andre,

Thanks for your latest work on CAF features.  Could you let us know whether 
this commit should be tested against the OpenCoarrays master branch or another 
branch?  With the master branch, I get one test failure (not counting two known 
teams failures that are actually false negatives that I need to fix):

lib_caf_mpi::sendget_by_ref(): Warning ! sendget_by_ref() is mostly 
unfunctional due to a design error. Split up your statement with coarray refs 
on both sides of the assignment when the datatype transfered is non 
4-byte-integer compatible.
libcaf_mpi RUNTIME ERROR: Cannot convert type 1 kind 4 to type 0 kind 4

Is the above expected?  Also, because the message comes from sendget, does that 
mean it only affects lines that involve three images such as the following:

if (this_image()==1) x[2] = x[3]


Damian

On February 19, 2018 at 9:32:06 AM, Andre Vehreschild (ve...@gmx.de) wrote:

Hi all,  

no objections received therefore committed as r257813. Thanks for fast review  
Jerry.  

- Andre  

On Sun, 18 Feb 2018 18:33:07 +0100  
Andre Vehreschild  wrote:  

> Well, after discussing on IRC whether RM should be bothered, I was asked to  
> simplify release managers lives and propose, that if no one objects within 
> one  
> day, I will merge the patch. So any objections?  
>  
> - Andre  
>  
> On Sun, 18 Feb 2018 18:07:28 +0100  
> Andre Vehreschild  wrote:  
>  
> > Dear release managers,  
> >  
> > this patch (for reference  
> > https://gcc.gnu.org/ml/fortran/2018-02/msg00124.html) fixes a regression in 
> >  
> > the coarray api by extending three relatively new functions with one or two 
> >  
> > arguments, respectively. The patch has been approved by gfortran devs.  
> > Asking your approval to merge it: Ok to merge to trunk?  
> >  
> > Regards,  
> > Andre  
> >  
> > On Sun, 18 Feb 2018 08:53:41 -0800  
> > Jerry DeLisle  wrote:  
> >  
> > > On 02/18/2018 07:39 AM, Andre Vehreschild wrote:  
> > > > Hi all,  
> > > >  
> > > > attached patch fixes an issue with the coarray API. When a component of 
> > > >  
> > > > a derived type coarray was referenced using a caf_*_by_ref () function  
> > > > and that component was not an array with a descriptor, then the type of 
> > > >  
> > > > the component was not known. Which additionally meant, that type  
> > > > conversion was not applied as required. This patch fixes that issue by  
> > > > adding type specifiers to the three caf_*_by_ref-calls and implements  
> > > > the functionality for libcaf_single. This is harmless because other  
> > > > coarray libraries that do not expect this argument just ignore it.  
> > > > Additionally does this patch also provide the first working version of  
> > > > caf_sendget_by_ref in libcaf_single, which previously only lead to a  
> > > > stack corruption and was not usable since the array descriptor rework  
> > > > (nice job, btw).  
> > > >  
> > > > I would like to have this patch in trunk knowing that I am somewhat  
> > > > late, but it would be quite necessary, because as it is now, the  
> > > > coarray feature for derived types is hardly usable. Furthermore do some 
> > > >  
> > > > people name this a regression, because the caf_*_by_ref are also used  
> > > > when the lhs of a caf_get_by_ref() is allocatable which now does not  
> > > > work as expected anymore but before gcc-6 using caf_get() (w/o  
> > > > reallocation) did.  
> > > >  
> > > > Bootstrapped and regtested ok on x86_64-linux/f27. Ok for trunk?  
> > > >  
> > > > - Andre  
> > > >  
> > >  
> > > This is OK from the Fortranners perspective. Should touch base with  
> > > release manager. It looks harmless though it changes coarray API, which  
> > > is hidden behind -fcoarray=  
> > >  
> > > Regards,  
> > >  
> > > Jerry  
> >  
> >  
>  
>  


--  
Andre Vehreschild * Email: vehre ad gmx dot de  

[PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478)

[Jakub Jelinek]

Hi!

The following testcase distilled from pdftex is miscompiled on i?86,
because gimple_fold_builtin_strlen sets incorrect value range on
strlen call on SSA_NAME with def_stmt of PHI <"mu", something> where
we can't determine anything about string length of something, so the
right value range is [0, maximum_possible_strlen], but we actually used
[2, INF].

get_range_strlen has many modes, for get_maxval_strlen we check return
value of get_range_strlen, don't set fuzzy and everything seems correct.
Otherwise we have the fuzzy mode which is used in 2 arg get_range_strlen
overload, which is used for warnings and recently also for
gimple_fold_builtin_strlen which sets value ranges from it.  Unlike
warnings, which can perhaps afford some heuristics and guessing, for
gimple_fold_builtin_strlen we need to be 100% conservative.

The thing that isn't handled conservatively is PHIs and COND_EXPR.
The current code, if we can't figure one of the args out, for PHIs in
fuzzy mode increases the *maxval value to +INF, but doesn't touch
*minval, for COND_EXPR doesn't adjust the *minval/*maxval at all and just
returns false.  Unfortunately, changing that breaks
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 165)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 166)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 167)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 168)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 309)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 310)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 311)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c  (test for warnings, line 312)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for excess errors)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 62)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 63)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 402)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 403)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 430)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 431)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 458)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c  (test for warnings, line 459)
so this patch instead stops using the 2 argument get_range_strlen
in gimple_fold_builtin_strlen and makes sure we handle that case
conservatively, i.e. PHI of something known + unknown is unknown, likewise
COND_EXPR.

Bootstrapped on x86_64-linux and i686-linux, regtest pending, ok for trunk
if it passes regtest on both?

For stage1 I guess Martin can tweak the warning code paths and if they will
become the same as the conservatively correct ones, what is in this patch
can be adjusted again.

2018-02-20  Jakub Jelinek  

PR tree-optimization/84478
* gimple-fold.c (get_range_strlen): Make minlen const and assume it
can't be NULL.  Add type 3 support which is conservatively correct
in PHIs.  Formatting and comment capitalization fixes.  Add warning
that the 2 argument get_range_strlen is only usable for warnings.
(gimple_fold_builtin_strlen): Use the 6 arg get_range_strlen overload
rather than 2 arg, use it only if it returns true and flexarray is
false, pass 3 as type to it.

* gcc.c-torture/execute/pr84478.c: New test.

--- gcc/gimple-fold.c.jj2018-02-19 19:57:03.424279589 +0100
+++ gcc/gimple-fold.c   2018-02-20 16:21:34.583020305 +0100
@@ -1283,10 +1283,11 @@ gimple_fold_builtin_memset (gimple_stmt_
value of ARG in LENGTH[0] and LENGTH[1], respectively.
If ARG is an SSA name variable, follow its use-def chains.  When
TYPE == 0, if LENGTH[1] is not equal to the length we determine or
-   if we are unable to determine the length or value, return False.
+   if we are unable to determine the length or value, return false.
VISITED is a bitmap of visited variables.
TYPE is 0 if string length should be obtained, 1 for maximum string
-   length and 2 for maximum value ARG can have.
+   length and 2 for maximum value ARG can have, 3 is like 1, but provide
+   conservatively correct rather than optimistic answer.
When FUZZY is set and the length of a string cannot be determined,
the function instead considers as the maximum possible length the
size of a character array it may refer to.
@@ -1302,9 +1303,8 @@ get_range_strlen (tree arg, tree length[
   tree var, val = NULL_TREE;
   gimple *def_stmt;
 
-  /* The minimum and maximum length.  The MAXLEN pointer stays unchanged
- but MINLEN may be cleared during the execution of the function.  */
-  tree *minlen = length;
+  /* The minimum and maximum length.  */
+  tree *const minlen = length;
  

Re: [PATCH] Add "native" as a valid option value for -march= on i386 (PR driver/83193).

[Jakub Jelinek]

On Tue, Feb 20, 2018 at 03:58:07PM +, Richard Sandiford wrote:
> Martin Liška  writes:
> > Hi.
> >
> > Following patch adds "native" as a possible option for -march value on
> > i386 target.  I have similar patches for other targets. Would it be
> > possible to install the patch in current stage?
> 
> [...]
> 
> > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> > index d54e7301e84..361d4df2663 100644
> > --- a/gcc/config/i386/i386.c
> > +++ b/gcc/config/i386/i386.c
> > @@ -4193,6 +4193,9 @@ ix86_option_override_internal (bool main_args_p,
> > || ((processor_alias_table[i].flags & PTA_64BIT) != 0)))
> >   candidates.safe_push (processor_alias_table[i].name);
> >  
> > +  /* Add also "native" as possible value.  */
> > +  candidates.safe_push ("native");
> > +
> >char *s;
> >const char *hint
> > = candidates_list_and_hint (opts->x_ix86_arch_string, s, candidates);
> 
> We should probably only do this if the driver supports -march=native.
> I think for x86 that means HAVE_LOCAL_CPU_DETECT.

Isn't the option available always?  It just doesn't do anything if
/* If we are compiling with GCC where %EBX register is fixed, then the
   driver will just ignore -march and -mtune "native" target and will leave
   to the newly built compiler to generate code for its default target.  */

Jakub

Re: plugin-api.h patch to add a new interface for linker plugins

[Sriraman Tallam via gcc-patches]

Ping.  Is this alright to apply now or should I wait for Stage 1?

* plugin-api.h (ld_plugin_get_wrap_symbols): New
  plugin interface.

Thanks
Sri

On Thu, Feb 15, 2018 at 1:52 PM, Sriraman Tallam  wrote:
> Ping,  this patch was approved for binutils by Cary:
> https://sourceware.org/ml/binutils/2017-12/msg00023.html
>
> Is it ok to apply this to GCC include/plugin-api.h now?  It is a
> pretty small change. Patch attached.
>
> * plugin-api.h (ld_plugin_get_wrap_symbols): New
>   plugin interface.
>
>
> Thanks
> Sri
>
> On Fri, Dec 8, 2017 at 11:02 AM, Sriraman Tallam  wrote:
>> Patch attached.
>>
>> * plugin-api.h (ld_plugin_get_wrap_symbols): New
>>   plugin interface.
>>
>> On Fri, Dec 8, 2017 at 11:01 AM, Sriraman Tallam  wrote:
>>> Hi,
>>>
>>>This patch was approved for binutils by Cary:
>>> https://sourceware.org/ml/binutils/2017-12/msg00023.html
>>>
>>>Is it ok to apply this to GCC include/plugin-api.h ?
>>>
>>> Thanks
>>> Sri
* plugin-api.h (ld_plugin_get_wrap_symbols): New
  plugin interface.

Index: include/plugin-api.h
===
--- include/plugin-api.h(revision 255515)
+++ include/plugin-api.h(working copy)
@@ -378,7 +378,15 @@
 enum ld_plugin_status
 (*ld_plugin_register_new_input) (ld_plugin_new_input_handler handler);
 
+/* The linker's interface for getting the list of wrapped symbols using the
+   --wrap option. This sets *NUM_SYMBOLS to number of wrapped symbols and
+   *WRAP_SYMBOL_LIST to the list of wrapped symbols. */
 
+typedef
+enum ld_plugin_status
+(*ld_plugin_get_wrap_symbols) (uint64_t *num_symbols,
+   const char ***wrap_symbol_list);
+
 enum ld_plugin_level
 {
   LDPL_INFO,
@@ -422,7 +430,8 @@
   LDPT_GET_SYMBOLS_V3 = 28,
   LDPT_GET_INPUT_SECTION_ALIGNMENT = 29,
   LDPT_GET_INPUT_SECTION_SIZE = 30,
-  LDPT_REGISTER_NEW_INPUT_HOOK = 31
+  LDPT_REGISTER_NEW_INPUT_HOOK = 31,
+  LDPT_GET_WRAP_SYMBOLS = 32
 };
 
 /* The plugin transfer vector.  */
@@ -457,6 +466,7 @@
 ld_plugin_get_input_section_alignment tv_get_input_section_alignment;
 ld_plugin_get_input_section_size tv_get_input_section_size;
 ld_plugin_register_new_input tv_register_new_input;
+ld_plugin_get_wrap_symbols tv_get_wrap_symbols;
   } tv_u;
 };
 

[Patch] Document __builtin_extend_pointer

[Steve Ellcey]

While working on PR 83335 I proposed a change to a test case that
used __builtin_extend_pointer and Richared Earnshaw pointed out
that this builtin is not documented.  Since I could not find any
other (reasonable) way to generate an extended address in inline
assembly other than this builtin I would like to document it for
use.

Here is a proposed patch, the one problem I found was the return
type of the builtin.  I don't know how to describe it other than
Pmode, but that is not a user visible type.

See https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01051.html
for my PR 83335 patch and follow up comments.

Should I go ahead and add this documentation?


2018-02-20  Steve Ellcey  

* doc/extend.texi (__builtin_extend_pointer): Document builtin.


diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index d38840e..94e47aa 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -11042,6 +11042,7 @@ the built-in function returns -1.
 @findex __builtin_alloca_with_align
 @findex __builtin_alloca_with_align_and_max
 @findex __builtin_call_with_static_chain
+@findex __builtin_extend_pointer
 @findex __builtin_fpclassify
 @findex __builtin_isfinite
 @findex __builtin_isnormal
@@ -12419,6 +12420,15 @@ Similar to @code{__builtin_bswap32}, except the 
argument and return types
 are 64 bit.
 @end deftypefn
 
+@deftypefn {Built-in Function} Pmode __builtin_extend_pointer (void * x)
+On targets where the user visible pointer size is different than the size
+of an actual hardware address this function returns the extended user
+pointer.  Targets where this is true included ILP32 mode on x86_64 or
+Aarch64.  This function is mainly useful when writing inline assembly
+code.
+@var{addr}
+@end deftypefn
+
 @node Target Builtins
 @section Built-in Functions Specific to Particular Target Machines
 

Fix find_widening_optab_handler_and_mode assertion (PR 84406)

[Richard Sandiford]

r254302 tried to clean up find_widening_optab_handler_and_mode
so that it really did only handle widening ops rather than
simple single-mode ones.  But this PR shows that I'd fluffed
the MODE_PARTIAL_INT handling.  It turns out that all four
combinations of:

  {MODE_INT,MODE_PARTIAL_INT}->{MODE_INT,MODE_PARTIAL_INT}

are possible, and GET_MODE_WIDER_MODE for a MODE_PARTIAL_INT
returns a MODE_INT (which makes sense in retrospect).

No test case since this is a build failure on affected targets.

Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu.
Also tested by Jozef on msp430-elf (thanks).  OK to install?

Richard


2018-02-20  Richard Sandiford  

gcc/
PR middle-end/84406
* optabs-query.c (find_widening_optab_handler_and_mode): If from_mode
is a scalar_int_mode, assert that to_mode is a scalar_int_mode with
greater precision.  If to_mode is a MODE_PARTIAL_INT, stop the
search at the associated MODE_INT.

Index: gcc/optabs-query.c
===
--- gcc/optabs-query.c  2018-01-13 18:01:51.236735252 +
+++ gcc/optabs-query.c  2018-02-20 17:16:11.783836883 +
@@ -473,9 +473,23 @@ find_widening_optab_handler_and_mode (op
  machine_mode from_mode,
  machine_mode *found_mode)
 {
-  gcc_checking_assert (GET_MODE_CLASS (from_mode) == GET_MODE_CLASS (to_mode));
-  gcc_checking_assert (from_mode < to_mode);
-  FOR_EACH_MODE (from_mode, from_mode, to_mode)
+  machine_mode limit_mode = to_mode;
+  if (is_a  (from_mode))
+{
+  gcc_checking_assert (is_a  (to_mode)
+  && known_lt (GET_MODE_PRECISION (from_mode),
+   GET_MODE_PRECISION (to_mode)));
+  /* The modes after FROM_MODE are all MODE_INT, so the only
+MODE_PARTIAL_INT mode we consider is FROM_MODE itself.
+If LIMIT_MODE is MODE_PARTIAL_INT, stop at the containing
+MODE_INT.  */
+  if (GET_MODE_CLASS (limit_mode) == MODE_PARTIAL_INT)
+   limit_mode = GET_MODE_WIDER_MODE (limit_mode).require ();
+}
+  else
+gcc_checking_assert (GET_MODE_CLASS (from_mode) == GET_MODE_CLASS (to_mode)
+&& from_mode < to_mode);
+  FOR_EACH_MODE (from_mode, from_mode, limit_mode)
 {
   enum insn_code handler = convert_optab_handler (op, to_mode, from_mode);
 

Make fix for PR 83965 handle SLP reduction chains

[Richard Sandiford]

This patch prevents pattern-matching of fold-left SLP reduction chains,
which the previous patch for 83965 didn't handle properly.  It only
stops the last statement in the group from being matched, but that's
enough to cause the group to be dissolved later.

A better fix would be to put all the information about the reduction
on the the first statement in the reduction chain, so that every
statement in the group can tell what the group is doing.  That doesn't
seem like stage 4 material though.

As it stands, things seem to be a bit of a mess.  In
vect_force_simple_reduction we attach the reduction type and
phi pointer to the last statement in a reduction chain:

  reduc_def_info = vinfo_for_stmt (def);
  STMT_VINFO_REDUC_TYPE (reduc_def_info) = v_reduc_type;
  STMT_VINFO_REDUC_DEF (reduc_def_info) = phi;

and mark it as vect_reduction_type in vect_analyze_scalar_cycles_1:

  STMT_VINFO_DEF_TYPE (vinfo_for_stmt (reduc_stmt)) =
   vect_reduction_def;

This code in vectorizable_reduction gave the impression that
everything really is keyed off the last statement:

  /* In case of reduction chain we switch to the first stmt in the chain, but
 we don't update STMT_INFO, since only the last stmt is marked as reduction
 and has reduction properties.  */
  if (GROUP_FIRST_ELEMENT (stmt_info)
  && GROUP_FIRST_ELEMENT (stmt_info) != stmt)
{
  stmt = GROUP_FIRST_ELEMENT (stmt_info);
  first_p = false;
}

But this code is dead these days.  GROUP_FIRST_ELEMENT is only nonnull
for SLP reduction chains, since we dissolve the group if SLP fails.
And SLP only analyses the first statement in the group, not the last:

  stmt = SLP_TREE_SCALAR_STMTS (node)[0];
  stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
  [...]
  bool res = vect_analyze_stmt (stmt, , node, node_instance);

So from that point of view the DEF_TYPE, REDUC_TYPE and REDUC_DEF
are being attached to the wrong statement, since we only analyse
the first one.  But it turns out that REDUC_TYPE and REDUC_DEF
don't matter for the first statement in the group, since that
takes the phi as an input, and when the phi is a direct input,
we use *its* REDUC_TYPE and REDUC_DEF instead of the statement's
own info.  The DEF_TYPE problem is handled by:

  /* Mark the first element of the reduction chain as reduction to properly
 transform the node.  In the reduction analysis phase only the last
 element of the chain is marked as reduction.  */
  if (!STMT_VINFO_GROUPED_ACCESS (vinfo_for_stmt (stmt)))
STMT_VINFO_DEF_TYPE (vinfo_for_stmt (stmt)) = vect_reduction_def;

in vect_analyze_slp_instance (cancelled by:

STMT_VINFO_DEF_TYPE (vinfo_for_stmt (first_element))
  = vect_internal_def;

in vect_analyze_slp on failure), with the operation being repeated
in vect_schedule_slp_instance (redundantly AFAICT):

  /* Mark the first element of the reduction chain as reduction to properly
 transform the node.  In the analysis phase only the last element of the
 chain is marked as reduction.  */
  if (GROUP_FIRST_ELEMENT (stmt_info) && !STMT_VINFO_GROUPED_ACCESS (stmt_info)
  && GROUP_FIRST_ELEMENT (stmt_info) == stmt)
{
  STMT_VINFO_DEF_TYPE (stmt_info) = vect_reduction_def;
  STMT_VINFO_TYPE (stmt_info) = reduc_vec_info_type;
}

Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64-linux-gnu.
OK to install?

Richard


2018-02-20  Richard Sandiford  

gcc/
PR tree-optimization/83965
* tree-vect-patterns.c (vect_reassociating_reduction_p): Assume
that grouped statements are part of a reduction chain.  Return
true if the statement is not marked as a reduction itself but
is part of a group.
(vect_recog_dot_prod_pattern): Don't check whether the statement
is part of a group here.
(vect_recog_sad_pattern): Likewise.
(vect_recog_widen_sum_pattern): Likewise.

gcc/testsuite/
PR tree-optimization/83965
* gcc.dg/vect/pr83965-2.c: New test.

Index: gcc/tree-vect-patterns.c
===
--- gcc/tree-vect-patterns.c2018-02-20 09:40:41.843451227 +
+++ gcc/tree-vect-patterns.c2018-02-20 16:28:55.636762056 +
@@ -222,13 +222,16 @@ vect_recog_temp_ssa_var (tree type, gimp
 }
 
 /* Return true if STMT_VINFO describes a reduction for which reassociation
-   is allowed.  */
+   is allowed.  If STMT_INFO is part of a group, assume that it's part of
+   a reduction chain and optimistically assume that all statements
+   except the last allow reassociation.  */
 
 static bool
 vect_reassociating_reduction_p (stmt_vec_info stmt_vinfo)
 {
   return (STMT_VINFO_DEF_TYPE (stmt_vinfo) == vect_reduction_def
- && STMT_VINFO_REDUC_TYPE (stmt_vinfo) != FOLD_LEFT_REDUCTION);
+ ? STMT_VINFO_REDUC_TYPE 

Re: [PATCH] Add "native" as a valid option value for -march= on i386 (PR driver/83193).

[Martin Liška]

On 02/20/2018 05:19 PM, Richard Sandiford wrote:
> Jakub Jelinek  writes:
>> On Tue, Feb 20, 2018 at 03:58:07PM +, Richard Sandiford wrote:
>>> Martin Liška  writes:
 Hi.

 Following patch adds "native" as a possible option for -march value on
 i386 target.  I have similar patches for other targets. Would it be
 possible to install the patch in current stage?
>>>
>>> [...]
>>>
 diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
 index d54e7301e84..361d4df2663 100644
 --- a/gcc/config/i386/i386.c
 +++ b/gcc/config/i386/i386.c
 @@ -4193,6 +4193,9 @@ ix86_option_override_internal (bool main_args_p,
|| ((processor_alias_table[i].flags & PTA_64BIT) != 0)))
  candidates.safe_push (processor_alias_table[i].name);
  
 +  /* Add also "native" as possible value.  */
 +  candidates.safe_push ("native");
 +
char *s;
const char *hint
= candidates_list_and_hint (opts->x_ix86_arch_string, s, candidates);
>>>
>>> We should probably only do this if the driver supports -march=native.
>>> I think for x86 that means HAVE_LOCAL_CPU_DETECT.
>>
>> Isn't the option available always?  It just doesn't do anything if
>> /* If we are compiling with GCC where %EBX register is fixed, then the
>>driver will just ignore -march and -mtune "native" target and will leave
>>to the newly built compiler to generate code for its default target.  */
> 
> It's only available for x86 hosts:
> 
> /* -march=native handling only makes sense with compiler running on
>an x86 or x86_64 chip.  If changing this condition, also change
>the condition in driver-i386.c.  */
> #if defined(__i386__) || defined(__x86_64__)
> /* In driver-i386.c.  */
> extern const char *host_detect_local_cpu (int argc, const char **argv);
> #define EXTRA_SPEC_FUNCTIONS \
>   { "local_cpu_detect", host_detect_local_cpu },
> #define HAVE_LOCAL_CPU_DETECT
> #endif
> 
> Non-native hosts are obviously a niche case for x86, but it still
> seems better to be consistent.
> 
> Richard
> 

So would it enough to wrap 'candidates.safe_push ("native");' by
#ifdef HAVE_LOCAL_CPU_DETECT
?

Thanks,
Martin

Re: [PATCH] Add "native" as a valid option value for -march= on i386 (PR driver/83193).

[Richard Sandiford]

Jakub Jelinek  writes:
> On Tue, Feb 20, 2018 at 03:58:07PM +, Richard Sandiford wrote:
>> Martin Liška  writes:
>> > Hi.
>> >
>> > Following patch adds "native" as a possible option for -march value on
>> > i386 target.  I have similar patches for other targets. Would it be
>> > possible to install the patch in current stage?
>> 
>> [...]
>> 
>> > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> > index d54e7301e84..361d4df2663 100644
>> > --- a/gcc/config/i386/i386.c
>> > +++ b/gcc/config/i386/i386.c
>> > @@ -4193,6 +4193,9 @@ ix86_option_override_internal (bool main_args_p,
>> >|| ((processor_alias_table[i].flags & PTA_64BIT) != 0)))
>> >  candidates.safe_push (processor_alias_table[i].name);
>> >  
>> > +  /* Add also "native" as possible value.  */
>> > +  candidates.safe_push ("native");
>> > +
>> >char *s;
>> >const char *hint
>> >= candidates_list_and_hint (opts->x_ix86_arch_string, s, candidates);
>> 
>> We should probably only do this if the driver supports -march=native.
>> I think for x86 that means HAVE_LOCAL_CPU_DETECT.
>
> Isn't the option available always?  It just doesn't do anything if
> /* If we are compiling with GCC where %EBX register is fixed, then the
>driver will just ignore -march and -mtune "native" target and will leave
>to the newly built compiler to generate code for its default target.  */

It's only available for x86 hosts:

/* -march=native handling only makes sense with compiler running on
   an x86 or x86_64 chip.  If changing this condition, also change
   the condition in driver-i386.c.  */
#if defined(__i386__) || defined(__x86_64__)
/* In driver-i386.c.  */
extern const char *host_detect_local_cpu (int argc, const char **argv);
#define EXTRA_SPEC_FUNCTIONS \
  { "local_cpu_detect", host_detect_local_cpu },
#define HAVE_LOCAL_CPU_DETECT
#endif

Non-native hosts are obviously a niche case for x86, but it still
seems better to be consistent.

Richard

Re: [PATCH] Add "native" as a valid option value for -march= on i386 (PR driver/83193).

[Richard Sandiford]

Martin Liška  writes:
> Hi.
>
> Following patch adds "native" as a possible option for -march value on
> i386 target.  I have similar patches for other targets. Would it be
> possible to install the patch in current stage?

[...]

> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index d54e7301e84..361d4df2663 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -4193,6 +4193,9 @@ ix86_option_override_internal (bool main_args_p,
>   || ((processor_alias_table[i].flags & PTA_64BIT) != 0)))
> candidates.safe_push (processor_alias_table[i].name);
>  
> +  /* Add also "native" as possible value.  */
> +  candidates.safe_push ("native");
> +
>char *s;
>const char *hint
>   = candidates_list_and_hint (opts->x_ix86_arch_string, s, candidates);

We should probably only do this if the driver supports -march=native.
I think for x86 that means HAVE_LOCAL_CPU_DETECT.

Thanks,
Richard

Re: [RFC] Tree loop unroller pass

[Michael Matz]

Hi,

On Fri, 16 Feb 2018, Wilco Dijkstra wrote:

> > How so?
> 
> I thought it is well-known for many years that the rtl unroller doesn't 
> work properly. In practically all cases where LLVM beats GCC, it is due 
> to unrolling small loops.

I thought it was because of vectorizing at -O2, not due to unrolling 
itself.

> > To generate more ILP for modern out-of-order processors you need to be
> > able to do followup transforms that remove dependences.  So rather than
> > inventing magic params we should look at those transforms and key
> > unrolling on them.  Like we do in predictive commoning or other passes
> > that end up performing unrolling as part of their transform.
> 
> This is why unrolling needs to be done at the tree level. Alias info is 
> correct, addressing modes end up more optimal and the scheduler can now 
> interleave the iterations (often not possible after the rtl-unroller due 
> to bad alias info).

The better alias info at gimple level is offsetted by worse information 
about addressing modes, register pressure and precise machine 
instructions.  It's not a very obvious tradeoff between both approaches.

> > Our measurements on x86 concluded that unrolling isn't worth it, in 
> > fact it very often hurts.  That was of course with saner params than 
> > the defaults of the RTL unroller.
> >
> > Often you even have to fight with followup passes doing stuff that 
> > ends up inreasing register pressure too much so we end up spilling.
> 
> Yes that's why I mentioned we should only unroll small loops where there 
> is always a benefit from reduced loop counter increments and branching.

With modern OOO processors the loop counter increments and branching costs 
about nothing on average due to speculation and bypassing.  In fact a loop 
cache might prefer small loops.

> > So _please_ first get testcases we know unrolling will be beneficial 
> > on and _also_ have a thorough description _why_.
> 
> I'm sure we can find good examples. The why will be obvious just from 
> instruction count.

The instruction count is only one part.  If the unrolling really enables 
followup transformations like cross-iteration redundancy removal, then 
it's worthwhile.  But for that we already have passes (e.g. predictive 
commoning).  If the only thing unrolling does is getting rid of N-1 loop 
counter test-and-branch, it's not beneficial on OOO processors (if the 
predictor is worth anything).

The reason why we tried some unrolling measurements last year is to 
establish if unrolling is worth it or not on an normal OOO x86-64 
processor (I basically wanted to have a case for implementing a generic 
unroller on gimple, and replace the RTL one (!)).  The results were so 
underwhelming that I dropped the plan.

> > With Ooo CPUs speculatively executing the next iterations I very much
> > doubt that.
> 
> OoO execution is like really dumb loop unrolling,

Actually I think of it as a rather clever way of unrolling.  The processor 
has exact knowledge of (non-)dependencies, even on those the compiler 
can't proof to not exist.

And if a dependency exists for real (such that an OOO CPU can't ignore it) 
how would you propose the compiler to magically remove it to get rid of 
the involved instructions?  That can usually only be done by duplicating 
threads-of-compute and that increases not decreases instruction count (or 
leaves them the same).  For instance the RTL unroller replaces IV adjusts 
ala:
  i = i + 1; use (i)
  i = i + 1; use (i)
with
  i1 = i + 1; use (i1)
  i2 = i + 2; use (i2)
So, compiler was able to get rid of one dependency chain, but in expense 
of having to use a new constant and register.  A cost analysis needs to 
happen to be sure that this is worthwhile.

> you still have all the dependencies between iterations, all the 
> branches, all the pointer increments etc. Optimizing those reduces 
> instruction counts like vectorization. Fewer instructions implies faster 
> code.

Also not true in this generality.  We recently had a case where 
vectorization hurt very much but it wasn't obvious why.  Our current 
theory (and we had many) is that because before the vector ALU actually 
could start working there had to be eight (strided) loads done per vector, 
for multiple streams, and all those loads eventually clogged the 
pipeline.  The scalar loop (on the OOO CPU) nicely intermingled ALU and 
memory ops.  In the end Richi got the vector loop to be only 2-3% slower 
than the scalar loop (and that was a fairly major effort), even though
from looking at the instructions it had only 70-80% of the original scalar 
dynamic instructions count.

In any case, I'm with Richi on this one about having some dependable 
testcases that speed up with a tree unroller, reasons of why this is so, 
and some cost model that reflects these reasons at least roughly.

All the above is of course only true for OOO CPUs.  For in-order unrolling 
is helpful.  But for getting that it would be good to work towards 

libgo patch committed: Check for preemption in fast syscall return

[Ian Lance Taylor]

This libgo patch checks for preemption in the fast return path from a
system call.  This helps with tight loops that make system calls, as
in BenchmarkSyscallExcessWork.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 257845)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-459a8a94e04a19bde7173ef7cf2db369c2e62e2d
+c6e0970f75508e209a10a7db5164d6ea3f9b28bf
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/runtime/proc.go
===
--- libgo/go/runtime/proc.go(revision 257743)
+++ libgo/go/runtime/proc.go(working copy)
@@ -2794,6 +2794,13 @@ func exitsyscall(dummy int32) {
exitsyscallclear(_g_)
_g_.m.locks--
_g_.throwsplit = false
+
+   // Check preemption, since unlike gc we don't check on
+   // every call.
+   if getg().preempt {
+   checkPreempt()
+   }
+
return
}
 

Re: [patch, fortran] Fix character length in constructors

[Steve Kargl]

On Tue, Feb 20, 2018 at 09:51:14AM +0200, Janne Blomqvist wrote:
> On Tue, Feb 20, 2018 at 12:41 AM, Thomas Koenig  wrote:
> > Hello world,
> >
> > when putting in a seemingly innocent simplification for PR 56342,
> > I caused a regression in PR 82823, in PACK. The root cause of
> > this one turned out to be PR 48890, in which structure
> > constructors containing characters were not handled correctly
> > if the lengths did not match.
> >
> > The attached patch fixes that.
> >
> > Regression-tested. OK for trunk?
> 
> It's better to use gfc_mpz_get_hwi() instead of mpz_get_si().
> mpz_get_si() returns a long, which is a 32-bit type on win64.
> 
> Otherwise Ok with the spelling fix suggested by Steve.
> 

Good catch.  I don't know how I forgot that you've
spent a lot time fixing int, long, size_t, ssize_t
issues.

-- 
Steve

Go patch committed: Look through aliases for type compatibility

[Ian Lance Taylor]

In Go, aliases are supposed to be identical to the type being aliased,
so questions about type compatibility need to always ignore aliases,
except for error messages involving the type name.  This patch
implements that.  This test case for this is
https://golang.org/cl/94995.  This fixes
https://golang.org/issue/23912.  Bootstrapped and ran Go testsuite  on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 257743)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-cef3934fbc63f5e121abb8f88d3799961ac95b59
+459a8a94e04a19bde7173ef7cf2db369c2e62e2d
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/types.cc
===
--- gcc/go/gofrontend/types.cc  (revision 257540)
+++ gcc/go/gofrontend/types.cc  (working copy)
@@ -597,10 +597,10 @@ Type::are_compatible_for_comparison(bool
  return false;
}
 
-  if (t1->named_type() != NULL)
-   return t1->named_type()->named_type_is_comparable(reason);
-  else if (t2->named_type() != NULL)
-   return t2->named_type()->named_type_is_comparable(reason);
+  if (t1->unalias()->named_type() != NULL)
+   return t1->unalias()->named_type()->named_type_is_comparable(reason);
+  else if (t2->unalias()->named_type() != NULL)
+   return t2->unalias()->named_type()->named_type_is_comparable(reason);
   else if (t1->struct_type() != NULL)
{
  if (t1->struct_type()->is_struct_incomparable())
@@ -678,6 +678,12 @@ Type::are_assignable(const Type* lhs, co
   if (Type::are_identical(lhs, rhs, true, reason))
 return true;
 
+  // Ignore aliases, except for error messages.
+  const Type* lhs_orig = lhs;
+  const Type* rhs_orig = rhs;
+  lhs = lhs->unalias();
+  rhs = rhs->unalias();
+
   // The types are assignable if they have identical underlying types
   // and either LHS or RHS is not a named type.
   if (((lhs->named_type() != NULL && rhs->named_type() == NULL)
@@ -740,15 +746,16 @@ Type::are_assignable(const Type* lhs, co
 {
   if (rhs->interface_type() != NULL)
reason->assign(_("need explicit conversion"));
-  else if (lhs->named_type() != NULL && rhs->named_type() != NULL)
+  else if (lhs_orig->named_type() != NULL
+  && rhs_orig->named_type() != NULL)
{
- size_t len = (lhs->named_type()->name().length()
-   + rhs->named_type()->name().length()
+ size_t len = (lhs_orig->named_type()->name().length()
+   + rhs_orig->named_type()->name().length()
+ 100);
  char* buf = new char[len];
  snprintf(buf, len, _("cannot use type %s as type %s"),
-  rhs->named_type()->message_name().c_str(),
-  lhs->named_type()->message_name().c_str());
+  rhs_orig->named_type()->message_name().c_str(),
+  lhs_orig->named_type()->message_name().c_str());
  reason->assign(buf);
  delete[] buf;
}
@@ -768,6 +775,10 @@ Type::are_convertible(const Type* lhs, c
   if (Type::are_assignable(lhs, rhs, reason))
 return true;
 
+  // Ignore aliases.
+  lhs = lhs->unalias();
+  rhs = rhs->unalias();
+
   // A pointer to a regular type may not be converted to a pointer to
   // a type that may not live in the heap, except when converting from
   // unsafe.Pointer.

[PATCH] [ARC] Cleanup unused functions.

[Claudiu Zissulescu]

From: Claudiu Zissulescu 

Cleanup unsed functions and macros.

OK to apply?
Claudiu

gcc/
2018-01-26  Claudiu Zissulescu  

* config/arc/arc.c (arc_finalize_pic): Remove function.
(arc_must_save_register): We use single base PIC register, remove
checks to save/restore the PIC register.
(arc_expand_prologue): Likewise.
* config/arc/arc-protos.h (arc_set_default_type_attributes):
Remove.
(arc_verify_short): Likewise.
(arc_attr_type): Likewise.
* config/arc/arc.c (arc_set_default_type_attributes): Remove.
(walk_stores): Likewise.
(arc_address_cost): Make it static.
(arc_verify_short): Likewise.
(branch_dest): Likewise.
(arc_attr_type): Likewise.
* config/arc/arc.c (TARGET_ADJUST_INSN_LENGTH): Remove.
(TARGET_INSN_LENGTH_PARAMETERS): Likewise.
(arc_final_prescan_insn): Remove inserting the nops due to
hardware hazards.  It is done in reorg step.
(insn_length_variant_t): Remove.
(insn_length_parameters_t): Likewise.
(arc_insn_length_parameters): Likewise.
(arc_get_insn_variants): Likewise.
* config/arc/arc.h (TARGET_UPSIZE_DBR): Remove.
---
 gcc/config/arc/arc-protos.h |   3 -
 gcc/config/arc/arc.c| 471 +---
 gcc/config/arc/arc.h|   3 -
 3 files changed, 4 insertions(+), 473 deletions(-)

diff --git a/gcc/config/arc/arc-protos.h b/gcc/config/arc/arc-protos.h
index b469cfc4c2c..75cfedab7f1 100644
--- a/gcc/config/arc/arc-protos.h
+++ b/gcc/config/arc/arc-protos.h
@@ -32,7 +32,6 @@ extern bool arc_double_limm_p (rtx);
 extern void arc_print_operand (FILE *, rtx, int);
 extern void arc_print_operand_address (FILE *, rtx);
 extern void arc_final_prescan_insn (rtx_insn *, rtx *, int);
-extern void arc_set_default_type_attributes(tree type);
 extern const char *arc_output_libcall (const char *);
 extern bool prepare_extend_operands (rtx *operands, enum rtx_code code,
 machine_mode omode);
@@ -97,10 +96,8 @@ extern void split_addsi (rtx *);
 extern void split_subsi (rtx *);
 extern void arc_pad_return (void);
 extern void arc_split_move (rtx *);
-extern int arc_verify_short (rtx_insn *insn, int unalign, int);
 extern const char *arc_short_long (rtx_insn *insn, const char *, const char *);
 extern rtx arc_regno_use_in (unsigned int, rtx);
-extern int arc_attr_type (rtx_insn *);
 extern bool arc_scheduling_not_expected (void);
 extern bool arc_sets_cc_p (rtx_insn *insn);
 extern int arc_label_align (rtx_insn *label);
diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index f572d6f4883..74007d8aba3 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -473,8 +473,6 @@ static void arc_function_arg_advance (cumulative_args_t, 
machine_mode,
  const_tree, bool);
 static rtx arc_legitimize_address_0 (rtx, rtx, machine_mode mode);
 
-static void arc_finalize_pic (void);
-
 /* initialize the GCC target structure.  */
 #undef  TARGET_COMP_TYPE_ATTRIBUTES
 #define TARGET_COMP_TYPE_ATTRIBUTES arc_comp_type_attributes
@@ -612,10 +610,6 @@ static void arc_finalize_pic (void);
 
 #define TARGET_LEGITIMIZE_ADDRESS arc_legitimize_address
 
-#define TARGET_ADJUST_INSN_LENGTH arc_adjust_insn_length
-
-#define TARGET_INSN_LENGTH_PARAMETERS arc_insn_length_parameters
-
 #undef TARGET_NO_SPECULATION_IN_DELAY_SLOTS_P
 #define TARGET_NO_SPECULATION_IN_DELAY_SLOTS_P \
   arc_no_speculation_in_delay_slots_p
@@ -2131,14 +2125,6 @@ arc_comp_type_attributes (const_tree type1,
   return 1;
 }
 
-/* Set the default attributes for TYPE.  */
-
-void
-arc_set_default_type_attributes (tree type ATTRIBUTE_UNUSED)
-{
-  gcc_unreachable();
-}
-
 /* Misc. utilities.  */
 
 /* X and Y are two things to compare using CODE.  Emit the compare insn and
@@ -2368,7 +2354,7 @@ arc_setup_incoming_varargs (cumulative_args_t args_so_far,
 /* Provide the costs of an addressing mode that contains ADDR.
If ADDR is not a valid address, its cost is irrelevant.  */
 
-int
+static int
 arc_address_cost (rtx addr, machine_mode, addr_space_t, bool speed)
 {
   switch (GET_CODE (addr))
@@ -2703,10 +2689,6 @@ arc_must_save_register (int regno, struct function *func)
   && !firq_auto_save_p)
 return true;
 
-  if (flag_pic && crtl->uses_pic_offset_table
-  && regno == PIC_OFFSET_TABLE_REGNUM)
-return true;
-
   return false;
 }
 
@@ -3296,10 +3278,6 @@ arc_expand_prologue (void)
emit_insn (gen_stack_tie (stack_pointer_rtx,
  hard_frame_pointer_rtx));
 }
-
-  /* Setup the gp register, if needed.  */
-  if (crtl->uses_pic_offset_table)
-arc_finalize_pic ();
 }
 
 /* Do any necessary cleanup after a function to restore stack, frame,
@@ -3545,37 +3523,6 @@ arc_unspec_offset (rtx loc, int unspec)
   

Re: Fixed incorrect TARGET_MEM_REF alignment (PR 84419)

[Richard Biener]

On February 20, 2018 10:43:40 AM GMT+01:00, Richard Sandiford 
 wrote:
>expand_call_mem_ref checks for TARGET_MEM_REFs that have compatible
>type, but it didn't then go on to install the specific type we need,
>which might have different alignment due to:
>
>  if (TYPE_ALIGN (type) != align)
>type = build_aligned_type (type, align);
>
>This was causing masked stores to be incorrectly marked as
>aligned on AVX512.
>
>Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64-linux-gnu.
>Also tested by Alexander on AVX512 hw.  OK to install?

OK. 

Richard. 

>Richard
>
>
>2018-02-20  Richard Sandiford  
>
>gcc/
>   PR tree-optimization/84419
>   * internal-fn.c (expand_call_mem_ref): Create a TARGET_MEM_REF
>   with the required type if its current type is compatible but
>   different.
>
>gcc/testsuite/
>   PR tree-optimization/84419
>   * gcc.dg/vect/pr84419.c: New test.
>
>Index: gcc/internal-fn.c
>===
>--- gcc/internal-fn.c  2018-01-13 18:01:51.235735290 +
>+++ gcc/internal-fn.c  2018-02-20 09:40:47.547217244 +
>@@ -2444,11 +2444,14 @@ expand_call_mem_ref (tree type, gcall *s
> && types_compatible_p (TREE_TYPE (mem), type))
>   {
> tree offset = TMR_OFFSET (mem);
>-if (alias_ptr_type != TREE_TYPE (offset) || !integer_zerop
>(offset))
>+if (type != TREE_TYPE (mem)
>+|| alias_ptr_type != TREE_TYPE (offset)
>+|| !integer_zerop (offset))
>   {
> mem = copy_node (mem);
> TMR_OFFSET (mem) = wide_int_to_tree (alias_ptr_type,
>  wi::to_poly_wide (offset));
>+TREE_TYPE (mem) = type;
>   }
> return mem;
>   }
>Index: gcc/testsuite/gcc.dg/vect/pr84419.c
>===
>--- /dev/null  2018-02-19 19:34:42.906488063 +
>+++ gcc/testsuite/gcc.dg/vect/pr84419.c2018-02-20 09:40:47.548217201
>+
>@@ -0,0 +1,21 @@
>+#include 
>+
>+#define SIZE 400
>+
>+int  foo[SIZE];
>+char bar[SIZE];
>+
>+void __attribute__ ((noinline)) foo_func(void)
>+{
>+  int i;
>+  for (i = 1; i < SIZE; i++)
>+if (bar[i])
>+  foo[i] = 1;
>+}
>+
>+int main()
>+{
>+  memset(bar, 1, sizeof(bar));
>+  foo_func();
>+  return 0;
>+}

[PATCH] Add "native" as a valid option value for -march= on i386 (PR driver/83193).

[Martin Liška]

Hi.

Following patch adds "native" as a possible option for -march value on i386 
target.
I have similar patches for other targets. Would it be possible to install the
patch in current stage?

Before:
$ ./xgcc -B. -march=abcdef /tmp/empty.c
cc1: error: bad value (‘abcdef’) for ‘-march=’ switch
cc1: note: valid arguments to ‘-march=’ switch are: nocona core2 nehalem corei7 
westmere sandybridge corei7-avx ivybridge core-avx-i haswell core-avx2 
broadwell skylake skylake-avx512 cannonlake icelake bonnell atom silvermont slm 
knl knm x86-64 eden-x2 nano nano-1000 nano-2000 nano-3000 nano-x2 eden-x4 
nano-x4 k8 k8-sse3 opteron opteron-sse3 athlon64 athlon64-sse3 athlon-fx 
amdfam10 barcelona bdver1 bdver2 bdver3 bdver4 znver1 btver1 btver2

After:
$ ./xgcc -B. -march=abcdef /tmp/empty.c
cc1: error: bad value (‘abcdef’) for ‘-march=’ switch
cc1: note: valid arguments to ‘-march=’ switch are: nocona core2 nehalem corei7 
westmere sandybridge corei7-avx ivybridge core-avx-i haswell core-avx2 
broadwell skylake skylake-avx512 cannonlake icelake bonnell atom silvermont slm 
knl knm x86-64 eden-x2 nano nano-1000 nano-2000 nano-3000 nano-x2 eden-x4 
nano-x4 k8 k8-sse3 opteron opteron-sse3 athlon64 athlon64-sse3 athlon-fx 
amdfam10 barcelona bdver1 bdver2 bdver3 bdver4 znver1 btver1 btver2 native

$ ./xgcc -B. -march=native2 /tmp/empty.c
cc1: error: bad value (‘native2’) for ‘-march=’ switch
cc1: note: valid arguments to ‘-march=’ switch are: nocona core2 nehalem corei7 
westmere sandybridge corei7-avx ivybridge core-avx-i haswell core-avx2 
broadwell skylake skylake-avx512 cannonlake icelake bonnell atom silvermont slm 
knl knm x86-64 eden-x2 nano nano-1000 nano-2000 nano-3000 nano-x2 eden-x4 
nano-x4 k8 k8-sse3 opteron opteron-sse3 athlon64 athlon64-sse3 athlon-fx 
amdfam10 barcelona bdver1 bdver2 bdver3 bdver4 znver1 btver1 btver2 native; did 
you mean ‘native’?

Ready for trunk after tests?
Thanks
Martin

gcc/ChangeLog:

2018-02-20  Martin Liska  

PR driver/83193
* config/i386/i386.c (ix86_option_override_internal):
Add "native" as a possible value.
---
 gcc/config/i386/i386.c | 3 +++
 1 file changed, 3 insertions(+)


diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index d54e7301e84..361d4df2663 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -4193,6 +4193,9 @@ ix86_option_override_internal (bool main_args_p,
 		|| ((processor_alias_table[i].flags & PTA_64BIT) != 0)))
 	  candidates.safe_push (processor_alias_table[i].name);
 
+  /* Add also "native" as possible value.  */
+  candidates.safe_push ("native");
+
   char *s;
   const char *hint
 	= candidates_list_and_hint (opts->x_ix86_arch_string, s, candidates);

Re: [PATCH] Defer pow (C, x) folding until after vectorization always (PR middle-end/82004)

[Richard Biener]

On February 19, 2018 11:02:50 PM GMT+01:00, Jakub Jelinek  
wrote:
>Hi!
>
>While I've over-simplified the testcase and so this patch doesn't help
>the 628.pop2_s miscompare, I still believe it is beneficial to defer
>this
>folding until late for these reasons:
>1) if we propagate a constant into the second pow argument too, it will
>   be likely more precise than going through the exp (cst * x) way
>2) except when C is M_E, pow is fewer operations and thus smaller IL
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK. 

Richard. 

>2018-02-19  Jakub Jelinek  
>
>   PR middle-end/82004
>   * match.pd (pow(C,x) -> exp(log(C)*x)): Delay all folding until
>   after vectorization.
>
>   * gfortran.dg/pr82004.f90: New test.
>
>--- gcc/match.pd.jj2018-02-15 12:15:51.655780636 +0100
>+++ gcc/match.pd   2018-02-19 17:38:06.390763194 +0100
>@@ -4006,7 +4006,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>   (simplify
>(pows REAL_CST@0 @1)
>(if (real_compare (GT_EXPR, TREE_REAL_CST_PTR (@0), )
>-  && real_isfinite (TREE_REAL_CST_PTR (@0)))
>+  && real_isfinite (TREE_REAL_CST_PTR (@0))
>+  /* As libmvec doesn't have a vectorized exp2, defer optimizing
>+ the use_exp2 case until after vectorization.  It seems actually
>+ beneficial for all constants to postpone this until later,
>+ because exp(log(C)*x), while faster, will have worse precision
>+ and if x folds into a constant too, that is unnecessary
>+ pessimization.  */
>+  && canonicalize_math_after_vectorization_p ())
> (with {
>const REAL_VALUE_TYPE *const value = TREE_REAL_CST_PTR (@0);
>bool use_exp2 = false;
>@@ -4021,10 +4028,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  }
>  (if (!use_exp2)
>   (exps (mult (logs @0) @1))
>-  /* As libmvec doesn't have a vectorized exp2, defer optimizing
>-   this until after vectorization.  */
>-  (if (canonicalize_math_after_vectorization_p ())
>-  (exp2s (mult (log2s @0) @1
>+  (exp2s (mult (log2s @0) @1)))
> 
>  (for sqrts (SQRT)
>   cbrts (CBRT)
>--- gcc/testsuite/gfortran.dg/pr82004.f90.jj   2018-02-19
>17:58:57.435682156 +0100
>+++ gcc/testsuite/gfortran.dg/pr82004.f90  2018-02-19 17:58:34.127684892
>+0100
>@@ -0,0 +1,18 @@
>+! PR middle-end/82004
>+! { dg-do run }
>+! { dg-options "-Ofast" }
>+
>+  integer, parameter :: r8 = selected_real_kind(13), i4 = kind(1)
>+  integer (i4), parameter :: a = 400, b = 2
>+  real (r8), parameter, dimension(b) :: c = (/ .001_r8, 10.00_r8 /)
>+  real (r8) :: d, e, f, g, h
>+  real (r8), parameter :: j &
>+= 10**(log10(c(1))-(log10(c(b))-log10(c(1)))/real(a))
>+
>+  d = c(1)
>+  e = c(b)
>+  f = (log10(e)-log10(d))/real(a)
>+  g = log10(d) - f
>+  h = 10**(g)
>+  if (h.ne.j) stop 1
>+end
>
>   Jakub

Re: RFA: Sanitize deprecation messages (PR 84195)

[Nick Clifton]

Hi Martin,


> Since the class manages a resource it should ideally make sure it
> doesn't try to release the same resource multiple times.  I.e., its
> copy constructor and assignment operators should either "do the right
> thing" (whatever you think that is) or be made inaccessible (or declared 
> deleted in C++ 11).
> 
> For example:
> 
>   {
>     escaped_string a;
>     a.escape ("foo\nbar");
> 
>     escaped_string b (a);
>     // b destroys its m_str
>     // double free in a's destructor here
>   }

I am not sure that this can happen.  First of the escaped_string
class does not have constructor that accepts a char* argument.
(Maybe in C++ this is done automatically ?  My C++-fu is very weak).

Secondly the m_owned private field is only set to true when
the m_str field is set to a string allocated by the particular
instance of the class, and memory is only freed by the destructor 
if m_owned is true.  

So even this should work:

  {
escaped_string a,b;

a.escape ("foo\nbar");
b.escape ((const char *) a);
  }

The destructor for B will not free any memory, even though its
m_str field has been set to the same string as A, because its
m_owned field will be set to FALSE.

Cheers
  Nick

Re: [PATCH] Fix missing info for -march and -mtune wrong values on aarch64 (PR driver/83193).

[Jakub Jelinek]

On Tue, Feb 20, 2018 at 01:05:33PM +0100, Martin Liška wrote:
> Hi.
> 
> This is fix for one of multiple issues seen in the PR. Note that I also have 
> some patches
> for the others, but it's not stage4 material in my opinion.
> This one is quite obvious and should land in GCC 8.
> 
> Output before:
> $ ./xgcc -B. -march=sparta /tmp/main.c
> cc1: error: unknown value ‘sparta’ for -march
> 
> And after:
> $ ./xgcc -B. -march=sparta /tmp/main.c
> cc1: error: unknown value ‘sparta’ for -march
> cc1: note: valid arguments are: armv8-a armv8.1-a armv8.2-a armv8.3-a 
> armv8.4-a;
> 
> Ready for trunk?
> 
> gcc/ChangeLog:
> 
> 2018-02-20  Martin Liska  
> 
>   PR driver/83193
>   * config/aarch64/aarch64.c (aarch64_print_hint_for_core_or_arch): Print
>   possible values if we don't have a hint.
> ---
>  gcc/config/aarch64/aarch64.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> 

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 228fd1b908d..5fb18c4ca20 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -10633,6 +10633,9 @@ aarch64_print_hint_for_core_or_arch (const char *str, 
> bool arch)
>if (hint)
>  inform (input_location, "valid arguments are: %s;"
>" did you mean %qs?", s, hint);
> +  else
> +inform (input_location, "valid arguments are: %s;", s);

No trailing ; in this case, that is a separator for did you mean.

Otherwise I'd say this is obvious, ok for trunk.

Jakub

Re: [PATCH] Add limit for maximal alignment options (PR c/84310).

[Jakub Jelinek]

On Tue, Feb 20, 2018 at 11:05:50AM +0100, Martin Liška wrote:
> Thanks Jakub!
> Would it be possible to backport that to active branches?

Yes, but give it some time on the trunk first.

Jakub

Re: [PATCH] Fix missing info for -march and -mtune wrong values on aarch64 (PR driver/83193).

[Martin Liška]

On 02/20/2018 01:09 PM, Jakub Jelinek wrote:
> On Tue, Feb 20, 2018 at 01:05:33PM +0100, Martin Liška wrote:
>> Hi.
>>
>> This is fix for one of multiple issues seen in the PR. Note that I also have 
>> some patches
>> for the others, but it's not stage4 material in my opinion.
>> This one is quite obvious and should land in GCC 8.
>>
>> Output before:
>> $ ./xgcc -B. -march=sparta /tmp/main.c
>> cc1: error: unknown value ‘sparta’ for -march
>>
>> And after:
>> $ ./xgcc -B. -march=sparta /tmp/main.c
>> cc1: error: unknown value ‘sparta’ for -march
>> cc1: note: valid arguments are: armv8-a armv8.1-a armv8.2-a armv8.3-a 
>> armv8.4-a;
>>
>> Ready for trunk?
>>
>> gcc/ChangeLog:
>>
>> 2018-02-20  Martin Liska  
>>
>>  PR driver/83193
>>  * config/aarch64/aarch64.c (aarch64_print_hint_for_core_or_arch): Print
>>  possible values if we don't have a hint.
>> ---
>>  gcc/config/aarch64/aarch64.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>>
> 
>> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
>> index 228fd1b908d..5fb18c4ca20 100644
>> --- a/gcc/config/aarch64/aarch64.c
>> +++ b/gcc/config/aarch64/aarch64.c
>> @@ -10633,6 +10633,9 @@ aarch64_print_hint_for_core_or_arch (const char 
>> *str, bool arch)
>>if (hint)
>>  inform (input_location, "valid arguments are: %s;"
>>   " did you mean %qs?", s, hint);
>> +  else
>> +inform (input_location, "valid arguments are: %s;", s);
> 
> No trailing ; in this case, that is a separator for did you mean.

Yep!

> 
> Otherwise I'd say this is obvious, ok for trunk.

Thanks, just installed.

Martin

> 
>   Jakub
> 

[PATCH] RL78 one_cmplhi2 improvement

[Sebastian Perta]

Hello,

The following patch defines one_cmplhi2 pattern.  The improvement does not
come from the two xor instructions used 
(in fact for the second xor the pattern uses xor saddr , #byte is used which
is 1 bytes longer than xor a, #byte) it comes 
from the fact that that the number of move instructions is reduced (16 bit
movw is used instead of 8 bit mov).

This can be seen on a very simple test case:

uint16_t test_one_cmplhi(uint16_t x) 
{
 return ~x; 
}

_test_one_cmplhi:
mov a, [sp+4]
xor a, #-1
mov r8, a
mov a, [sp+5]
xor a, #-1
mov r9, a
ret

_test_one_cmplhi:
movwax, [sp+4]
xor a, #-1 
xor 0xFFEF8, #-1 ;one_cmplhi2 ax, ax
movwr8, ax
ret

In "gcc.c-torture/execute/" I have seen the patch being effective in 18
cases with the biggest improvement is occurring in
gcc.c-torture/execute/pr68376-2 where this patch reduces the code size from
792 bytes to 704.

Unfortunately there's also a downside to this patch. In cases when the
second xor is redundant GCC never gets the chance to remove the second xor. 
This can be seen in gcc.c-torture/execute/pr42833.c for example line 57: 
vdest.v1 = (vsrc1.v1 + (1 << (-1 - tmp))) >> -tmp;
This is because all variables are 8 bit while the operation is being
promoted to 16 bit (because of the constants being of type int by default)
this is why the one_cmplhi2 is used in this case.

The relevant insns for this case are:

(insn 35 257 260 14 (set (reg:HI 0 x)
(xor:HI (reg:HI 0 x)
(const_int -1 [0x])))
"./xgcc_rw_trunk2/gcc/testsuite/gcc.c-torture/execute/pr42833.c":57 59
{*one_cmplhi2_real}
 (nil))
(insn 260 35 261 14 (set (reg:QI 1 a)
(reg:QI 0 x [8]))
"./xgcc_rw_trunk2/gcc/testsuite/gcc.c-torture/execute/pr42833.c":57 44
{*movqi_real}
 (expr_list:REG_DEAD (reg:QI 8 r8)
(nil)))

As it can be seen it is quite obvious that reg "a" is dead after insn 35.
I think I can write a simple rtl_opt_pass (similar to pass_rl78_move_elim)
to look for such cases and replace the one_cmplhi2 with one_cmplqi2.

Even without this new pass (which I plan to do in the near future) I still
think (based on testing) the advantages of this patch outweigh the
disadvantage described above.
Is OK to check-in? Or do I need to add the new pass as well for this to be
considered? Thank you!

Regression test is OK, tested with the following command:
make -k check-gcc RUNTESTFLAGS=--target_board=rl78-sim


Best Regards,
Sebastian

2018-02-20  Sebastian Perta  

* config/rl78/rl78-expand.md (one_cmplhi2): New define expand.
* config/rl78/rl78-virt.md (one_cmplhi2_virt): New define insn.
* config/rl78/rl78-real.md (one_cmplhi2_real): New define insn.


Index: rl78-expand.md
===
--- rl78-expand.md  (revision 257806)
+++ rl78-expand.md  (working copy)
@@ -211,6 +211,16 @@
  DONE;"
 )
 
+(define_expand "one_cmplhi2"
+ [(set (match_operand:HI 0 "nonimmediate_operand")
+  (xor:HI (match_operand:HI 1 "general_operand")
+   (const_int -1)))
+  ]
+  ""
+  "if (rl78_force_nonfar_2 (operands, gen_one_cmplhi2))
+ DONE;"
+)
+
 ;;-- Shifts 
 
 (define_expand "ashl3"
Index: rl78-real.md
===
--- rl78-real.md(revision 257806)
+++ rl78-real.md(working copy)
@@ -240,6 +240,16 @@
   [(set (attr "update_Z") (const_string "update_Z"))]
 )
 
+(define_insn "*one_cmplhi2_real"
+ [(set (match_operand:HI 0 "register_operand" "=A")
+   (xor:HI (match_operand:HI 1 "general_operand" "0")
+   (const_int -1)))
+ ]
+ "rl78_real_insns_ok ()"
+ "xor a, #-1 \;xor 0xFFEF8, #-1 ;one_cmplhi2 %0, %1"
+ [(set_attr "update_Z" "clobber")]
+)
+
 ;;-- Shifts 
 
 (define_insn "*ashlqi3_real"
Index: rl78-virt.md
===
--- rl78-virt.md(revision 257806)
+++ rl78-virt.md(working copy)
@@ -165,6 +165,16 @@
   "v.xor\t%0, %1, %2"
 )
 
+(define_insn "*one_cmplhi2_virt"
+ [(set (match_operand:HI 0 "rl78_nonfar_nonimm_operand" "=v")
+  (xor:HI (match_operand:HI 1 "general_operand"  "ivU")
+   (const_int -1)))
+  ]
+  "rl78_virt_insns_ok ()"
+  "v.one_cmplhi2\t%0, %1"
+ [(set_attr "valloc" "op1")]
+)
+
 ;;-- Shifts 
 
 (define_insn "*ashl3_virt"

[PATCH] Fix missing info for -march and -mtune wrong values on aarch64 (PR driver/83193).

[Martin Liška]

Hi.

This is fix for one of multiple issues seen in the PR. Note that I also have 
some patches
for the others, but it's not stage4 material in my opinion.
This one is quite obvious and should land in GCC 8.

Output before:
$ ./xgcc -B. -march=sparta /tmp/main.c
cc1: error: unknown value ‘sparta’ for -march

And after:
$ ./xgcc -B. -march=sparta /tmp/main.c
cc1: error: unknown value ‘sparta’ for -march
cc1: note: valid arguments are: armv8-a armv8.1-a armv8.2-a armv8.3-a armv8.4-a;

Ready for trunk?

gcc/ChangeLog:

2018-02-20  Martin Liska  

PR driver/83193
* config/aarch64/aarch64.c (aarch64_print_hint_for_core_or_arch): Print
possible values if we don't have a hint.
---
 gcc/config/aarch64/aarch64.c | 3 +++
 1 file changed, 3 insertions(+)


diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 228fd1b908d..5fb18c4ca20 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10633,6 +10633,9 @@ aarch64_print_hint_for_core_or_arch (const char *str, bool arch)
   if (hint)
 inform (input_location, "valid arguments are: %s;"
 			 " did you mean %qs?", s, hint);
+  else
+inform (input_location, "valid arguments are: %s;", s);
+
   XDELETEVEC (s);
 }
 

Re: [PATCH] c++: Fix spurious fallthrough warning on break

[Siddhesh Poyarekar]

On Tuesday 20 February 2018 03:14 PM, Jakub Jelinek wrote:
> On Mon, Feb 19, 2018 at 09:59:00PM +0530, Siddhesh Poyarekar wrote:
>> The C++ frontend generates a break that results in the fallthrough
>> warning misfiring in nested switch blocks where cases in the inner
>> switch block return, rendering the break pointless.  The fallthrough
>> detection in finish_break_stmt does not work either because the
>> condition is encoded as an IF_STMT and not a COND_EXPR.
>>
>> Fix this by adding a condition for IF_STMT in the
>> langhooks.block_may_fallthru for C++.  Fix tested on x86_64.  Full
>> testsuite run is in progress.
>>
>> Siddhesh
>>
>> gcc/cp
>>  * cp-objcp-common.c (cxx_block_may_fallthru): Add case for
>>  IF_STMT.
>>
>> gcc/testsuite
>>  * g++.dg/nested-switch.C: New test case.
> 
> C++ testcase shouldn't be put directly into g++.dg/ (though yes, several
> have been), but into a subdirectory.  In this case, because it is a (bogus)
> warning, it should best go into g++.dg/warn/, and perhaps best named as
> g++.dg/warn/Wimplicit-fallthrough-3.C .
> 
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/nested-switch.C
>> @@ -0,0 +1,30 @@
>> +// Verify that there are no spurious warnings in nested switch statements 
>> due
>> +// to the unnecessary break in the inner switch block.
>> +// { dg-do compile }
>> +// { dg-options "-Werror=implicit-fallthrough" } */
> 
> I'd go just for -Wimplicit-fallthrough.
> 
>> +
>> +int
>> +foo (int c1, int c2, int c3)
>> +{
>> +  switch (c2)
>> +{
>> +case 0:   
>> +  switch (c3) {
> 
> And turn the above line into
>   switch (c3) {   // { dg-bogus "may fall through" }
> 
>> +case 0:
>> +  if (c1)
>> +return 1;
>> +  else
>> +return 2;
>> +  break;
>> +
>> +default:
>> +  return 3;
>> +  }
>> +
>> +case 1: 
>> +  return 4;
>> +default:
>> +  return 5;
>> +  break;
>> +}
>> +}
> 
> Ok for trunk with those nits, thanks.

Thanks, fixed up and pushed.

Siddhesh

Re: Mising Patch #2 from the RISC-V v3 Submission

[Andreas Schwab]

On Feb 15 2018, Palmer Dabbelt  wrote:

> On Mon, 12 Feb 2018 15:18:39 PST (-0800), Jim Wilson wrote:
>> On 02/12/2018 03:23 AM, Andreas Schwab wrote:
>>> On Feb 06 2017, Palmer Dabbelt  wrote:
>>>
 +/* Because RISC-V only has word-sized atomics, it requries libatomic where
 +   others do not.  So link libatomic by default, as needed.  */
 +#undef LIB_SPEC
 +#ifdef LD_AS_NEEDED_OPTION
 +#define LIB_SPEC GNU_USER_TARGET_LIB_SPEC \
 +  " %{pthread:" LD_AS_NEEDED_OPTION " -latomic " LD_NO_AS_NEEDED_OPTION 
 "}"
 +#else
 +#define LIB_SPEC GNU_USER_TARGET_LIB_SPEC " -latomic "
 +#endif
>>>
>>> Why is -latomic added only with -pthread if --as-needed is supported,
>>> but unconditionally if not?  Wouldn't it make sense to add it
>>> unconditionally in both cases?
>>
>> I don't know the history here, but I do know that the most common atomic
>> related bug report we get is for people using pthread, so we were
>> probably thinking about that when this was written.
>
> IIRC that's why it's done this way.

There needs to be some standard way to find -latomic during build, a
couple of libgo configure tests fail because they do not find it.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Add limit for maximal alignment options (PR c/84310).

[Martin Liška]

On 02/20/2018 09:43 AM, Jakub Jelinek wrote:
> On Mon, Feb 12, 2018 at 01:09:43PM +0100, Martin Liška wrote:
>> Following patch fixes 2 issues with -falign-*:
>> 1) when using -malign-x=16 (or corresponding -falign-* value) then ICE 
>> appeared
>> as code in final.c can deal just with limited alignment.
>> 2) thus I also documented and limited the maximum value of -falign-* options.
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>> i386.exp test-suite works fine on x86_64 machine.
>>
>> Ready to be installed?
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2018-02-09  Martin Liska  
>>
>>  PR c/84310
>>  PR target/79747
>>  * final.c (shorten_branches): Build align_tab array with one
>>  more element.
>>  * opts.c (finish_options): Add alignment option limit check.
>>  (MAX_CODE_ALIGN): Likewise.
>>  (MAX_CODE_ALIGN_VALUE): Likewise.
>>  * doc/invoke.texi: Document maximum allowed option value for
>>  all -falign-* options.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2018-02-12  Martin Liska  
>>
>>  PR c/84310
>>  PR target/79747
>>  * gcc.target/i386/pr84310.c: New test.
>>  * gcc.target/i386/pr84310-2.c: Likewise.
> 
> Ok, thanks.
> 
>   Jakub
> 

Thanks Jakub!
Would it be possible to backport that to active branches?

Martin

Re: [PATCH] c++: Fix spurious fallthrough warning on break

[Jakub Jelinek]

On Mon, Feb 19, 2018 at 09:59:00PM +0530, Siddhesh Poyarekar wrote:
> The C++ frontend generates a break that results in the fallthrough
> warning misfiring in nested switch blocks where cases in the inner
> switch block return, rendering the break pointless.  The fallthrough
> detection in finish_break_stmt does not work either because the
> condition is encoded as an IF_STMT and not a COND_EXPR.
> 
> Fix this by adding a condition for IF_STMT in the
> langhooks.block_may_fallthru for C++.  Fix tested on x86_64.  Full
> testsuite run is in progress.
> 
> Siddhesh
> 
> gcc/cp
>   * cp-objcp-common.c (cxx_block_may_fallthru): Add case for
>   IF_STMT.
> 
> gcc/testsuite
>   * g++.dg/nested-switch.C: New test case.

C++ testcase shouldn't be put directly into g++.dg/ (though yes, several
have been), but into a subdirectory.  In this case, because it is a (bogus)
warning, it should best go into g++.dg/warn/, and perhaps best named as
g++.dg/warn/Wimplicit-fallthrough-3.C .

> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/nested-switch.C
> @@ -0,0 +1,30 @@
> +// Verify that there are no spurious warnings in nested switch statements due
> +// to the unnecessary break in the inner switch block.
> +// { dg-do compile }
> +// { dg-options "-Werror=implicit-fallthrough" } */

I'd go just for -Wimplicit-fallthrough.

> +
> +int
> +foo (int c1, int c2, int c3)
> +{
> +  switch (c2)
> +{
> +case 0:   
> +  switch (c3) {

And turn the above line into
  switch (c3) { // { dg-bogus "may fall through" }

> + case 0:
> +   if (c1)
> + return 1;
> +   else
> + return 2;
> +   break;
> +
> + default:
> +   return 3;
> +  }
> +
> +case 1: 
> +  return 4;
> +default:
> +  return 5;
> +  break;
> +}
> +}

Ok for trunk with those nits, thanks.

Jakub

Fixed incorrect TARGET_MEM_REF alignment (PR 84419)

[Richard Sandiford]

expand_call_mem_ref checks for TARGET_MEM_REFs that have compatible
type, but it didn't then go on to install the specific type we need,
which might have different alignment due to:

  if (TYPE_ALIGN (type) != align)
type = build_aligned_type (type, align);

This was causing masked stores to be incorrectly marked as
aligned on AVX512.

Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64-linux-gnu.
Also tested by Alexander on AVX512 hw.  OK to install?

Richard


2018-02-20  Richard Sandiford  

gcc/
PR tree-optimization/84419
* internal-fn.c (expand_call_mem_ref): Create a TARGET_MEM_REF
with the required type if its current type is compatible but
different.

gcc/testsuite/
PR tree-optimization/84419
* gcc.dg/vect/pr84419.c: New test.

Index: gcc/internal-fn.c
===
--- gcc/internal-fn.c   2018-01-13 18:01:51.235735290 +
+++ gcc/internal-fn.c   2018-02-20 09:40:47.547217244 +
@@ -2444,11 +2444,14 @@ expand_call_mem_ref (tree type, gcall *s
  && types_compatible_p (TREE_TYPE (mem), type))
{
  tree offset = TMR_OFFSET (mem);
- if (alias_ptr_type != TREE_TYPE (offset) || !integer_zerop (offset))
+ if (type != TREE_TYPE (mem)
+ || alias_ptr_type != TREE_TYPE (offset)
+ || !integer_zerop (offset))
{
  mem = copy_node (mem);
  TMR_OFFSET (mem) = wide_int_to_tree (alias_ptr_type,
   wi::to_poly_wide (offset));
+ TREE_TYPE (mem) = type;
}
  return mem;
}
Index: gcc/testsuite/gcc.dg/vect/pr84419.c
===
--- /dev/null   2018-02-19 19:34:42.906488063 +
+++ gcc/testsuite/gcc.dg/vect/pr84419.c 2018-02-20 09:40:47.548217201 +
@@ -0,0 +1,21 @@
+#include 
+
+#define SIZE 400
+
+int  foo[SIZE];
+char bar[SIZE];
+
+void __attribute__ ((noinline)) foo_func(void)
+{
+  int i;
+  for (i = 1; i < SIZE; i++)
+if (bar[i])
+  foo[i] = 1;
+}
+
+int main()
+{
+  memset(bar, 1, sizeof(bar));
+  foo_func();
+  return 0;
+}

Re: [PATCH] c++: Fix spurious fallthrough warning on break

[Siddhesh Poyarekar]

On Monday 19 February 2018 09:59 PM, Siddhesh Poyarekar wrote:
> The C++ frontend generates a break that results in the fallthrough
> warning misfiring in nested switch blocks where cases in the inner
> switch block return, rendering the break pointless.  The fallthrough
> detection in finish_break_stmt does not work either because the
> condition is encoded as an IF_STMT and not a COND_EXPR.
> 
> Fix this by adding a condition for IF_STMT in the
> langhooks.block_may_fallthru for C++.  Fix tested on x86_64.  Full
> testsuite run is in progress.

Now confirmed that the patch does not introduce any new regressions.

Siddhesh

Re: [PATCH] Add limit for maximal alignment options (PR c/84310).

[Jakub Jelinek]

On Mon, Feb 12, 2018 at 01:09:43PM +0100, Martin Liška wrote:
> Following patch fixes 2 issues with -falign-*:
> 1) when using -malign-x=16 (or corresponding -falign-* value) then ICE 
> appeared
> as code in final.c can deal just with limited alignment.
> 2) thus I also documented and limited the maximum value of -falign-* options.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> i386.exp test-suite works fine on x86_64 machine.
> 
> Ready to be installed?
> Martin
> 
> gcc/ChangeLog:
> 
> 2018-02-09  Martin Liska  
> 
>   PR c/84310
>   PR target/79747
>   * final.c (shorten_branches): Build align_tab array with one
>   more element.
>   * opts.c (finish_options): Add alignment option limit check.
>   (MAX_CODE_ALIGN): Likewise.
>   (MAX_CODE_ALIGN_VALUE): Likewise.
>   * doc/invoke.texi: Document maximum allowed option value for
>   all -falign-* options.
> 
> gcc/testsuite/ChangeLog:
> 
> 2018-02-12  Martin Liska  
> 
>   PR c/84310
>   PR target/79747
>   * gcc.target/i386/pr84310.c: New test.
>   * gcc.target/i386/pr84310-2.c: Likewise.

Ok, thanks.

Jakub

Re: [PATCH] Add limit for maximal alignment options (PR c/84310).

[Martin Liška]

PING^1

On 02/12/2018 01:09 PM, Martin Liška wrote:
> Hi.
> 
> Following patch fixes 2 issues with -falign-*:
> 1) when using -malign-x=16 (or corresponding -falign-* value) then ICE 
> appeared
> as code in final.c can deal just with limited alignment.
> 2) thus I also documented and limited the maximum value of -falign-* options.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> i386.exp test-suite works fine on x86_64 machine.
> 
> Ready to be installed?
> Martin
> 
> gcc/ChangeLog:
> 
> 2018-02-09  Martin Liska  
> 
>   PR c/84310
>   PR target/79747
>   * final.c (shorten_branches): Build align_tab array with one
>   more element.
>   * opts.c (finish_options): Add alignment option limit check.
>   (MAX_CODE_ALIGN): Likewise.
>   (MAX_CODE_ALIGN_VALUE): Likewise.
>   * doc/invoke.texi: Document maximum allowed option value for
>   all -falign-* options.
> 
> gcc/testsuite/ChangeLog:
> 
> 2018-02-12  Martin Liska  
> 
>   PR c/84310
>   PR target/79747
>   * gcc.target/i386/pr84310.c: New test.
>   * gcc.target/i386/pr84310-2.c: Likewise.
> ---
>  gcc/doc/invoke.texi   |  4 
>  gcc/final.c   |  4 ++--
>  gcc/opts.c| 20 
>  gcc/testsuite/gcc.target/i386/pr84310-2.c | 10 ++
>  gcc/testsuite/gcc.target/i386/pr84310.c   |  8 
>  5 files changed, 44 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr84310-2.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr84310.c
> 
>