Re: [PATCH 3/3] Come up with new --completion option.

[Martin Liška]

On 06/22/2018 06:09 PM, Jeff Law wrote:
> On 06/22/2018 05:25 AM, Martin Liška wrote:
>> On 06/20/2018 05:27 PM, David Malcolm wrote:
>>> On Mon, 2018-05-14 at 14:51 +0200, Martin Liška wrote:
 Main part where I still need to write ChangeLog file and
 gcc.sh needs to be moved to bash-completions project.

 Martin
>>>
>>> As before, I'm not an official reviewer for it, but it touches code
>>> that I wrote, so here goes.
>>>
>>> Overall looks good to me, but I have some nitpicks:
>>>
>>> (needs a ChangeLog)
>>
>> Added.
>>
>>>
>>> [...snip gcc.sh change; I don't feel qualified to comment...]
>>>
>>> [...snip sane-looking changes to common.opt and gcc.c.  */
>>>  
>>> diff --git a/gcc/opt-suggestions.c b/gcc/opt-suggestions.c
>>> index e322fcd91c5..2da094a5960 100644
>>> --- a/gcc/opt-suggestions.c
>>> +++ b/gcc/opt-suggestions.c
>>> @@ -28,18 +28,6 @@ along with GCC; see the file COPYING3.  If not see
>>>  #include "opt-suggestions.h"
>>>  #include "selftest.h"
>>>  
>>> -option_proposer::~option_proposer ()
>>> -{
>>> -  if (m_option_suggestions)
>>> -{
>>> -  int i;
>>> -  char *str;
>>> -  FOR_EACH_VEC_ELT (*m_option_suggestions, i, str)
>>> -   free (str);
>>> -  delete m_option_suggestions;
>>> -}
>>> -}
>>>
>>> Why is this dtor going away?  If I'm reading the patch correctly,
>>> option_proposer still "owns" m_option_suggestions.
>>>
>>> What happens if you run "make selftest-valgrind" ?  (my guess is some
>>> new memory leaks)
>>
>> Fixed that, should not be removed.
>>
>>>  
>>> +void
>>> +option_proposer::get_completions (const char *option_prefix,
>>> + auto_string_vec &results)
>>>
>>> Missing leading comment.  Maybe something like:
>>>
>>> /* Populate RESULTS with valid completions of options that begin
>>>with OPTION_PREFIX.  */
>>>
>>> or somesuch.
>>>
>>> +{
>>> +  /* Bail out for an invalid input.  */
>>> +  if (option_prefix == NULL || option_prefix[0] == '\0')
>>> +return;
>>> +
>>> +  /* Option suggestions are built without first leading dash character.  */
>>> +  if (option_prefix[0] == '-')
>>> +option_prefix++;
>>> +
>>> +  size_t length = strlen (option_prefix);
>>> +
>>> +  /* Handle parameters.  */
>>> +  const char *prefix = "-param";
>>> +  if (length >= strlen (prefix)
>>> +  && strstr (option_prefix, prefix) == option_prefix)
>>>
>>> Maybe reword that leading comment to
>>>
>>>/* Handle OPTION_PREFIX starting with "-param".  */
>>>
>>> (or somesuch) to clarify the intent?
>>
>> Thanks, done.
>>
>>>
>>> [...snip...]
>>>
>>> +void
>>> +option_proposer::suggest_completion (const char *option_prefix)
>>> +{
>>>
>>> Missing leading comment.  Maybe something like:
>>>
>>> /* Print on stdout a list of valid options that begin with OPTION_PREFIX,
>>>one per line, suitable for use by Bash completion.
>>>
>>>Implementation of the "-completion=" option.  */
>>>
>>> or somesuch.
>>
>> Likewise.
>>
>>>
>>> [...snip...]
>>>
>>> +void
>>> +option_proposer::find_param_completions (const char separator,
>>> +const char *option_prefix,
>>> +auto_string_vec &results)
>>>
>>> Maybe rename "option_prefix" to "param_prefix"?  I believe it's now
>>> the prefix of the param name, rather than the option.
>>
>> Makes sense.
>>
>>>
>>> Missing leading comment.  Maybe something like:
>>>
>>> /* Populate RESULTS with bash-completions options for all parameters
>>>that begin with PARAM_PREFIX, using SEPARATOR.
>>
>> It's in header file, thus I copied that.
>>
>>>
>>>For example, if PARAM_PREFIX is "max-" and SEPARATOR is "=" then
>>>strings of the form:
>>>  "--param=max-unrolled-insns"
>>>  "--param=max-early-inliner-iterations"
>>>will be added to RESULTS.  */
>>>
>>> (did I get that right?)
>>
>> Yes.
>>
>>>
>>> +{
>>> +  char separator_str[] {separator, '\0'};
>>>
>>> Is the above initialization valid C++98, or is it a C++11-ism?
>>
>> You are right. I fixed that and 2 more occurrences of the same
>> issue.
>>
>>>
>>> +  size_t length = strlen (option_prefix);
>>> +  for (unsigned i = 0; i < get_num_compiler_params (); ++i)
>>> +{
>>> +  const char *candidate = compiler_params[i].option;
>>> +  if (strlen (candidate) >= length
>>> + && strstr (candidate, option_prefix) == candidate)
>>> +   results.safe_push (concat ("--param", separator_str, candidate, NULL));
>>> +}
>>> +}
>>> +
>>> +#if CHECKING_P
>>> +
>>> +namespace selftest {
>>> +
>>> +/* Verify that PROPOSER generates sane auto-completion suggestions
>>> +   for OPTION_PREFIX.  */
>>> +static void
>>> +verify_autocompletions (option_proposer &proposer, const char 
>>> *option_prefix)
>>> +{
>>> +  auto_string_vec suggestions;
>>> +  proposer.get_completions (option_prefix, suggestions);
>>>
>>>
>>> Maybe a comment here to specify:
>>>
>>>/* There must be at least one suggestion, and every suggestion must
>>>   indeed begin with

Re: [PATCH][Middle-end]3rd patch of PR78809

[Richard Biener]

On Wed, 27 Jun 2018, Jeff Law wrote:

> 
> On 06/18/2018 09:37 AM, Qing Zhao wrote:
> > Hi,
> > 
> > this is the 3rd (and the last) patch for PR78809:
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78809
> > Inline strcmp with small constant strings
> > 
> > The design doc for PR78809 is at:
> > https://www.mail-archive.com/gcc@gcc.gnu.org/msg83822.html
> > 
> > this patch is for the third part of change of PR78809.
> > 
> > C. for strcmp (s1, s2), strncmp (s1, s2, n), and memcmp (s1, s2, n)
> >if the result is NOT used to do simple equality test against zero, one of
> > "s1" or "s2" is a small constant string, n is a constant, and the Min value 
> > of
> > the length of the constant string and "n" is smaller than a predefined
> > threshold T,
> >inline the call by a byte-to-byte comparision sequence to avoid calling
> > overhead.
> > 
> > adding test case strcmpopt_5.c into gcc.dg for part C of PR78809.
> > 
> > bootstraped and tested on both X86 and Aarch64. no regression.
> > 
> > I have done some experiments to check the run-time performance impact 
> > and code-size impact from such inlining with different threshold for the
> > length of the constant string to inline, the data were recorded in the 
> > attachments of 
> > PR78809:
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78809.
> > 
> > and finally decide that the default value of the threshold is 3. 
> > this value of the threshold can be adjusted by the new option:
> > 
> > --param builtin-string-cmp-inline-length=
> > 
> > The following is the patch.
> > 
> > thanks.
> > 
> > Qing
> > 
> > gcc/ChangeLog:
> > 
> > +2018-06-18  Qing Zhao  
> > +
> > +   PR middle-end/78809
> > +   * builtins.c (expand_builtin_memcmp): Inline the calls first
> > +   when result_eq is false.
> > +   (expand_builtin_strcmp): Inline the calls first.
> > +   (expand_builtin_strncmp): Likewise.
> > +   (inline_string_cmp): New routine. Expand a string compare 
> > +   call by using a sequence of char comparison.
> > +   (inline_expand_builtin_string_cmp): New routine. Inline expansion
> > +   a call to str(n)cmp/memcmp.
> > +   * doc/invoke.texi (--param builtin-string-cmp-inline-length): New 
> > option.
> > +   * params.def (BUILTIN_STRING_CMP_INLINE_LENGTH): New.
> > +
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > +2018-06-18  Qing Zhao  
> > +
> > +   PR middle-end/78809
> > +   * gcc.dg/strcmpopt_5.c: New test.
> So I still need to dig into this patch.  But I wanted to raise an
> potential issue and get yours and Martin's thoughts on it.
> 
> Martin (and others) have been working hard to improve GCC's ability to
> give good diagnostics for various problems with calls to mem* and str*
> functions (buffer overflows, restrict issues, etc).
> 
> One of the problems Martin has identified is early conversion of these
> calls into inlined direct operations.  If those conversions happen prior
> to the analysis for warnings we obviously can't issue any relevant warnings.

>From the looks of the changelog this seems to be done at RTL expansion 
time -- which also makes me question why targets do not provide
cmpstr[n] / cmpmem optabs when doing the inlining is so obviously
beneficial?

Note I didn't look at the actual patch.

Richard.

> 
> > +
> > 
> > 
> > 
> > 0001-3nd-Patch-for-PR78009.patch
> > 
> > 
> > From fcf6ced44e6e3e4a14894cc8f43ac39bc17aafee Mon Sep 17 00:00:00 2001
> > From: qing zhao 
> > Date: Thu, 14 Jun 2018 14:32:46 -0700
> > Subject: [PATCH] 3nd Patch for PR78009
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78809
> > Inline strcmp with small constant strings
> > 
> > The design doc for PR78809 is at:
> > https://www.mail-archive.com/gcc@gcc.gnu.org/msg83822.html
> > 
> > this patch is for the third part of change of PR78809.
> > 
> > C. for strcmp (s1, s2), strncmp (s1, s2, n), and memcmp (s1, s2, n)
> >if the result is NOT used to do simple equality test against zero, one of
> > "s1" or "s2" is a small constant string, n is a constant, and the Min value 
> > of
> > the length of the constant string and "n" is smaller than a predefined
> > threshold T,
> >inline the call by a byte-to-byte comparision sequence to avoid calling
> > overhead.
> > 
> > adding test case strcmpopt_5.c into gcc.dg for part C of PR78809.
> > 
> > bootstraped and tested on both X86 and Aarch64. no regression.
> > ---
> >  gcc/builtins.c | 172 
> > +++--
> >  gcc/doc/invoke.texi|   5 ++
> >  gcc/params.def |   4 +
> >  gcc/testsuite/gcc.dg/strcmpopt_5.c |  80 +
> >  4 files changed, 253 insertions(+), 8 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/strcmpopt_5.c
> > 
> > diff --git a/gcc/builtins.c b/gcc/builtins.c
> > index 6b3e6b2..cf50d05 100644
> > --- a/gcc/builtins.c
> > +++ b/gcc/builtins.c
> > @@ -4358,6 +4360,17 @@ expand_builtin_memcmp (tree exp, rtx target, bool 
> > r

Re: [patch, i386] false dependencies fix

[Uros Bizjak]

Hello!

>> --- i386.md (revision 259756)
>> +++ i386.md (working copy)
>> @@ -3547,7 +3547,7 @@
>>   {
>>   case MODE_DF:
>>if (TARGET_AVX && REG_P (operands[0]) && REG_P (operands[1]))
>> -return "vmovsd\t{%1, %0, %0|%0, %0, %1}";
>> +return "%vmovsd\t{%d1, %0|%0, %d1}";
>>return "%vmovsd\t{%1, %0|%0, %1}";
>>
>>   case MODE_V4SF:
>> @@ -3748,7 +3748,7 @@
>>   {
>>   case MODE_SF:
>>if (TARGET_AVX && REG_P (operands[0]) && REG_P (operands[1]))
>> -return "vmovss\t{%1, %0, %0|%0, %0, %1}";
>> +return "%vmovss\t{%d1, %0|%0, %d1}";
>>return "%vmovss\t{%1, %0|%0, %1}";
> So what I'm confused about is in the original output template operand 0
> is duplicated. In the new template operand 1 is duplicated.
>
> Presumably what you're trying to accomplish is avoiding a false read on
> operand 0 (the destination)?  Can you please confirm?

> Knowing that should also help me evaluate the changes to recp and rsqrt
> since they're being changed to the same style encoding when operating
> strictly on registers.

Please don't change "v" -> "%v" for TARGET_AVX templates. We know that
in this case, all insn mnemonics are prefixed with "v".

Uros.

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

Hi Thomas,

> if we add a warning, we should add it both for
>
> if (flag .and. func())
>
> and for
>
> if (func() .and. flag)
>
> because the standard also allows reversing the test (which my
> original test did).

well, I really don't want to warn for hypothetical problems. Since we
currently do not apply such an optimization, we should not warn about
it (unless you find any other compiler which actually does).

Whether to add this optimization in the future is a different topic of
course, and can still be discussed later.

Cheers,
Janus

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

[Mikhail Kashkarov]

^ gentle ping.


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

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

[PATCH v2 0/4] some vxworks/powerpc patches

[Rasmus Villemoes]

Patches 1, 2 and 4 (the latter was number 6 previously) are unchanged
from the first round, apart from small changes in the commit log
wording. Patch 3 now includes parentheses in the macro
definition. Patches 4 and 5 are dropped.

4/4 is mostly a minor optimization, omitting a tiny bit of dead code
from the compiler binary. But the preprocessor directives also serve
as a reminder that the custom vxworks functions will not be used if
the compiler is built with --enable-initfini-array.

Rasmus Villemoes (4):
  vxworks: add target/h/wrn/coreip to the set of system include paths
  libgcc: add crt{begin,end} for powerpc-wrs-vxworks target
  vxworks: enable use of .init_array/.fini_array for cdtors
  vxworks: don't define vxworks_asm_out_constructor when using
.init_array

 gcc/config/vxworks.c |  9 +++--
 gcc/config/vxworks.h | 21 +++--
 libgcc/config.host   |  1 +
 3 files changed, 23 insertions(+), 8 deletions(-)

-- 
2.16.4

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

[Rasmus Villemoes]

In order to allow ZCX on VxWorks, we need the frame_dummy function to do
the register_frame_info(). So make sure crtbegin.o and crtend.o are
available for use with a custom spec file.

2018-06-04  Rasmus Villemoes  

libgcc/

* config.host: Add crtbegin.o and crtend.o for
  powerpc-wrs-vxworks target.
---
 libgcc/config.host | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libgcc/config.host b/libgcc/config.host
index 18cabaf24f6..3466ba70f27 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -1129,6 +1129,7 @@ powerpc*-*-linux*)
;;
 powerpc-wrs-vxworks*)
tmake_file="$tmake_file rs6000/t-ppccomm rs6000/t-savresfgpr t-fdpbit"
+   extra_parts="$extra_parts crtbegin.o crtend.o"
;;
 powerpc-*-lynxos*)
tmake_file="$tmake_file rs6000/t-lynx t-fdpbit"
-- 
2.16.4

[PATCH v2 3/4] vxworks: enable use of .init_array/.fini_array for cdtors

[Rasmus Villemoes]

The target OS actually runs all function pointers found in the _ctors
array when the module is loaded. So it is not that hard to make use of
the "modern" .init_array/.fini_array mechanism - it mostly just requires
a linker script adding the _ctors and _dtors symbols and terminating
LONG(0) entries.

Assume that if the user passed --enable-initfini-array when building
gcc, the rest of the toolchain (including the link spec and linker
script) is set up appropriately.

Note that configuring with --enable-initfini-array may prevent the -mrtp
mode from working, due to the (unconditional) use of .init_array.*
sections instead of .ctors.* - however, that is the case regardless of this
patch.

2018-06-04  Rasmus Villemoes  

gcc/
config/vxworks.c: Set targetm.have_ctors_dtors if 
HAVE_INITFINI_ARRAY_SUPPORT.
config/vxworks.h: Set SUPPORTS_INIT_PRIORITY if 
HAVE_INITFINI_ARRAY_SUPPORT.
---
 gcc/config/vxworks.c |  7 +--
 gcc/config/vxworks.h | 10 +++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/gcc/config/vxworks.c b/gcc/config/vxworks.c
index 061f02057c2..953f74f71af 100644
--- a/gcc/config/vxworks.c
+++ b/gcc/config/vxworks.c
@@ -143,8 +143,11 @@ vxworks_override_options (void)
   targetm.emutls.debug_form_tls_address = true;
 }
 
-  /* We can use .ctors/.dtors sections only in RTP mode.  */
-  targetm.have_ctors_dtors = TARGET_VXWORKS_RTP;
+  /* We can use .ctors/.dtors sections only in RTP mode.  But, if the
+ compiler was built with --enable-initfini-array, assume the
+ toolchain implements the proper glue to make .init_array and
+ .fini_array work.  */
+  targetm.have_ctors_dtors = TARGET_VXWORKS_RTP || HAVE_INITFINI_ARRAY_SUPPORT;
 
   /* PIC is only supported for RTPs.  */
   if (flag_pic && !TARGET_VXWORKS_RTP)
diff --git a/gcc/config/vxworks.h b/gcc/config/vxworks.h
index 08d2c9d76d6..4c2d98381f6 100644
--- a/gcc/config/vxworks.h
+++ b/gcc/config/vxworks.h
@@ -142,9 +142,13 @@ along with GCC; see the file COPYING3.  If not see
 #define VXWORKS_OVERRIDE_OPTIONS vxworks_override_options ()
 extern void vxworks_override_options (void);
 
-/* Only RTPs support prioritized constructors and destructors:
-   the implementation relies on numbered .ctors* sections.  */
-#define SUPPORTS_INIT_PRIORITY TARGET_VXWORKS_RTP
+/* RTPs support prioritized constructors and destructors: the
+   implementation relies on numbered .ctors* sections. If the compiler
+   was built with --enable-initfini-array, we assume the user uses a
+   linker script that sorts and merges the .init_array.* sections
+   appropriately.  */
+#define SUPPORTS_INIT_PRIORITY \
+  (TARGET_VXWORKS_RTP || HAVE_INITFINI_ARRAY_SUPPORT)
 
 /* VxWorks requires special handling of constructors and destructors.
All VxWorks configurations must use these functions.  */
-- 
2.16.4

[PATCH v2 1/4] vxworks: add target/h/wrn/coreip to the set of system include paths

[Rasmus Villemoes]

In order to build crtstuff.c, I need to ensure the headers in
target/h/wrn/coreip are also searched. Of course, that can be done
similar to how wrn/coreip gets manually added for libgcc, e.g. by adding

  CRTSTUFF_T_CFLAGS += -I$(WIND_BASE)/target/h 
-I$(WIND_BASE)/target/h/wrn/coreip

But without target/h/wrn/coreip in the default search path, all
user-code that include  (crtstuff.c just being one such
example) will have to manually add an -isystem
flag adding the wrn/coreip directory: unistd.h include ioLib.h, which
has

#include "net/uio.h"

and that header is found in target/h/wrn/coreip. In other words, the
VxWorks system headers (at least for VxWorks 5.5) are written in a way
that assumes wrn/coreip is in the search path, so I think it makes sense
to have that by default.

It will change the search order for existing setups that pass
-I.../target/h/wrn/coreip (without -nostdinc), since that flag will now
be ignored. I can't know whether that will break anything, but I do
believe it makes sense to have the defaults actually usable without
expecting all invocations to add -I/-isystem flags.

2018-06-04  Rasmus Villemoes  

gcc/

* config/vxworks.h: Add $(WIND_BASE)/target/h/wrn/coreip to
  default search path.
---
 gcc/config/vxworks.h | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/gcc/config/vxworks.h b/gcc/config/vxworks.h
index e37af775157..08d2c9d76d6 100644
--- a/gcc/config/vxworks.h
+++ b/gcc/config/vxworks.h
@@ -61,9 +61,12 @@ along with GCC; see the file COPYING3.  If not see
 #undef VXWORKS_ADDITIONAL_CPP_SPEC
 #define VXWORKS_ADDITIONAL_CPP_SPEC\
  "%{!nostdinc: \
-%{isystem*} -idirafter \
-%{mrtp: %:getenv(WIND_USR /h)  \
-  ;:%:getenv(WIND_BASE /target/h)}}"
+%{isystem*}\
+%{mrtp: -idirafter %:getenv(WIND_USR /h)   \
+   -idirafter %:getenv(WIND_USR /h/wrn/coreip) \
+  ;:-idirafter %:getenv(WIND_BASE /target/h) \
+   -idirafter %:getenv(WIND_BASE /target/h/wrn/coreip) \
+}}"
 
 #endif
 
-- 
2.16.4

[PATCH v2 4/4] vxworks: don't define vxworks_asm_out_constructor when using .init_array

[Rasmus Villemoes]

When the compiler is configured with --enable-initfini-array,
config/initfini-array.h gets included after config/vxworks.h by tm.h, so
the definitions of TARGET_ASM_CONSTRUTOR/TARGET_ASM_DESTRUCTOR in
vxworks.h get undone in initfini-array.h. Hence, we might as well not
define the vxworks_asm_out_* functions.

2018-06-01  Rasmus Villemoes  

gcc/
* config/vxworks.h: Guard vxworks_asm_out_constructor and
  vxworks_asm_out_destructor by !HAVE_INITFINI_ARRAY_SUPPORT
* config/vxworks.c: Likewise.
---
 gcc/config/vxworks.c | 2 ++
 gcc/config/vxworks.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/gcc/config/vxworks.c b/gcc/config/vxworks.c
index 953f74f71af..3b6b2343859 100644
--- a/gcc/config/vxworks.c
+++ b/gcc/config/vxworks.c
@@ -28,6 +28,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "output.h"
 #include "fold-const.h"
 
+#if !HAVE_INITFINI_ARRAY_SUPPORT
 /* Like default_named_section_asm_out_constructor, except that even
constructors with DEFAULT_INIT_PRIORITY must go in a numbered
section on VxWorks.  The VxWorks runtime uses a clever trick to get
@@ -56,6 +57,7 @@ vxworks_asm_out_destructor (rtx symbol, int priority)
/*constructor_p=*/false);
   assemble_addr_to_section (symbol, sec);
 }
+#endif
 
 /* Return the list of FIELD_DECLs that make up an emulated TLS
variable's control object.  TYPE is the structure these are fields
diff --git a/gcc/config/vxworks.h b/gcc/config/vxworks.h
index 4c2d98381f6..86773868ec2 100644
--- a/gcc/config/vxworks.h
+++ b/gcc/config/vxworks.h
@@ -150,6 +150,7 @@ extern void vxworks_override_options (void);
 #define SUPPORTS_INIT_PRIORITY \
   (TARGET_VXWORKS_RTP || HAVE_INITFINI_ARRAY_SUPPORT)
 
+#if !HAVE_INITFINI_ARRAY_SUPPORT
 /* VxWorks requires special handling of constructors and destructors.
All VxWorks configurations must use these functions.  */
 #undef TARGET_ASM_CONSTRUCTOR
@@ -158,6 +159,7 @@ extern void vxworks_override_options (void);
 #define TARGET_ASM_DESTRUCTOR vxworks_asm_out_destructor
 extern void vxworks_asm_out_constructor (rtx symbol, int priority);
 extern void vxworks_asm_out_destructor (rtx symbol, int priority);
+#endif
 
 /* Override the vxworks-dummy.h definitions.  TARGET_VXWORKS_RTP
is defined by vxworks.opt.  */
-- 
2.16.4

RE: [patch, i386] false dependencies fix

[Nesterovskiy, Alexander]

Hello!

> So what I'm confused about is in the original output template operand 
> 0 is duplicated. In the new template operand 1 is duplicated.
>
> Presumably what you're trying to accomplish is avoiding a false read 
> on operand 0 (the destination)?  Can you please confirm?

> Knowing that should also help me evaluate the changes to recp and 
> rsqrt since they're being changed to the same style encoding when 
> operating strictly on registers.

Yes, it's the same for all instructions in the patch - we're not just avoiding
read but present more possibilities to execute speculatively for CPU here.

The destination depends only on the source after the patch, and (thanks
to CPU register renaming) CPU can successfully execute this instruction
even if some previous instruction with write to the same destination is
not finished currently.

--
Alexander Nesterovskiy

Re: [C++ Patch] More location fixes to grokdeclarator

[Paolo Carlini]

Hi,

On 28/06/2018 03:22, David Malcolm wrote:

[snip]

If I'm following you right, the idea is that gcc should complain
because two different things in the user's source code contradict
each
other.

In such circumstances, I think we ought to try to print *both*
locations, so that we're showing, rather than just telling.

Or to put in another way, if two things in the user's source contradict
each other, we should show the user both.  The user is going to have to
decide to delete one (or both) of them, and we don't know which one,
but at least by showing both it helps him/her take their next action.
Sure, makes sense. Thus the below uses rich_location the way you 
explained. I also added 2 specific testcases and extended a bit another 
one to exercise a bit more min_location..Of course the patch doesn't add 
max_location anymore, I suspect we are not going to find uses for a max 
anytime soon, because we really want rich_location with multiple ranges 
in all those cases...


Thanks!
Paolo.

/


Index: cp/decl.c
===
--- cp/decl.c   (revision 262214)
+++ cp/decl.c   (working copy)
@@ -8545,15 +8545,18 @@ check_concept_fn (tree fn)
 {
   // A constraint is nullary.
   if (DECL_ARGUMENTS (fn))
-error ("concept %q#D declared with function parameters", fn);
+error_at (DECL_SOURCE_LOCATION (fn),
+ "concept %q#D declared with function parameters", fn);
 
   // The declared return type of the concept shall be bool, and
   // it shall not be deduced from it definition.
   tree type = TREE_TYPE (TREE_TYPE (fn));
   if (is_auto (type))
-error ("concept %q#D declared with a deduced return type", fn);
+error_at (DECL_SOURCE_LOCATION (fn),
+ "concept %q#D declared with a deduced return type", fn);
   else if (type != boolean_type_node)
-error ("concept %q#D with non-% return type %qT", fn, type);
+error_at (DECL_SOURCE_LOCATION (fn),
+ "concept %q#D with non-% return type %qT", fn, type);
 }
 
 /* Helper function.  Replace the temporary this parameter injected
@@ -9808,16 +9811,32 @@ smallest_type_quals_location (int type_quals, cons
 loc = locations[ds_const];
 
   if ((type_quals & TYPE_QUAL_VOLATILE)
-  && (loc == UNKNOWN_LOCATION || locations[ds_volatile] < loc))
+  && (loc == UNKNOWN_LOCATION
+ || linemap_location_before_p (line_table,
+   locations[ds_volatile], loc)))
 loc = locations[ds_volatile];
 
   if ((type_quals & TYPE_QUAL_RESTRICT)
-  && (loc == UNKNOWN_LOCATION || locations[ds_restrict] < loc))
+  && (loc == UNKNOWN_LOCATION
+ || linemap_location_before_p (line_table,
+   locations[ds_restrict], loc)))
 loc = locations[ds_restrict];
 
   return loc;
 }
 
+/* Returns the smallest location that is not UNKNOWN_LOCATION.  */
+
+static location_t
+min_location (location_t loca, location_t locb)
+{
+  if (loca == UNKNOWN_LOCATION
+  || (locb != UNKNOWN_LOCATION
+ && linemap_location_before_p (line_table, locb, loca)))
+return locb;
+  return loca;
+}
+
 /* Check that it's OK to declare a function with the indicated TYPE
and TYPE_QUALS.  SFK indicates the kind of special function (if any)
that this function is.  OPTYPE is the type given in a conversion
@@ -10710,14 +10729,20 @@ grokdeclarator (const cp_declarator *declarator,
 {
   if (staticp == 2)
{
- error ("member %qD cannot be declared both % "
-"and %", dname);
+ rich_location richloc (line_table, declspecs->locations[ds_virtual]);
+ richloc.add_range (declspecs->locations[ds_storage_class], false);
+ error_at (&richloc, "member %qD cannot be declared both % "
+   "and %", dname);
  storage_class = sc_none;
  staticp = 0;
}
   if (constexpr_p)
-   error ("member %qD cannot be declared both % "
-  "and %", dname);
+   {
+ rich_location richloc (line_table, declspecs->locations[ds_virtual]);
+ richloc.add_range (declspecs->locations[ds_constexpr], false);
+ error_at (&richloc, "member %qD cannot be declared both % "
+   "and %", dname);
+   }
 }
   friendp = decl_spec_seq_has_spec_p (declspecs, ds_friend);
 
@@ -10726,18 +10751,27 @@ grokdeclarator (const cp_declarator *declarator,
 {
   if (typedef_p)
{
- error ("typedef declaration invalid in parameter declaration");
+ error_at (declspecs->locations[ds_typedef],
+   "typedef declaration invalid in parameter declaration");
  return error_mark_node;
}
   else if (template_parm_flag && storage_class != sc_none)
{
- error ("storage class specified for template parameter %qs", name);
+ error_at (min_location (declspecs->locations[ds_thread],
+ 

Re: [patch] Remap goto_locus on edges during inlining

[Richard Biener]

On Tue, Jun 26, 2018 at 10:12 PM Jeff Law  wrote:
>
> On 06/26/2018 02:54 AM, Eric Botcazou wrote:
> > Hi,
> >
> > this makes sure the goto_locus present (or not) on edges is properly 
> > remapped
> > during inlining.
> >
> > Tested on x86-64/Linux, OK for the mainline?
> >
> >
> > 2018-06-26  Eric Botcazou  
> >
> >   * tree-inline.c (remap_location): New function extracted from...
> >   (copy_edges_for_bb): Add ID parameter.  Remap goto_locus.
> >   (copy_phis_for_bb): ...here.  Call remap_location.
> >   (copy_cfg_body): Adjust call to copy_edges_for_bb.
> OK.

Related we're also missing to verify_location () on those in
verify_gimple_in_cfg.  Having stale references to GCed BLOCKs
via locations can be difficult to track down...

So can you add the verification bits as well?

Thanks,
Richard.

> jeff

Re: [PATCH] Hide alt_dump_file within dumpfile.c

[Richard Biener]

On Tue, Jun 26, 2018 at 10:27 PM David Malcolm  wrote:
>
> On Mon, 2018-06-25 at 15:34 +0200, Richard Biener wrote:
> > On Wed, Jun 20, 2018 at 6:34 PM David Malcolm 
> > wrote:
> > >
> > > Here's v3 of the patch (one big patch this time, rather than a
> > > kit).
> > >
> > > Like the v2 patch kit, this patch reuses the existing dump API,
> > > rather than inventing its own.
> > >
> > > Specifically, it uses the dump_* functions in dumpfile.h that don't
> > > take a FILE *, the ones that implicitly write to dump_file and/or
> > > alt_dump_file.  I needed a name for them, so I've taken to calling
> > > them the "structured dump API" (better name ideas welcome).
> > >
> > > v3 eliminates v2's optinfo_guard class, instead using "dump_*_loc"
> > > calls as delimiters when consolidating "dump_*" calls.  There's a
> > > new dump_context class which has responsibility for consolidating
> > > them into optimization records.
> > >
> > > The dump_*_loc calls now capture more than just a location_t: they
> > > capture the profile_count and the location in GCC's own sources
> > > where
> > > the dump is being emitted from.
> > >
> > > This works by introducing a new "dump_location_t" class as the
> > > argument of those dump_*_loc calls.  The dump_location_t can
> > > be constructed from a gimple * or from an rtx_insn *, so that
> > > rather than writing:
> > >
> > >   dump_printf_loc (MSG_NOTE, gimple_location (stmt),
> > >"some message: %i", 42);
> > >
> > > you can write:
> > >
> > >   dump_printf_loc (MSG_NOTE, stmt,
> > >"some message: %i", 42);
> > >
> > > and the dump_location_t constructor will grab the location_t and
> > > profile_count of stmt, and the location of the "dump_printf_loc"
> > > callsite (and gracefully handle "stmt" being NULL).
> > >
> > > Earlier versions of the patch captured the location of the
> > > dump_*_loc call via preprocessor hacks, or didn't work properly;
> > > this version of the patch works more cleanly: internally,
> > > dump_location_t is split into two new classes:
> > >   * dump_user_location_t: the location_t and profile_count within
> > > the *user's code*, and
> > >   * dump_impl_location_t: the __builtin_FILE/LINE/FUNCTION within
> > > the *implementation* code (i.e. GCC or a plugin), captured
> > > "automagically" via default params
> > >
> > > These classes are sometimes used elsewhere in the code.  For
> > > example, "vect_location" becomes a dump_user_location_t
> > > (location_t and profile_count), so that in e.g:
> > >
> > >   vect_location = find_loop_location (loop);
> > >
> > > it's capturing the location_t and profile_count, and then when
> > > it's used here:
> > >
> > >   dump_printf_loc (MSG_NOTE, vect_location, "foo");
> > >
> > > the dump_location_t is constructed from the vect_location
> > > plus the dump_impl_location_t at that callsite.
> > >
> > > In contrast, loop-unroll.c's report_unroll's "locus" param
> > > becomes a dump_location_t: we're interested in where it was
> > > called from, not in the locations of the various dump_*_loc calls
> > > within it.
> > >
> > > Previous versions of the patch captured a gimple *, and needed
> > > GTY markers; in this patch, the dump_user_location_t is now just a
> > > location_t and a profile_count.
> > >
> > > The v2 patch added an overload for dump_printf_loc so that you
> > > could pass in either a location_t, or the new type; this version
> > > of the patch eliminates that: they all now take dump_location_t.
> > >
> > > Doing so required adding support for rtx_insn *, so that one can
> > > write this kind of thing in RTL passes:
> > >
> > >   dump_printf_loc (MSG_NOTE, insn, "foo");
> > >
> > > One knock-on effect is that get_loop_location now returns a
> > > dump_user_location_t rather than a location_t, so that it has
> > > hotness information.
> > >
> > > Richi: would you like me to split out this location-handling
> > > code into a separate patch?  (It's kind of redundant without
> > > adding the remarks and optimization records work, but if that's
> > > easier I can do it)
> >
> > I think that would be easier because it doesn't require the JSON
> > stuff and so I'll happily approve it.
> >
> > Thus - trying to review that bits (and sorry for the delay).
> >
> > +  location_t srcloc = loc.get_location_t ();
> > +
> >if (dump_file && (dump_kind & pflags))
> >  {
> > -  dump_loc (dump_kind, dump_file, loc);
> > +  dump_loc (dump_kind, dump_file, srcloc);
> >print_gimple_stmt (dump_file, gs, spc, dump_flags |
> > extra_dump_flags);
> >  }
> >
> >if (alt_dump_file && (dump_kind & alt_flags))
> >  {
> > -  dump_loc (dump_kind, alt_dump_file, loc);
> > +  dump_loc (dump_kind, alt_dump_file, srcloc);
> >print_gimple_stmt (alt_dump_file, gs, spc, dump_flags |
> > extra_dump_flags);
> >  }
> > +
> > +  if (optinfo_enabled_p ())
> > +{
> > +  optinfo &info = begin_next_optinfo (loc);
> > +  info.h

Re: [PATCH 2/3][POPCOUNT] Check if zero check is done before entering the loop

[Richard Biener]

On Wed, Jun 27, 2018 at 7:01 AM Kugan Vivekanandarajah
 wrote:
>
> Hi Richard,
>
> Thanks for the review.
>
> On 25 June 2018 at 20:02, Richard Biener  wrote:
> > On Fri, Jun 22, 2018 at 11:14 AM Kugan Vivekanandarajah
> >  wrote:
> >>
> >> gcc/ChangeLog:
> >
> > The canonical way is calling simplify_using_initial_conditions on the
> > may_be_zero condition.
> >
> > Richard.
> >
> >> 2018-06-22  Kugan Vivekanandarajah  
> >>
> >> * tree-ssa-loop-niter.c (number_of_iterations_popcount): If popcount
> >> argument is checked for zero before entering loop, avoid checking 
> >> again.
> Do you like the attached patch which does this.

Yes.

OK if bootstrapped/tested.

Richard.

> Thanks,
> Kugan

Re: [9a/n] PR85694: Reorder vect_is_simple_use arguments

[Richard Biener]

On Wed, Jun 27, 2018 at 11:31 AM Richard Sandiford
 wrote:
>
> Richard Sandiford  writes:
> > Richard Biener  writes:
> >> On Mon, Jun 18, 2018 at 5:04 PM Richard Sandiford
> >>  wrote:
> >>>
> >>> When following the definitions of SSA names, some recognisers
> >>> already cope with statements that have been replaced by patterns.
> >>> This patch makes that happen automatically for users of
> >>> type_conversion_p and vect_get_internal_def.  It also adds
> >>> a vect_look_through_pattern helper that can be used directly.
> >>>
> >>> The reason for doing this is that the main patch for PR85694
> >>> makes over_widening handle more general cases.  These over-widened
> >>> patterns can still be useful when matching later statements;
> >>> e.g. an overwidened MULT_EXPR could be the input to a DOT_PROD_EXPR.
> >>>
> >>> The patch doesn't do anything with the STMT_VINFO_IN_PATTERN_P checks
> >>> in vect_recog_over_widening_pattern or vect_recog_widen_shift_pattern
> >>> since later patches rewrite them anyway.
> >>>
> >>> Doing this fixed an XFAIL in vect-reduc-dot-u16b.c.
> >>>
> >>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
> >>
> >> Hmm.  It seems to me that *def_stmt for vect_is_simple_use should
> >> eventually be the pattern def given the vectype overload takes the
> >> vectype from the pattern def already but oddly enough the
> >> DEF_TYPE is taken from the non-pattern stmt.
> >>
> >> I wonder which callers look at def_stmt at all (and how...)
> >>
> >> I guess swapping the def_stmt and dt arguments and adding yet another
> >> overload to remove all unused &def_stmt args might this easier to review...
> >>
> >> So - I'm suggesting to change vect_is_simple_use.
> >
> > OK, I'll try that.  Might end up being its own mini-series. :-)
>
> Turned out to be simpler than feared. :-)
>
> This patch does the first bit: reorder the arguments to
> vect_is_simple_use so that def_stmt comes last and is optional.
> Many callers can then drop it, making it more obvious which of the
> remaining calls would be affected by the next patch.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

OK.

Richard.

> Richard
>
>
> 2018-06-27  Richard Sandiford  
>
> gcc/
> * tree-vectorizer.h (vect_is_simple_use): Move the gimple ** to the
> end and default to null.
> * tree-vect-loop.c (vect_create_epilog_for_reduction)
> (vectorizable_reduction): Update calls accordingly, dropping the
> gimple ** argument if the passed-back statement isn't needed.
> * tree-vect-patterns.c (vect_get_internal_def, type_conversion_p)
> (vect_recog_rotate_pattern): Likewise.
> (vect_recog_mask_conversion_pattern): Likewise.
> * tree-vect-slp.c (vect_get_and_check_slp_defs): Likewise.
> (vect_mask_constant_operand_p): Likewise.
> * tree-vect-stmts.c (is_simple_and_all_uses_invariant, process_use):
> (vect_model_simple_cost, vect_get_vec_def_for_operand): Likewise.
> (get_group_load_store_type, get_load_store_type): Likewise.
> (vect_check_load_store_mask, vect_check_store_rhs): Likewise.
> (vectorizable_call, vectorizable_simd_clone_call): Likewise.
> (vectorizable_conversion, vectorizable_assignment): Likewise.
> (vectorizable_shift, vectorizable_operation): Likewise.
> (vectorizable_store, vect_is_simple_cond): Likewise.
> (vectorizable_condition, vectorizable_comparison): Likewise.
> (get_same_sized_vectype, vect_get_mask_type_for_stmt): Likewise.
> (vect_is_simple_use): Rename the def_stmt argument to def_stmt_out
> and move it to the end.  Cope with null def_stmt_outs.
>
> Index: gcc/tree-vectorizer.h
> ===
> --- gcc/tree-vectorizer.h   2018-06-27 10:27:12.366628072 +0100
> +++ gcc/tree-vectorizer.h   2018-06-27 10:27:31.782458413 +0100
> @@ -1476,10 +1476,10 @@ extern tree get_vectype_for_scalar_type_
>  extern tree get_mask_type_for_scalar_type (tree);
>  extern tree get_same_sized_vectype (tree, tree);
>  extern bool vect_get_loop_mask_type (loop_vec_info);
> -extern bool vect_is_simple_use (tree, vec_info *, gimple **,
> -enum vect_def_type *);
> -extern bool vect_is_simple_use (tree, vec_info *, gimple **,
> -   enum vect_def_type *, tree *);
> +extern bool vect_is_simple_use (tree, vec_info *, enum vect_def_type *,
> +   gimple ** = NULL);
> +extern bool vect_is_simple_use (tree, vec_info *, enum vect_def_type *,
> +   tree *, gimple ** = NULL);
>  extern bool supportable_widening_operation (enum tree_code, gimple *, tree,
> tree, enum tree_code *,
> enum tree_code *, int *,
> Index: gcc/tree-vect-loop.c
> ===

Re: [9b/n] PR85694: Make vect_is_simple_use look through pattern statements

[Richard Biener]

On Wed, Jun 27, 2018 at 11:34 AM Richard Sandiford
 wrote:
>
> Richard Sandiford  writes:
> > Richard Biener  writes:
> >> On Mon, Jun 18, 2018 at 5:04 PM Richard Sandiford
> >>  wrote:
> >>>
> >>> When following the definitions of SSA names, some recognisers
> >>> already cope with statements that have been replaced by patterns.
> >>> This patch makes that happen automatically for users of
> >>> type_conversion_p and vect_get_internal_def.  It also adds
> >>> a vect_look_through_pattern helper that can be used directly.
> >>>
> >>> The reason for doing this is that the main patch for PR85694
> >>> makes over_widening handle more general cases.  These over-widened
> >>> patterns can still be useful when matching later statements;
> >>> e.g. an overwidened MULT_EXPR could be the input to a DOT_PROD_EXPR.
> >>>
> >>> The patch doesn't do anything with the STMT_VINFO_IN_PATTERN_P checks
> >>> in vect_recog_over_widening_pattern or vect_recog_widen_shift_pattern
> >>> since later patches rewrite them anyway.
> >>>
> >>> Doing this fixed an XFAIL in vect-reduc-dot-u16b.c.
> >>>
> >>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
> >>
> >> Hmm.  It seems to me that *def_stmt for vect_is_simple_use should
> >> eventually be the pattern def given the vectype overload takes the
> >> vectype from the pattern def already but oddly enough the
> >> DEF_TYPE is taken from the non-pattern stmt.
> >>
> >> I wonder which callers look at def_stmt at all (and how...)
> >>
> >> I guess swapping the def_stmt and dt arguments and adding yet another
> >> overload to remove all unused &def_stmt args might this easier to review...
> >>
> >> So - I'm suggesting to change vect_is_simple_use.
> >
> > OK, I'll try that.  Might end up being its own mini-series. :-)
>
> And here's the second and final part: make vect_is_simple_use check
> whether a defining statement has been replaced by a pattern statement,
> and if so return the pattern statement instead.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

OK.

Thanks,
Richard.

> Richard
>
>
> 2018-06-14  Richard Sandiford  
>
> gcc/
> * tree-vect-loop.c (vectorizable_reduction): Assert that the
> phi is not a pattern statement and has not been replaced by
> a pattern statement.
> * tree-vect-patterns.c (type_conversion_p): Don't check
> STMT_VINFO_IN_PATTERN_P.
> (vect_recog_vector_vector_shift_pattern): Likewise.
> (vect_recog_dot_prod_pattern): Expect vect_is_simple_use to return
> the pattern statement rather than the original statement; check
> directly for a WIDEN_MULT_EXPR here.
> * tree-vect-slp.c (vect_get_and_check_slp_defs): Expect
> vect_is_simple_use to return the pattern statement rather
> than the original statement; use is_pattern_stmt_p to check
> for such a pattern statement.
> * tree-vect-stmts.c (process_use): Expect vect_is_simple_use
> to return the pattern statement rather than the original statement;
> don't do the same transformation here.
> (vect_is_simple_use): If the defining statement has been replaced
> by a pattern statement, return the pattern statement instead.
> Remove the corresponding (local) transformation from the vectype
> overload.
>
> gcc/testsuite/
> * gcc.dg/vect/vect-reduc-dot-u16b.c: Remove xfail and update the
> test for vectorization along the lines described in the comment.
>
> Index: gcc/tree-vect-loop.c
> ===
> --- gcc/tree-vect-loop.c2018-06-27 10:27:31.782458413 +0100
> +++ gcc/tree-vect-loop.c2018-06-27 10:27:34.490434751 +0100
> @@ -6482,6 +6482,8 @@ vectorizable_reduction (gimple *stmt, gi
>  }
>
>stmt_vec_info reduc_def_info = vinfo_for_stmt (reduc_def_stmt);
> +  /* PHIs should not participate in patterns.  */
> +  gcc_assert (!STMT_VINFO_RELATED_STMT (reduc_def_info));
>enum vect_reduction_type v_reduc_type
>  = STMT_VINFO_REDUC_TYPE (reduc_def_info);
>gimple *tmp = STMT_VINFO_REDUC_DEF (reduc_def_info);
> Index: gcc/tree-vect-patterns.c
> ===
> --- gcc/tree-vect-patterns.c2018-06-27 10:27:31.782458413 +0100
> +++ gcc/tree-vect-patterns.c2018-06-27 10:27:34.490434751 +0100
> @@ -193,13 +193,6 @@ type_conversion_p (tree name, gimple *us
>if (!*def_stmt)
>  return false;
>
> -  if (dt == vect_internal_def)
> -{
> -  stmt_vec_info def_vinfo = vinfo_for_stmt (*def_stmt);
> -  if (STMT_VINFO_IN_PATTERN_P (def_vinfo))
> -   return false;
> -}
> -
>if (!is_gimple_assign (*def_stmt))
>  return false;
>
> @@ -383,20 +376,11 @@ vect_recog_dot_prod_pattern (vec/* FORNOW.  Can continue analyzing the def-use chain when this stmt in a 
> phi
>   inside the loop (in case we are analyzing an outer-loop).  */
>gassig

[MAINTAINERS, committed] Add myself to write after approval

[Jackson Woodruff]

Add myself to write after approval in MAINTAINERS.

Committed as r262216.

Jackson

Index: ChangeLog
===
--- ChangeLog   (revision 262215)
+++ ChangeLog   (working copy)
@@ -1,3 +1,7 @@
+2018-06-28  Jackson Woodruff  
+
+   * MAINTAINERS (write after approval): Add myself.
+
 2018-06-26  Rasmus Villemoes  
 
* MAINTAINERS (write after approval): Add myself.
Index: MAINTAINERS
===
--- MAINTAINERS (revision 262215)
+++ MAINTAINERS (working copy)
@@ -614,6 +614,7 @@
 Ollie Wild 
 Kevin Williams 
 Carlo Wood 
+Jackson Woodruff   
 Mingjie Xing   
 Chenghua Xu
 Canqun Yang

Re: [patch] Do not leak location information during inlining (2nd try)

[Eric Botcazou]

> More references to input_location aren't ideal.  But I don't think
> that's a strong enough reason to reject.

This can be avoided relatively easily though, patch to that effect attached.

Tested on x86-64/Linux, OK for the mainline?


* tree-inline.c (remap_gimple_stmt): Replace input_location with
gimple_location (id->call_stmt) throughout.
(copy_phis_for_bb): Likewise.
(expand_call_inline): Likewise.  Tweak formatting.

-- 
Eric BotcazouIndex: tree-inline.c
===
--- tree-inline.c	(revision 262207)
+++ tree-inline.c	(working copy)
@@ -1631,7 +1631,7 @@ remap_gimple_stmt (gimple *stmt, copy_bo
    gimple_debug_bind_get_value (stmt),
    stmt);
 	  if (id->reset_location)
-	gimple_set_location (copy, input_location);
+	gimple_set_location (copy, gimple_location (id->call_stmt));
 	  id->debug_stmts.safe_push (copy);
 	  gimple_seq_add_stmt (&stmts, copy);
 	  return stmts;
@@ -1643,7 +1643,7 @@ remap_gimple_stmt (gimple *stmt, copy_bo
 			gimple_debug_source_bind_get_value (stmt),
 			stmt);
 	  if (id->reset_location)
-	gimple_set_location (copy, input_location);
+	gimple_set_location (copy, gimple_location (id->call_stmt));
 	  id->debug_stmts.safe_push (copy);
 	  gimple_seq_add_stmt (&stmts, copy);
 	  return stmts;
@@ -1658,7 +1658,7 @@ remap_gimple_stmt (gimple *stmt, copy_bo
 
 	  gdebug *copy = as_a  (gimple_copy (stmt));
 	  if (id->reset_location)
-	gimple_set_location (copy, input_location);
+	gimple_set_location (copy, gimple_location (id->call_stmt));
 	  id->debug_stmts.safe_push (copy);
 	  gimple_seq_add_stmt (&stmts, copy);
 	  return stmts;
@@ -1758,7 +1758,7 @@ remap_gimple_stmt (gimple *stmt, copy_bo
 }
 
   if (id->reset_location)
-gimple_set_location (copy, input_location);
+gimple_set_location (copy, gimple_location (id->call_stmt));
 
   /* Debug statements ought to be rebuilt and not copied.  */
   gcc_checking_assert (!is_gimple_debug (copy));
@@ -2386,7 +2386,7 @@ copy_phis_for_bb (basic_block bb, copy_b
 		}
 		  locus = gimple_phi_arg_location_from_edge (phi, old_edge);
 		  if (id->reset_location)
-		locus = input_location;
+		locus = gimple_location (id->call_stmt);
 		  else
 		locus = remap_location (locus, id);
 		  add_phi_arg (new_phi, new_arg, new_edge, locus);
@@ -4336,8 +4336,8 @@ expand_call_inline (basic_block bb, gimp
   gimple *simtenter_stmt = NULL;
   vec *simtvars_save;
 
-  /* The gimplifier uses input_location in too many places, such as
- internal_get_tmp_var ().  */
+  /* FIXME: the gimplifier uses input_location in too many places,
+ such as internal_get_tmp_var.  Cope with it for now.  */
   location_t saved_location = input_location;
   input_location = gimple_location (stmt);
 
@@ -4559,12 +4559,18 @@ expand_call_inline (basic_block bb, gimp
 			GSI_NEW_STMT);
 }
   initialize_inlined_parameters (id, stmt, fn, bb);
-  if (debug_nonbind_markers_p && debug_inline_points && id->block
+
+  if (debug_nonbind_markers_p
+  && debug_inline_points
+  && id->block
   && inlined_function_outer_scope_p (id->block))
 {
   gimple_stmt_iterator si = gsi_last_bb (bb);
-  gsi_insert_after (&si, gimple_build_debug_inline_entry
-			(id->block, input_location), GSI_NEW_STMT);
+  gsi_insert_after (&si,
+			gimple_build_debug_inline_entry (id->block,
+			 gimple_location
+			 (id->call_stmt)),
+			GSI_NEW_STMT);
 }
 
   if (DECL_INITIAL (fn))

Re: [patch] Do not leak location information during inlining (2nd try)

[Richard Biener]

On June 28, 2018 12:28:15 PM GMT+02:00, Eric Botcazou  
wrote:
>> More references to input_location aren't ideal.  But I don't think
>> that's a strong enough reason to reject.
>
>This can be avoided relatively easily though, patch to that effect
>attached.
>
>Tested on x86-64/Linux, OK for the mainline?

But then why not expose this as extra field in ID instead of repeatedly calling 
gimple_location? 

I don't have an issue with using input_location here until we fix the 
gimplifier... 

Richard. 

>
>   * tree-inline.c (remap_gimple_stmt): Replace input_location with
>   gimple_location (id->call_stmt) throughout.
>   (copy_phis_for_bb): Likewise.
>   (expand_call_inline): Likewise.  Tweak formatting.

Re: [patch] Remap goto_locus on edges during inlining

[Eric Botcazou]

> Related we're also missing to verify_location () on those in
> verify_gimple_in_cfg.  Having stale references to GCed BLOCKs
> via locations can be difficult to track down...
> 
> So can you add the verification bits as well?

Like this?


* tree-cfg.c (verify_gimple_in_cfg): Call verify_location on the
goto_locus of each outgoing edge of each basic block.

-- 
Eric BotcazouIndex: tree-cfg.c
===
--- tree-cfg.c	(revision 262207)
+++ tree-cfg.c	(working copy)
@@ -5286,6 +5286,8 @@ verify_gimple_in_cfg (struct function *f
   FOR_EACH_BB_FN (bb, fn)
 {
   gimple_stmt_iterator gsi;
+  edge_iterator ei;
+  edge e;
 
   for (gphi_iterator gpi = gsi_start_phis (bb);
 	   !gsi_end_p (gpi);
@@ -5407,6 +5409,10 @@ verify_gimple_in_cfg (struct function *f
 	debug_gimple_stmt (stmt);
 	  err |= err2;
 	}
+
+  FOR_EACH_EDGE (e, ei, bb->succs)
+	if (e->goto_locus != UNKNOWN_LOCATION)
+	  err |= verify_location (&blocks, e->goto_locus);
 }
 
   hash_map *eh_table = get_eh_throw_stmt_table (cfun);

Re: [patch] Do not leak location information during inlining (2nd try)

[Eric Botcazou]

> But then why not expose this as extra field in ID instead of repeatedly
> calling gimple_location?

That's a matter of taste I guess.

> I don't have an issue with using input_location here until we fix the
> gimplifier...

OK, patch dropped.

-- 
Eric Botcazou

Re: [patch, fortran] Handling of .and. and .or. expressions

[N.M. Maclaren]

On Jun 28 2018, Janus Weil wrote:



if we add a warning, we should add it both for

if (flag .and. func())
and for
if (func() .and. flag)

because the standard also allows reversing the test (which my
original test did).


well, I really don't want to warn for hypothetical problems. Since we
currently do not apply such an optimization, we should not warn about
it (unless you find any other compiler which actually does).


I have used such compilers; they used to be fairly common and may still be.
I agree with Thomas, and think it should be in -Wextra.  -Wsurprising is in
-Wall, and this is done in so many programs (usually with not-pure functions
that are actually pure, or effectively so) that the number of warnings would
put people off using -Wall.

Basically, it it is warning people about a feature of Fortran that they
might not know, can almost always be resolved by modernising the code, and
might just cause trouble in future gfortrans or some other compilers.  It is
not in the same class as the examples for -Wsurprising in the documentation.
It's far more similar to those for -Wextra.  I am looking at:

https://gcc.gnu.org/onlinedocs/gcc-5.5.0/gfortran/Error-and-Warning-Options.html

Regards,
Nick Maclaren.

Re: [PATCH, committed] Convert pdp11 back end to CCmode

[Eric Botcazou]

> This change converts the pdp11 back end to use CCmode rather than cc0.  It
> is functional and the testsuite compile sections runs at least as well as
> before.

Nice work!  You need to update the pdp11 entry in:
  https://gcc.gnu.org/backends.html

> There is additional work left to be done; for example, the compare
> elimination pass, which this target uses, does not know how to deal with a
> machine that has two condition code registers.

Right, patches lifting this limitation are most welcome.

-- 
Eric Botcazou

Re: [committed] [2/3] Converting the v850 port away from cc0 -- basic support for new scheme

[Eric Botcazou]

> This is the bulk of the changes.  With this change in installed the v850
> port should be using the new mechanism for condition code handling
> consistently, though without optimizing well.

Nice work!  You need to update the v850 entry in:
  https://gcc.gnu.org/backends.html

> One of the interesting v8 tidbits is that it was already using the new
> style for its floating point comparisons & branches.  We actually get to
> simplify that a bit because we no longer need special branching patterns
> (FP comparison initially are stored in FCC, then get copied into the Z
> bit of CC_REG, then we use standard bz/bnz insns).

Does this mean that the 'F' in the above document is also obsolete?

-- 
Eric Botcazou

Re: [PATCH 1/3][POPCOUNT] Handle COND_EXPR in expression_expensive_p

[Richard Biener]

On Wed, Jun 27, 2018 at 7:00 AM Kugan Vivekanandarajah
 wrote:
>
> Hi Richard,
>
> Thanks for the review.
>
> On 25 June 2018 at 20:01, Richard Biener  wrote:
> > On Fri, Jun 22, 2018 at 11:13 AM Kugan Vivekanandarajah
> >  wrote:
> >>
> >> [PATCH 1/3][POPCOUNT] Handle COND_EXPR in expression_expensive_p
> >
> > This says that COND_EXPR itself isn't expensive.  I think we should
> > constrain that a bit.
> > I think a good default would be to only allow a single COND_EXPR which
> > you can achieve
> > by adding a bool in_cond_expr_p = false argument to the function, pass
> > in_cond_expr_p
> > down and pass true down from the COND_EXPR handling itself.
> >
> > I'm not sure if we should require either COND_EXPR arm (operand 1 or
> > 2) to be constant
> > or !EXPR_P (then multiple COND_EXPRs might be OK).
> >
> > The main idea is to avoid evaluating many expressions but only
> > choosing one in the end.
> >
> > The simplest patch achieving that is sth like
> >
> > +  if (code == COND_EXPR)
> > +return (expression_expensive_p (TREE_OPERAND (expr, 0))
> >   || (EXPR_P (TREE_OPERAND (expr, 1)) && EXPR_P
> > (TREE_OPERAND (expr, 2)))
> > +   || expression_expensive_p (TREE_OPERAND (expr, 1))
> > +   || expression_expensive_p (TREE_OPERAND (expr, 2)));
> >
> > OK with that change.
>
> Is || (EXPR_P (TREE_OPERAND (expr, 1)) || EXPR_P (TREE_OPERAND (expr,
> 2))) slightly better ?
> Attaching  with the change. Is this OK?

Well, it won't allow x != 0 ? popcount (x) : 1 because popcount(x) is CALL_EXPR.

>
>
> Because, for pr81661.c, we now allow as not expensive
>  type  size 
> unit-size 
> align:32 warn_if_not_align:0 symtab:0 alias-set 1
> canonical-type 0x769455e8 precision:32 min  0x7692dee8 -2147483648> max  2147483647>
> pointer_to_this >
>
> arg:0 
>
> arg:0 
> visited
> def_stmt a.1_10 = a;
> version:10>
> arg:1 >
> arg:1 
>
> arg:0 
>
> arg:0  0x769455e8 int>
>
> arg:0  0x769455e8 int>
> arg:0  arg:1  0x7694a0d8 -1>>
> arg:1  0x769455e8 int>
> visited
> def_stmt c.2_11 = c;
> version:11>>
> arg:1  0x769455e8 int>
> visited
> def_stmt b.3_13 = b;
> version:13>>
> arg:1  int>
>
> arg:0  0x769455e8 int>
>
> arg:0  0x76a55b28>
>
> arg:0  0x76a55b28>
>
> arg:0  
> arg:0  arg:1
> >>
> arg:1  0x76a55b28>
> arg:0 
> arg:2 >>
>
> Which also leads to an ICE in gimplify_modify_expr. I think this is a
> latent issue and I am trying to find the source

Well, I think that's because some COND_EXPRs only gimplify to
conditional code.  See gimplify_cond_expr:

  if (gimplify_ctxp->allow_rhs_cond_expr
  /* If either branch has side effects or could trap, it can't be
 evaluated unconditionally.  */
  && !TREE_SIDE_EFFECTS (then_)
  && !generic_expr_could_trap_p (then_)
  && !TREE_SIDE_EFFECTS (else_)
  && !generic_expr_could_trap_p (else_))
return gimplify_pure_cond_expr (expr_p, pre_p);

so we probably have to treat TREE_SIDE_EFFECTS / generic_expr_could_trap_p as
"expensive" as well for the purpose of final value replacement unless we are
going to support a code-generation way different from gimplification.

The testcase you cite uses -ftrapv which is why we run into this issue.  Note
that final value replacement deals with this by rewriting the expression to
unsigned but it does so only after gimplification.  IIRC Jakub recently
added a helper to rewrite GENERIC to unsigned so that might be useful
in this context.

Richard.

> the expr in gimple_modify_expr is
>  type  size 
> unit-size 
> align:32 warn_if_not_align:0 symtab:0 alias-set 1
> canonical-type 0x769455e8 precision:32 min  0x7692dee8 -2147483648> max  2147483647>
> pointer_to_this >
> side-effects
> arg:0  0x769455e8 int>
> used ignored SI (null):0:0 size  32> unit-size 
> align:32 warn_if_not_align:0 context  foo>>
> arg:1 
>
> arg:0 
>
> arg:0  0x76a55b28>
>
> arg:0  0x76a55b28>
>
> arg:0  0x769455e8 int>
>
> arg:0  
> arg:0  arg:1
> >
> arg:1  
> visited
> def_stmt c.2_11 = c;
> version:11>>>
> arg:1  0x76a55b28>
>
> arg:0  0x769455e8 int>
> visited
> def_st

Re: [patch] Remap goto_locus on edges during inlining

[Richard Biener]

On Thu, Jun 28, 2018 at 12:46 PM Eric Botcazou  wrote:
>
> > Related we're also missing to verify_location () on those in
> > verify_gimple_in_cfg.  Having stale references to GCed BLOCKs
> > via locations can be difficult to track down...
> >
> > So can you add the verification bits as well?
>
> Like this?

Yes, OK if it (hopefully!) passes testing.

Richard.

>
> * tree-cfg.c (verify_gimple_in_cfg): Call verify_location on the
> goto_locus of each outgoing edge of each basic block.
>
> --
> Eric Botcazou

Re: [committed] Introduce dump_location_t

[Richard Biener]

On Tue, Jun 26, 2018 at 3:54 PM David Malcolm  wrote:
>
> On Mon, 2018-06-25 at 15:34 +0200, Richard Biener wrote:
> > On Wed, Jun 20, 2018 at 6:34 PM David Malcolm 
> > wrote:
> > >
> > > Here's v3 of the patch (one big patch this time, rather than a
> > > kit).
> > >
> > > Like the v2 patch kit, this patch reuses the existing dump API,
> > > rather than inventing its own.
> > >
> > > Specifically, it uses the dump_* functions in dumpfile.h that don't
> > > take a FILE *, the ones that implicitly write to dump_file and/or
> > > alt_dump_file.  I needed a name for them, so I've taken to calling
> > > them the "structured dump API" (better name ideas welcome).
> > >
> > > v3 eliminates v2's optinfo_guard class, instead using "dump_*_loc"
> > > calls as delimiters when consolidating "dump_*" calls.  There's a
> > > new dump_context class which has responsibility for consolidating
> > > them into optimization records.
> > >
> > > The dump_*_loc calls now capture more than just a location_t: they
> > > capture the profile_count and the location in GCC's own sources
> > > where
> > > the dump is being emitted from.
> > >
> > > This works by introducing a new "dump_location_t" class as the
> > > argument of those dump_*_loc calls.  The dump_location_t can
> > > be constructed from a gimple * or from an rtx_insn *, so that
> > > rather than writing:
> > >
> > >   dump_printf_loc (MSG_NOTE, gimple_location (stmt),
> > >"some message: %i", 42);
> > >
> > > you can write:
> > >
> > >   dump_printf_loc (MSG_NOTE, stmt,
> > >"some message: %i", 42);
> > >
> > > and the dump_location_t constructor will grab the location_t and
> > > profile_count of stmt, and the location of the "dump_printf_loc"
> > > callsite (and gracefully handle "stmt" being NULL).
> > >
> > > Earlier versions of the patch captured the location of the
> > > dump_*_loc call via preprocessor hacks, or didn't work properly;
> > > this version of the patch works more cleanly: internally,
> > > dump_location_t is split into two new classes:
> > >   * dump_user_location_t: the location_t and profile_count within
> > > the *user's code*, and
> > >   * dump_impl_location_t: the __builtin_FILE/LINE/FUNCTION within
> > > the *implementation* code (i.e. GCC or a plugin), captured
> > > "automagically" via default params
> > >
> > > These classes are sometimes used elsewhere in the code.  For
> > > example, "vect_location" becomes a dump_user_location_t
> > > (location_t and profile_count), so that in e.g:
> > >
> > >   vect_location = find_loop_location (loop);
> > >
> > > it's capturing the location_t and profile_count, and then when
> > > it's used here:
> > >
> > >   dump_printf_loc (MSG_NOTE, vect_location, "foo");
> > >
> > > the dump_location_t is constructed from the vect_location
> > > plus the dump_impl_location_t at that callsite.
> > >
> > > In contrast, loop-unroll.c's report_unroll's "locus" param
> > > becomes a dump_location_t: we're interested in where it was
> > > called from, not in the locations of the various dump_*_loc calls
> > > within it.
> > >
> > > Previous versions of the patch captured a gimple *, and needed
> > > GTY markers; in this patch, the dump_user_location_t is now just a
> > > location_t and a profile_count.
> > >
> > > The v2 patch added an overload for dump_printf_loc so that you
> > > could pass in either a location_t, or the new type; this version
> > > of the patch eliminates that: they all now take dump_location_t.
> > >
> > > Doing so required adding support for rtx_insn *, so that one can
> > > write this kind of thing in RTL passes:
> > >
> > >   dump_printf_loc (MSG_NOTE, insn, "foo");
> > >
> > > One knock-on effect is that get_loop_location now returns a
> > > dump_user_location_t rather than a location_t, so that it has
> > > hotness information.
> > >
> > > Richi: would you like me to split out this location-handling
> > > code into a separate patch?  (It's kind of redundant without
> > > adding the remarks and optimization records work, but if that's
> > > easier I can do it)
> >
> > I think that would be easier because it doesn't require the JSON
> > stuff and so I'll happily approve it.
> >
> > Thus - trying to review that bits (and sorry for the delay).
> >
> > +  location_t srcloc = loc.get_location_t ();
> > +
> >if (dump_file && (dump_kind & pflags))
> >  {
> > -  dump_loc (dump_kind, dump_file, loc);
> > +  dump_loc (dump_kind, dump_file, srcloc);
> >print_gimple_stmt (dump_file, gs, spc, dump_flags |
> > extra_dump_flags);
> >  }
> >
> >if (alt_dump_file && (dump_kind & alt_flags))
> >  {
> > -  dump_loc (dump_kind, alt_dump_file, loc);
> > +  dump_loc (dump_kind, alt_dump_file, srcloc);
> >print_gimple_stmt (alt_dump_file, gs, spc, dump_flags |
> > extra_dump_flags);
> >  }
> > +
> > +  if (optinfo_enabled_p ())
> > +{
> > +  optinfo &info = begin_next_optinfo (loc);
> > +  info.ha

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

2018-06-28 13:05 GMT+02:00 N.M. Maclaren :
> On Jun 28 2018, Janus Weil wrote:
>>
>>
>>> if we add a warning, we should add it both for
>>>
>>> if (flag .and. func())
>>> and for
>>> if (func() .and. flag)
>>>
>>> because the standard also allows reversing the test (which my
>>> original test did).
>>
>>
>> well, I really don't want to warn for hypothetical problems. Since we
>> currently do not apply such an optimization, we should not warn about
>> it (unless you find any other compiler which actually does).
>
>
> I have used such compilers; they used to be fairly common and may still be.

You mean compilers which transform "if (func() .and. flag)" into "if
(flag .and. func())", and then possibly remove the call?
If yes, could you tell us which compilers you are talking about specifically?


> I agree with Thomas, and think it should be in -Wextra.  -Wsurprising is in
> -Wall, and this is done in so many programs (usually with not-pure functions
> that are actually pure, or effectively so) that the number of warnings would
> put people off using -Wall.

I agree that -Wextra might be more suitable than -Wall.

Btw, effectively pure functions which are not marked with the PURE
attribute should hopefully not be a problem, since gfortran has
implicit_pure detection that should deal with that. (However, it might
need some further improvements, I think.)

Regarding warnings for "if (func() .and. flag)", I would like to have
a very concrete reason, otherwise it will not be very useful. Which
other compilers optimize this call away?

Thanks for the constructive comments!

Cheers,
Janus

Re: [PATCH, rs6000] Add missing builtin test cases

[Segher Boessenkool]

Hi!

On Wed, Jun 27, 2018 at 03:14:18PM -0700, Carl Love wrote:
> The following patch adds missing test cases for
> vec_extract_fp32_from_shortl(), vec_extract_fp32_from_shorth(), and
> vec_extract().  

This is fine for trunk.  Thank you!


Segher


> 2018-06-27  Carl Love  
> 
>   * gcc.target/p9-extract-1.c: Add test case.
>   * gcc.target/builtins-3-p9-runnable.c: Add test case to match
>   name in ABI.

Re: [patch, fortran] Handling of .and. and .or. expressions

[N.M. Maclaren]

On Jun 28 2018, Janus Weil wrote:


You mean compilers which transform "if (func() .and. flag)" into "if 
(flag .and. func())", and then possibly remove the call? If yes, could 
you tell us which compilers you are talking about specifically?


I am 70, and haven't supported multiple compilers for over a decade!
No, I can't remember.  If I recall, the most widespread optimising
compiler of the 1980s did (IBM Fortran Q), as well as several others of
that era.  I can't remember which of those I have used recently did,
because it was not something I looked at - I had known for ages that it
was likely, so followed the simple rule that ANY function call might
disappear.  Yes, even in statements like X = FRED(), because the value
of X might not be needed.


Btw, effectively pure functions which are not marked with the PURE
attribute should hopefully not be a problem, since gfortran has
implicit_pure detection that should deal with that. (However, it might
need some further improvements, I think.)


Most practical programmers use external libraries, often supplied as
binary.  The only information the compiler can rely on is whether the
function is marked as PURE.

It's moot whether such a warning should rely on implicit pure detection,
because the gotcha with doing so is the programmer modifies the function,
adds a side-effect, does NOT recompile the calling code, and can't work
out why nothing has changed!  Been there - done that :-)  Response to
self on locating it: you IDIOT, Nick!


Regards,
Nick Maclaren.

Re: [PATCH][GCC][AArch64] Add SIMD to REG pattern for movhf without armv8.2-a support (PR85769)

[Christophe Lyon]

On Tue, 26 Jun 2018 at 23:53, James Greenhalgh  wrote:
>
> On Wed, Jun 20, 2018 at 05:15:37AM -0500, Tamar Christina wrote:
> > Hi Kyrill,
> >
> > Many thanks for the review!
> >
> > The 06/19/2018 16:47, Kyrill Tkachov wrote:
> > > Hi Tamar,
> > >
> > > On 19/06/18 15:07, Tamar Christina wrote:
> > > > Hi All,
> > > >
> > > > This fixes a regression where we don't have an instruction for pre 
> > > > Armv8.2-a
> > > > to do a move of an fp16 value from a GP reg to a SIMD reg.
> > > >
> > > > This patch adds that pattern to movhf_aarch64 using a dup and only 
> > > > selectes it
> > > > using a very low priority.
> > > >
> > > > This fixes an ICE at -O0.
> > > >
> > > > Regtested on aarch64-none-elf and no issues.
> > > > Bootstrapped on aarch64-none-linux-gnu and no issues.
> > > >
> > > > Ok for master?
>
> OK,
>

Hi,

I've noticed that the new test fails:
FAIL:gcc.target/aarch64/f16_mov_immediate_3.c scan-assembler-times
dup\\tv[0-9]+.4h, w[0-9]+ 1

It might be a matter of old binutils not supporting v8.2, and maybe
your test should use an effective target to check that?

Thanks,

Christophe

> Thanks,
> James
>
> > > >
> > > > gcc/
> > > > 2018-06-19  Tamar Christina  
> > > >
> > > > PR target/85769
> > > > * config/aarch64/aarch64.md (*movhf_aarch64): Add dup v0.4h 
> > > > pattern.
> > > >
> > > > gcc/testsuite/
> > > > 2018-06-19  Tamar Christina  
> > > >
> > > > PR target/85769
> > > > * gcc.target/aarch64/f16_mov_immediate_3.c: New.
> > > > Thanks,
> > > > Tamar
> > > >
> > > > --
> >

Re: [patch, i386] false dependencies fix

[Jeff Law]

On 06/28/2018 01:16 AM, Uros Bizjak wrote:
> Hello!
> 
>>> --- i386.md (revision 259756)
>>> +++ i386.md (working copy)
>>> @@ -3547,7 +3547,7 @@
>>>   {
>>>   case MODE_DF:
>>>if (TARGET_AVX && REG_P (operands[0]) && REG_P (operands[1]))
>>> -return "vmovsd\t{%1, %0, %0|%0, %0, %1}";
>>> +return "%vmovsd\t{%d1, %0|%0, %d1}";
>>>return "%vmovsd\t{%1, %0|%0, %1}";
>>>
>>>   case MODE_V4SF:
>>> @@ -3748,7 +3748,7 @@
>>>   {
>>>   case MODE_SF:
>>>if (TARGET_AVX && REG_P (operands[0]) && REG_P (operands[1]))
>>> -return "vmovss\t{%1, %0, %0|%0, %0, %1}";
>>> +return "%vmovss\t{%d1, %0|%0, %d1}";
>>>return "%vmovss\t{%1, %0|%0, %1}";
>> So what I'm confused about is in the original output template operand 0
>> is duplicated. In the new template operand 1 is duplicated.
>>
>> Presumably what you're trying to accomplish is avoiding a false read on
>> operand 0 (the destination)?  Can you please confirm?
> 
>> Knowing that should also help me evaluate the changes to recp and rsqrt
>> since they're being changed to the same style encoding when operating
>> strictly on registers.
> 
> Please don't change "v" -> "%v" for TARGET_AVX templates. We know that
> in this case, all insn mnemonics are prefixed with "v".
ACK on that Uros -- I'd convinced myself that v->%v for TARGET_AVX
couldn't hurt anything since  we already had the v prefix in place.  I'm
happy to ensure this follows your preferred convention.

I was mostly trying to make sure I understood the other aspects of the
proposed change.

Cheers,
jeff

Re: [PATCH][Middle-end]3rd patch of PR78809

[Jeff Law]

On 06/28/2018 01:12 AM, Richard Biener wrote:
> On Wed, 27 Jun 2018, Jeff Law wrote:
> 
>>
>> On 06/18/2018 09:37 AM, Qing Zhao wrote:
>>> Hi,
>>>
>>> this is the 3rd (and the last) patch for PR78809:
>>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78809
>>> Inline strcmp with small constant strings
>>>
>>> The design doc for PR78809 is at:
>>> https://www.mail-archive.com/gcc@gcc.gnu.org/msg83822.html
>>>
>>> this patch is for the third part of change of PR78809.
>>>
>>> C. for strcmp (s1, s2), strncmp (s1, s2, n), and memcmp (s1, s2, n)
>>>if the result is NOT used to do simple equality test against zero, one of
>>> "s1" or "s2" is a small constant string, n is a constant, and the Min value 
>>> of
>>> the length of the constant string and "n" is smaller than a predefined
>>> threshold T,
>>>inline the call by a byte-to-byte comparision sequence to avoid calling
>>> overhead.
>>>
>>> adding test case strcmpopt_5.c into gcc.dg for part C of PR78809.
>>>
>>> bootstraped and tested on both X86 and Aarch64. no regression.
>>>
>>> I have done some experiments to check the run-time performance impact 
>>> and code-size impact from such inlining with different threshold for the
>>> length of the constant string to inline, the data were recorded in the 
>>> attachments of 
>>> PR78809:
>>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78809.
>>>
>>> and finally decide that the default value of the threshold is 3. 
>>> this value of the threshold can be adjusted by the new option:
>>>
>>> --param builtin-string-cmp-inline-length=
>>>
>>> The following is the patch.
>>>
>>> thanks.
>>>
>>> Qing
>>>
>>> gcc/ChangeLog:
>>>
>>> +2018-06-18  Qing Zhao  
>>> +
>>> +   PR middle-end/78809
>>> +   * builtins.c (expand_builtin_memcmp): Inline the calls first
>>> +   when result_eq is false.
>>> +   (expand_builtin_strcmp): Inline the calls first.
>>> +   (expand_builtin_strncmp): Likewise.
>>> +   (inline_string_cmp): New routine. Expand a string compare 
>>> +   call by using a sequence of char comparison.
>>> +   (inline_expand_builtin_string_cmp): New routine. Inline expansion
>>> +   a call to str(n)cmp/memcmp.
>>> +   * doc/invoke.texi (--param builtin-string-cmp-inline-length): New 
>>> option.
>>> +   * params.def (BUILTIN_STRING_CMP_INLINE_LENGTH): New.
>>> +
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> +2018-06-18  Qing Zhao  
>>> +
>>> +   PR middle-end/78809
>>> +   * gcc.dg/strcmpopt_5.c: New test.
>> So I still need to dig into this patch.  But I wanted to raise an
>> potential issue and get yours and Martin's thoughts on it.
>>
>> Martin (and others) have been working hard to improve GCC's ability to
>> give good diagnostics for various problems with calls to mem* and str*
>> functions (buffer overflows, restrict issues, etc).
>>
>> One of the problems Martin has identified is early conversion of these
>> calls into inlined direct operations.  If those conversions happen prior
>> to the analysis for warnings we obviously can't issue any relevant warnings.
> 
> From the looks of the changelog this seems to be done at RTL expansion 
> time -- which also makes me question why targets do not provide
> cmpstr[n] / cmpmem optabs when doing the inlining is so obviously
> beneficial?
> 
> Note I didn't look at the actual patch.
Some do.  And there's other code that does generic inlining of this
stuff (all the move by pieces related thingies).  Qing's patch is
handling another case where we have a bit more data on how the result it
used.

We could pass that info down to the targets, but then we'd have to
twiddle each one which would be painful.  In general I'd rather handle
most of the inlining decisions in the generic parts of the expanders
rather than in the targets themselves.

Jeff

Re: [committed] [2/3] Converting the v850 port away from cc0 -- basic support for new scheme

[Jeff Law]

On 06/28/2018 05:20 AM, Eric Botcazou wrote:
>> This is the bulk of the changes.  With this change in installed the v850
>> port should be using the new mechanism for condition code handling
>> consistently, though without optimizing well.
> 
> Nice work!  You need to update the v850 entry in:
>   https://gcc.gnu.org/backends.html
It's on  my list :-)


> 
>> One of the interesting v8 tidbits is that it was already using the new
>> style for its floating point comparisons & branches.  We actually get to
>> simplify that a bit because we no longer need special branching patterns
>> (FP comparison initially are stored in FCC, then get copied into the Z
>> bit of CC_REG, then we use standard bz/bnz insns).
> 
> Does this mean that the 'F' in the above document is also obsolete?
Yea, not sure how we got into the inconsistent state, but modern
versions of the v850 have a FP unit or at least it's an option.

jeff

Re: [committed] Introduce dump_location_t

[David Malcolm]

On Thu, 2018-06-28 at 13:29 +0200, Richard Biener wrote:
> On Tue, Jun 26, 2018 at 3:54 PM David Malcolm 
> wrote:
> > 
> > On Mon, 2018-06-25 at 15:34 +0200, Richard Biener wrote:
> > > On Wed, Jun 20, 2018 at 6:34 PM David Malcolm  > > m>
> > > wrote:
> > > > 
> > > > Here's v3 of the patch (one big patch this time, rather than a
> > > > kit).
> > > > 
> > > > Like the v2 patch kit, this patch reuses the existing dump API,
> > > > rather than inventing its own.
> > > > 
> > > > Specifically, it uses the dump_* functions in dumpfile.h that
> > > > don't
> > > > take a FILE *, the ones that implicitly write to dump_file
> > > > and/or
> > > > alt_dump_file.  I needed a name for them, so I've taken to
> > > > calling
> > > > them the "structured dump API" (better name ideas welcome).
> > > > 
> > > > v3 eliminates v2's optinfo_guard class, instead using
> > > > "dump_*_loc"
> > > > calls as delimiters when consolidating "dump_*" calls.  There's
> > > > a
> > > > new dump_context class which has responsibility for
> > > > consolidating
> > > > them into optimization records.
> > > > 
> > > > The dump_*_loc calls now capture more than just a location_t:
> > > > they
> > > > capture the profile_count and the location in GCC's own sources
> > > > where
> > > > the dump is being emitted from.
> > > > 
> > > > This works by introducing a new "dump_location_t" class as the
> > > > argument of those dump_*_loc calls.  The dump_location_t can
> > > > be constructed from a gimple * or from an rtx_insn *, so that
> > > > rather than writing:
> > > > 
> > > >   dump_printf_loc (MSG_NOTE, gimple_location (stmt),
> > > >"some message: %i", 42);
> > > > 
> > > > you can write:
> > > > 
> > > >   dump_printf_loc (MSG_NOTE, stmt,
> > > >"some message: %i", 42);
> > > > 
> > > > and the dump_location_t constructor will grab the location_t
> > > > and
> > > > profile_count of stmt, and the location of the
> > > > "dump_printf_loc"
> > > > callsite (and gracefully handle "stmt" being NULL).
> > > > 
> > > > Earlier versions of the patch captured the location of the
> > > > dump_*_loc call via preprocessor hacks, or didn't work
> > > > properly;
> > > > this version of the patch works more cleanly: internally,
> > > > dump_location_t is split into two new classes:
> > > >   * dump_user_location_t: the location_t and profile_count
> > > > within
> > > > the *user's code*, and
> > > >   * dump_impl_location_t: the __builtin_FILE/LINE/FUNCTION
> > > > within
> > > > the *implementation* code (i.e. GCC or a plugin), captured
> > > > "automagically" via default params
> > > > 
> > > > These classes are sometimes used elsewhere in the code.  For
> > > > example, "vect_location" becomes a dump_user_location_t
> > > > (location_t and profile_count), so that in e.g:
> > > > 
> > > >   vect_location = find_loop_location (loop);
> > > > 
> > > > it's capturing the location_t and profile_count, and then when
> > > > it's used here:
> > > > 
> > > >   dump_printf_loc (MSG_NOTE, vect_location, "foo");
> > > > 
> > > > the dump_location_t is constructed from the vect_location
> > > > plus the dump_impl_location_t at that callsite.
> > > > 
> > > > In contrast, loop-unroll.c's report_unroll's "locus" param
> > > > becomes a dump_location_t: we're interested in where it was
> > > > called from, not in the locations of the various dump_*_loc
> > > > calls
> > > > within it.
> > > > 
> > > > Previous versions of the patch captured a gimple *, and needed
> > > > GTY markers; in this patch, the dump_user_location_t is now
> > > > just a
> > > > location_t and a profile_count.
> > > > 
> > > > The v2 patch added an overload for dump_printf_loc so that you
> > > > could pass in either a location_t, or the new type; this
> > > > version
> > > > of the patch eliminates that: they all now take
> > > > dump_location_t.
> > > > 
> > > > Doing so required adding support for rtx_insn *, so that one
> > > > can
> > > > write this kind of thing in RTL passes:
> > > > 
> > > >   dump_printf_loc (MSG_NOTE, insn, "foo");
> > > > 
> > > > One knock-on effect is that get_loop_location now returns a
> > > > dump_user_location_t rather than a location_t, so that it has
> > > > hotness information.
> > > > 
> > > > Richi: would you like me to split out this location-handling
> > > > code into a separate patch?  (It's kind of redundant without
> > > > adding the remarks and optimization records work, but if that's
> > > > easier I can do it)
> > > 
> > > I think that would be easier because it doesn't require the JSON
> > > stuff and so I'll happily approve it.
> > > 
> > > Thus - trying to review that bits (and sorry for the delay).
> > > 
> > > +  location_t srcloc = loc.get_location_t ();
> > > +
> > >if (dump_file && (dump_kind & pflags))
> > >  {
> > > -  dump_loc (dump_kind, dump_file, loc);
> > > +  dump_loc (dump_kind, dump_file, srcloc);
> > >print_gimple_stmt (dump_file, gs, spc, dum

Re: [patch, fortran] Handling of .and. and .or. expressions

[Steve Kargl]

On Thu, Jun 28, 2018 at 07:58:22AM +0200, Janus Weil wrote:
> 2018-06-27 23:47 GMT+02:00 Steve Kargl :
> > On Wed, Jun 27, 2018 at 10:46:05PM +0200, Janus Weil wrote:
> >> 2018-06-27 21:34 GMT+02:00 Thomas Koenig :
> >> >
> >> > And we are not going to change Fortran semantics. And I also oppose
> >> > defining our own subset of Fortran in the hope that people will make
> >> > fewer mistakes.
> >> >
> >> > A function is something that produces a value.  If the value is not
> >> > needed to produce the correct result, the function need not be called.
> >>
> >> That's a bit oversimplified, isn't it?
> >
> > Technically, the standard says an operand need not be evaluate,
> > but you've asked people not to cite the Standard.  I've also
> > pointed you to F2018 Note 10.28 where it very clearly says that
> > a function need not be evaluated with example nearly identical
> > to the one in the PR.  PURE vs IMPURE (or unmarked) function
> > is a red herring.  The standard makes no distinction.
> 
> Look, Steve. This argument has been going in circles for weeks now. I
> think we can stop it here.
> 

I've already stated that I have no problem with the warning.  As
Thomas noted, gfortran should warn for both '.false. .and. check()'
and 'check() .and. .false.'

I am opposed to the change of TRUTH_AND_EXPR to TRUTH_ANDIF_EXPR,
where you are special casing logical expressions that involve
.and. and .or.  

In fact, I'll be submitting a bug report for a missed optimization.

subroutine foo(x,y)   subroutine foo(x,y)
   real x(10), y(10) real x(10), y(10)
   y = 0*sin(x)  y = 0
end subroutine fooend subroutine foo

.L2:  pxor%xmm0, %xmm0
   callsinf   movq$0, 32(%rsi)
   pxor%xmm1, %xmm1   movups  %xmm0, (%rsi)
   mulss   %xmm1, %xmm0   movups  %xmm0, 16(%rsi)
   movss   %xmm0, 0(%rbp,%rbx)
   addq$4, %rbx
   cmpq$40, %rbx
   jne .L2

which I'm sure you'll just be thrilled with.

-- 
Steve

Re: [patch] Remap goto_locus on edges during inlining

[Eric Botcazou]

> Yes, OK if it (hopefully!) passes testing.

Yes, it passed bootstrap/regtest on x86-64/Linux so was applied.

-- 
Eric Botcazou

Re: [PATCH] tighten up -Wbuiltin-declaration-mismatch (PR 86125)

[Martin Sebor]

On 06/27/2018 11:20 PM, Jeff Law wrote:

On 06/26/2018 05:32 PM, Martin Sebor wrote:

Attached is an updated patch to tighten up the warning and also
prevent ICEs in the middle-end like in PR 86308 or PR 86202.

I took Richard's suggestion to add the POINTER_TYPE_P() check
to detect pointer/integer conflicts.  That also avoids the ICEs
above.

I also dealt with the fileptr_type_node problem so that file
I/O built-ins can be declared to take any object pointer type
as an argument, and that argument has to be the same for all
them.

I'm not too happy about the interaction with -Wextra but short
of enabling the stricter checks even without it or introducing
multiple levels for -Wbuiltin-declaration-mismatch I don't see
a good alternative.

Martin

gcc-86125.diff


PR c/86125 - missing -Wbuiltin-declaration-mismatch on a mismatched return type
PR middle-end/86308 - ICE in verify_gimple calling index() with an invalid 
declaration
PR middle-end/86202 - ICE in get_range_info calling an invalid memcpy() 
declaration

gcc/c/ChangeLog:

PR c/86125
PR middle-end/86202
PR middle-end/86308
* c-decl.c (match_builtin_function_types): Add arguments.
(diagnose_mismatched_decls): Diagnose mismatched declarations
of built-ins more strictly.
* doc/invoke.texi (-Wbuiltin-declaration-mismatch): Update.

gcc/testsuite/ChangeLog:

PR c/86125
PR middle-end/86202
PR middle-end/86308
* gcc.dg/Wbuiltin-declaration-mismatch.c: New test.
* gcc.dg/Wbuiltin-declaration-mismatch-2.c: New test.
* gcc.dg/Wbuiltin-declaration-mismatch-3.c: New test.
* gcc.dg/Wbuiltin-declaration-mismatch-4.c: New test.
* gcc.dg/builtins-69.c: New test.


[ ... ]



diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index af16cfd..6c9e667 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -1628,43 +1628,82 @@ c_bind (location_t loc, tree decl, bool is_global)
   bind (DECL_NAME (decl), decl, scope, false, nested, loc);
 }

+
 /* Subroutine of compare_decls.  Allow harmless mismatches in return
and argument types provided that the type modes match.  This function
-   return a unified type given a suitable match, and 0 otherwise.  */
+   returns a unified type given a suitable match, and 0 otherwise.  */

 static tree
-match_builtin_function_types (tree newtype, tree oldtype)
+match_builtin_function_types (tree newtype, tree oldtype,
+ tree *strict, unsigned *argno)

As Joseph notes, you need to update the function comment here.

[ ... ]


+ /* Store the first FILE* argument type seen (whatever it is),

+and expect any subsequent declarations of file I/O built-ins
+to refer to it rather than to fileptr_type_node which is just
+void*.  */
+ static tree last_fileptr_type;

Is this actually safe?  Isn't the type in GC memory?  And if so, what
prevents it from being GC'd?  At the least I think you need to register
this as a GC root.  Why are we handling fileptr_types specially here to
begin with?


IIUC, garbage collection runs after front end processing (between
separate passes) so the node should not be freed while the front
end is holding on to it.  There are other examples in the FE of
similar static usage (e.g., in c-format.c).

The code detects mismatches between arguments to different file
I/O functions, as in:

  struct SomeFile;

  // okay, FILE is struct SomeFile
  int fputc (int, struct SomeFile*);

  struct OtherFile;
  int fputs (const char*, struct OtherFile*);   // warning

Martin

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

2018-06-28 16:41 GMT+02:00 Steve Kargl :
>> > Technically, the standard says an operand need not be evaluate,
>> > but you've asked people not to cite the Standard.  I've also
>> > pointed you to F2018 Note 10.28 where it very clearly says that
>> > a function need not be evaluated with example nearly identical
>> > to the one in the PR.  PURE vs IMPURE (or unmarked) function
>> > is a red herring.  The standard makes no distinction.
>>
>> Look, Steve. This argument has been going in circles for weeks now. I
>> think we can stop it here.
>>
>
> I've already stated that I have no problem with the warning.  As
> Thomas noted, gfortran should warn for both '.false. .and. check()'
> and 'check() .and. .false.'

It doesn't really help the discussion if you just re-state other
people's positions. It might help if you would actually tell use *why*
you think both cases should be checked?

gfortran's current implementation of .and. is intrinsically asymmetric
and only optimizes away the second operand if possible. My motivation
for the warning is mostly to signal compiler-dependent behavior, and I
still haven't seen a compiler that treats the case 'check() .and.
.false.' different from gfortran. Are you aware of one?


> I am opposed to the change of TRUTH_AND_EXPR to TRUTH_ANDIF_EXPR,
> where you are special casing logical expressions that involve
> .and. and .or.

It's the other way around. We currently have TRUTH_ANDIF_EXPR. And no
one really wants to change it right now. Let's move on.


> In fact, I'll be submitting a bug report for a missed optimization.
>
> subroutine foo(x,y)   subroutine foo(x,y)
>real x(10), y(10) real x(10), y(10)
>y = 0*sin(x)  y = 0
> end subroutine fooend subroutine foo
>
> .L2:  pxor%xmm0, %xmm0
>callsinf   movq$0, 32(%rsi)
>pxor%xmm1, %xmm1   movups  %xmm0, (%rsi)
>mulss   %xmm1, %xmm0   movups  %xmm0, 16(%rsi)
>movss   %xmm0, 0(%rbp,%rbx)
>addq$4, %rbx
>cmpq$40, %rbx
>jne .L2
>
> which I'm sure you'll just be thrilled with.

I can't say I'm totally thrilled with it, but, yes, I do agree it's a
missed optimization. That probably comes as a surprise to you, since
you are apparently trying to tease me in some way here (didn't work).
After all, SIN is an elemental function, thus pure and without any
side effects. The call can certainly be removed if the value is not
needed. Please submit your bug report, but please don't put me in CC.

Thanks,
Janus

[COMMITTED][testsuite] Fix f16_mov_immediate_3.c

[Wilco Dijkstra]

Fix and simplify the testcase so it generates dup even on latest trunk.

This fixes the failure reported in: 
https://gcc.gnu.org/ml/gcc-patches/2018-06/msg01799.html

Committed as obvious.

ChangeLog:
2018-06-28  Wilco Dijkstra  

* gcc.target/aarch64/f16_mov_immediate_3.c: Fix testcase.
--
diff --git a/gcc/testsuite/gcc.target/aarch64/f16_mov_immediate_3.c 
b/gcc/testsuite/gcc.target/aarch64/f16_mov_immediate_3.c
index 
6dc325b10b76b2a04f7081f892ce175622eaf49d..66218e3eaac95d858245d28d0c370760012d30d8
 100644
--- a/gcc/testsuite/gcc.target/aarch64/f16_mov_immediate_3.c
+++ b/gcc/testsuite/gcc.target/aarch64/f16_mov_immediate_3.c
@@ -1,18 +1,11 @@
 /* { dg-do compile } */
-/* { dg-options "-O0" } */
-
-extern __fp16 foo ();
+/* { dg-options "-O2" } */
 
 __fp16 f4 ()
 {
-  __fp16 a = 0;
-  __fp16 b = 1;
-  __fp16 c = 2;
-  __fp16 d = 4;
+  __fp16 a = 0.1;
 
-  __fp16 z = a + b;
-  z = z + c;
-  z = z - d;
+  __fp16 z = a * a;
   return z;
 }
 

Re: [patch, fortran] Handling of .and. and .or. expressions

[Steve Kargl]

On Thu, Jun 28, 2018 at 05:52:43PM +0200, Janus Weil wrote:
> 2018-06-28 16:41 GMT+02:00 Steve Kargl :
> >> > Technically, the standard says an operand need not be evaluate,
> >> > but you've asked people not to cite the Standard.  I've also
> >> > pointed you to F2018 Note 10.28 where it very clearly says that
> >> > a function need not be evaluated with example nearly identical
> >> > to the one in the PR.  PURE vs IMPURE (or unmarked) function
> >> > is a red herring.  The standard makes no distinction.
> >>
> >> Look, Steve. This argument has been going in circles for weeks now. I
> >> think we can stop it here.
> >>
> >
> > I've already stated that I have no problem with the warning.  As
> > Thomas noted, gfortran should warn for both '.false. .and. check()'
> > and 'check() .and. .false.'
> 
> It doesn't really help the discussion if you just re-state other
> people's positions. It might help if you would actually tell use *why*
> you think both cases should be checked?
> 
> gfortran's current implementation of .and. is intrinsically asymmetric
> and only optimizes away the second operand if possible. My motivation
> for the warning is mostly to signal compiler-dependent behavior, and I
> still haven't seen a compiler that treats the case 'check() .and.
> .false.' different from gfortran. Are you aware of one?

Why I think it a warning should be emitted:  symmetry!.

You complained about the lack of symmetry in 'check() .and. .false.'
and '.false. .and. check()'.

For the record, I also think Thomas's original patch that actually
switch the operands should be applied.

> > In fact, I'll be submitting a bug report for a missed optimization.
> >
> > subroutine foo(x,y)   subroutine foo(x,y)
> >real x(10), y(10) real x(10), y(10)
> >y = 0*sin(x)  y = 0
> > end subroutine fooend subroutine foo
> >
> > .L2:  pxor%xmm0, %xmm0
> >callsinf   movq$0, 32(%rsi)
> >pxor%xmm1, %xmm1   movups  %xmm0, (%rsi)
> >mulss   %xmm1, %xmm0   movups  %xmm0, 16(%rsi)
> >movss   %xmm0, 0(%rbp,%rbx)
> >addq$4, %rbx
> >cmpq$40, %rbx
> >jne .L2
> >
> > which I'm sure you'll just be thrilled with.
> 
> I can't say I'm totally thrilled with it, but, yes, I do agree it's a
> missed optimization. That probably comes as a surprise to you, since
> you are apparently trying to tease me in some way here (didn't work).
> After all, SIN is an elemental function, thus pure and without any
> side effects. The call can certainly be removed if the value is not
> needed. Please submit your bug report, but please don't put me in CC.

Change sin(x) to my_function_with_side_effects() if like.  It
is a missed optimization regardless of the function's pureness.

You continue to miss my point or conveniently ignore it.
You want to special case the forced evaluation of the
operands in two specific logical expressions; namely, .and.
and .or.  If you want to force evaluation of operands, then
do it for all binary operators. 

-- 
Steve

Re: [PATCH][2/3] Share dataref and dependence analysis for multi-vector size vectorization

[Christophe Lyon]

On Fri, 22 Jun 2018 at 12:52, Richard Biener  wrote:
>
>
> This is the main part to make considering multiple vector sizes based on
> costs less compile-time costly.  It shares dataref analysis and
> dependence analysis for loop vectorization (BB vectorization is only
> adjusted to comply with the new APIs sofar).
>
> Sharing means that DRs (and of course DDRs) may not be modified during
> vectorization analysis which means splitting out dataref_aux from
> dangling from dr->aux to separate copies in the DR_STMTs vinfo.
> I've put measures in place that assure that DRs are not modified
> (because they were) but refrained from doing the same for DDRs
> (because I didn't run into any issues).
>
> The sharing then is accomplished by moving the dataref and
> ddr array as well as the dependent loop-nest array into a
> separate structure with bigger lifetime than vinfo and appropriately
> link to it from there.
>
> Bootstrapped on x86_64-unknown-linux-gnu (together with [1/3]),
> testing in progress.
>

Hi Richard,

This you committed this patch (r262009) I've noticed a regression on
aarch64 and arm,
and I think it's also present on x86 according to gcc-testresults:
FAIL:gcc.dg/params/blocksort-part.c -O3 --param
loop-max-datarefs-for-datadeps=0 (test for excess errors)
FAIL:gcc.dg/params/blocksort-part.c -O3 --param
loop-max-datarefs-for-datadeps=0 (internal compiler error)

gcc.log says:

during GIMPLE pass: vect
/gcc/testsuite/gcc.dg/params/blocksort-part.c: In function 'fallbackQSort3':
/gcc/testsuite/gcc.dg/params/blocksort-part.c:116:6: internal compiler
error: Segmentation fault
0xbef215 crash_signal
/gcc/toplev.c:324
0x14715cc gimple_uid
/gcc/tree-vectorizer.h:1043
0x14715cc vinfo_for_stmt
/gcc/tree-vectorizer.h:1043
0x14715cc vect_dr_stmt
/gcc/tree-vectorizer.h:1331
0x14715cc vect_analyze_data_ref_dependence
/gcc/tree-vect-data-refs.c:297
0x14715cc vect_analyze_data_ref_dependences(_loop_vec_info*, unsigned int*)
/gcc/tree-vect-data-refs.c:593
0xecabb8 vect_analyze_loop_2
/gcc/tree-vect-loop.c:1910
0xecc70c vect_analyze_loop(loop*, _loop_vec_info*, vec_info_shared*)
/gcc/tree-vect-loop.c:2337
0xeec188 try_vectorize_loop_1
/gcc/tree-vectorizer.c:705
0xeed512 vectorize_loops()
/gcc/tree-vectorizer.c:918

Thanks,

Christophe



> Richard.
>
> From 2ef07d775e4eef1b52a77cb9ecd65c37e9f73d95 Mon Sep 17 00:00:00 2001
> From: Richard Guenther 
> Date: Mon, 18 Jun 2018 15:21:22 +0200
> Subject: [PATCH] share-dr-and-ddr-analysis
>
> 2018-06-22  Richard Biener  
>
> * tree-vectorizer.h (struct vec_info_shared): New structure
> with parts split out from struct vec_info and loop_nest from
> struct _loop_vec_info.
> (struct vec_info): Adjust accordingly.
> (struct _loop_vec_info): Likewise.
> (LOOP_VINFO_LOOP_NEST): Adjust.
> (LOOP_VINFO_DATAREFS): Likewise.
> (LOOP_VINFO_DDRS): Likewise.
> (struct _bb_vec_info): Likewise.
> (BB_VINFO_DATAREFS): Likewise.
> (BB_VINFO_DDRS): Likewise.
> (struct _stmt_vec_info): Add dr_aux member.
> (DR_VECT_AUX): Adjust to refer to member of DR_STMTs vinfo.
> (DR_MISALIGNMENT_UNINITIALIZED): New.
> (set_dr_misalignment): Adjust.
> (dr_misalignment): Assert misalign isn't 
> DR_MISALIGNMENT_UNINITIALIZED.
> (vect_analyze_loop): Adjust prototype.
> (vect_analyze_loop_form): Likewise.
> * tree-vect-data-refs.c (vect_analyze_data_ref_dependences):
> Compute dependences lazily.
> (vect_record_base_alignments): Use shared datarefs/ddrs.
> (vect_verify_datarefs_alignment): Likewise.
> (vect_analyze_data_refs_alignment): Likewise.
> (vect_analyze_data_ref_accesses): Likewise.
> (vect_analyze_data_refs): Likewise.
> * tree-vect-loop.c (_loop_vec_info::_loop_vec_info): Add
> constructor parameter for shared part.
> (vect_analyze_loop_form): Pass in shared part and adjust.
> (vect_analyze_loop_2): Pass in storage for the number of
> stmts.  Move loop nest finding to the caller.  Compute
> datarefs lazily.
> (vect_analyze_loop): Pass in shared part.
> (vect_transform_loop): Verify shared datarefs are unchanged.
> * tree-vect-slp.c (_bb_vec_info::_bb_vec_info): Add
> constructor parameter for shared part.
> (vect_slp_analyze_bb_1): Pass in shared part and adjust.
> (vect_slp_bb): Verify shared datarefs are unchanged before
> transform.
> * tree-vect-stmts.c (ensure_base_align): Adjust for DR_AUX
> change.
> (new_stmt_vec_info): Initialize DR_AUX misalignment to
> DR_MISALIGNMENT_UNINITIALIZED.
> * tree-vectorizer.c (vec_info::vec_info): Add constructor
> parameter for shared part.
> (vec_info::~vec_info): Adjust.
> (vec_info_shared::vec_info_shared): New.

Re: [patch, fortran] Handling of .and. and .or. expressions

[Jakub Jelinek]

On Thu, Jun 28, 2018 at 07:41:30AM -0700, Steve Kargl wrote:
> I am opposed to the change of TRUTH_AND_EXPR to TRUTH_ANDIF_EXPR,
> where you are special casing logical expressions that involve
> .and. and .or.  

I think using TRUTH_AND_EXPR is the right thing to do, and if
fortran functions can be optimized away if their results aren't used,
then let's add some new attribute like const or pure attributes that
would allow the middle-end to optimize away calls to functions with
that attribute if the lhs isn't needed.
This attribute has been discussed recently in the thread about caching
functions.

The question is what exactly should this attribute allow, if just DCE if lhs
is unused, or also CSE, if the function has been called with the same
arguments earlier, if it can be optimized into copying the result of the
earlier call.

> In fact, I'll be submitting a bug report for a missed optimization.
> 
> subroutine foo(x,y)   subroutine foo(x,y)
>real x(10), y(10) real x(10), y(10)
>y = 0*sin(x)  y = 0
> end subroutine fooend subroutine foo
> 
> .L2:  pxor%xmm0, %xmm0
>callsinf   movq$0, 32(%rsi)
>pxor%xmm1, %xmm1   movups  %xmm0, (%rsi)
>mulss   %xmm1, %xmm0   movups  %xmm0, 16(%rsi)
>movss   %xmm0, 0(%rbp,%rbx)
>addq$4, %rbx
>cmpq$40, %rbx
>jne .L2
> 
> which I'm sure you'll just be thrilled with.

For floating point, you generally need -ffast-math to be able to
optimize such computations away, sinf already has const attribute (at least
in C/C++), so with -ffast-math it is optimized.

Jakub

Re: [patch, fortran] Handling of .and. and .or. expressions

[N.M. Maclaren]

On Jun 28 2018, Steve Kargl wrote:


You continue to miss my point or conveniently ignore it.
You want to special case the forced evaluation of the
operands in two specific logical expressions; namely, .and.
and .or.  If you want to force evaluation of operands, then
do it for all binary operators. 


It's not just binary operators.  It also includes code like:
   IF (FRED()) CONTINUE
and even:
   X = FRED()
   X = 0.0
There are a zillion other possible ways in which this can arise;
I have certainly seen those optimised out, as well as the constructs
being discussed.

I have no idea how many are currently optimised out by gfortran,
still less how many may be in the future.  Janus would like to know
of compilers that do this sort of thing - the place to look is the
ones sold by the supercomputing companies, like Cray, IBM, Hitachi
and Fujitsu. I don't know how many others still maintain their own
compilers.  I am retired and no longer have access to any of them.

Having a warning for known confusing cases (like .AND.) but not
others is fine.  Lots of warnings are like that.

Having an option that makes a couple of constructs in the language
behave in a way not required by the standard, but not others with
similar properties, merely adds a new gotcha.


Regards,
Nick Maclaren.

Re: [patch, fortran] Handling of .and. and .or. expressions

[Janus Weil]

2018-06-28 18:22 GMT+02:00 Steve Kargl :
> On Thu, Jun 28, 2018 at 05:52:43PM +0200, Janus Weil wrote:
>> 2018-06-28 16:41 GMT+02:00 Steve Kargl :
>> >> > Technically, the standard says an operand need not be evaluate,
>> >> > but you've asked people not to cite the Standard.  I've also
>> >> > pointed you to F2018 Note 10.28 where it very clearly says that
>> >> > a function need not be evaluated with example nearly identical
>> >> > to the one in the PR.  PURE vs IMPURE (or unmarked) function
>> >> > is a red herring.  The standard makes no distinction.
>> >>
>> >> Look, Steve. This argument has been going in circles for weeks now. I
>> >> think we can stop it here.
>> >>
>> >
>> > I've already stated that I have no problem with the warning.  As
>> > Thomas noted, gfortran should warn for both '.false. .and. check()'
>> > and 'check() .and. .false.'
>>
>> It doesn't really help the discussion if you just re-state other
>> people's positions. It might help if you would actually tell use *why*
>> you think both cases should be checked?
>>
>> gfortran's current implementation of .and. is intrinsically asymmetric
>> and only optimizes away the second operand if possible. My motivation
>> for the warning is mostly to signal compiler-dependent behavior, and I
>> still haven't seen a compiler that treats the case 'check() .and.
>> .false.' different from gfortran. Are you aware of one?
>
> Why I think it a warning should be emitted:  symmetry!.
>
> You complained about the lack of symmetry in 'check() .and. .false.'
> and '.false. .and. check()'.

well, my original complaint in PR85599 was that the second one is
broken, and your reaction to that is to break the first one as well.


>> > In fact, I'll be submitting a bug report for a missed optimization.
>> >
>> > subroutine foo(x,y)   subroutine foo(x,y)
>> >real x(10), y(10) real x(10), y(10)
>> >y = 0*sin(x)  y = 0
>> > end subroutine fooend subroutine foo
>> >
>> > .L2:  pxor%xmm0, %xmm0
>> >callsinf   movq$0, 32(%rsi)
>> >pxor%xmm1, %xmm1   movups  %xmm0, (%rsi)
>> >mulss   %xmm1, %xmm0   movups  %xmm0, 16(%rsi)
>> >movss   %xmm0, 0(%rbp,%rbx)
>> >addq$4, %rbx
>> >cmpq$40, %rbx
>> >jne .L2
>> >
>> > which I'm sure you'll just be thrilled with.
>>
>> I can't say I'm totally thrilled with it, but, yes, I do agree it's a
>> missed optimization. That probably comes as a surprise to you, since
>> you are apparently trying to tease me in some way here (didn't work).
>> After all, SIN is an elemental function, thus pure and without any
>> side effects. The call can certainly be removed if the value is not
>> needed. Please submit your bug report, but please don't put me in CC.
>
> Change sin(x) to my_function_with_side_effects() if like.  It
> is a missed optimization regardless of the function's pureness.

Yes, if you're an orthodox believer in The One True Standard. I'd
rather use my own brain cells now and then.

In this case my brain just tells me that it's not a good idea to apply
an optimization that can totally change the results of my code, and
that it's not a good idea for a 'standard' to not define the semantics
of an operator / expression, but instead leave it to the compiler how
it should be evaluated.

You insist that the standard does not forbid this optimization. The
standard also does not forbid explicitly that you shoot yourself it
the foot with a nuclear missile. So, you go ahead and shoot. Then
someone comes along and asks why you did that, and you reply: "The
standard did not forbid it."

One thing that I always failed to comprehend is people's fixation on
optimization. What's so great about your code running 0.1% faster if
the second compiler you try gives you totally different results, with
no hints whether it's your code that's broken, or the first compiler,
or the second one, or the standard that both compilers tried to
implement? With a ten-line code that might not be such a big problem,
but above 100.000 loc or so it can quickly become a huge issue.

Cheers,
Janus

Re: [patch, fortran] Handling of .and. and .or. expressions

[Thomas Koenig]

Am 28.06.2018 um 19:21 schrieb Janus Weil:

One thing that I always failed to comprehend is people's fixation on
optimization. What's so great about your code running 0.1% faster if
the second compiler you try gives you totally different results, with
no hints


The warning added for my patch is supposed to be a hint :-)

Re: [patch, fortran] Handling of .and. and .or. expressions

[Steve Kargl]

On Thu, Jun 28, 2018 at 07:03:05PM +0200, Jakub Jelinek wrote:
> > In fact, I'll be submitting a bug report for a missed optimization.
> > 
> > subroutine foo(x,y)   subroutine foo(x,y)
> >real x(10), y(10) real x(10), y(10)
> >y = 0*sin(x)  y = 0
> > end subroutine fooend subroutine foo
> > 
> > .L2:  pxor%xmm0, %xmm0
> >callsinf   movq$0, 32(%rsi)
> >pxor%xmm1, %xmm1   movups  %xmm0, (%rsi)
> >mulss   %xmm1, %xmm0   movups  %xmm0, 16(%rsi)
> >movss   %xmm0, 0(%rbp,%rbx)
> >addq$4, %rbx
> >cmpq$40, %rbx
> >jne .L2
> > 
> > which I'm sure you'll just be thrilled with.
> 
> For floating point, you generally need -ffast-math to be able to
> optimize such computations away, sinf already has const attribute (at least
> in C/C++), so with -ffast-math it is optimized.

Doesn't -ffast-math allow the violaton of the integrity
of parentheses?  That is, it allows more optimizations
that violate the standard.

-- 
Steve

Re: [patch, fortran] Handling of .and. and .or. expressions

[Steve Kargl]

On Thu, Jun 28, 2018 at 07:21:21PM +0200, Janus Weil wrote:
> 2018-06-28 18:22 GMT+02:00 Steve Kargl :
> >
> > Why I think it a warning should be emitted:  symmetry!.
> >
> > You complained about the lack of symmetry in 'check() .and. .false.'
> > and '.false. .and. check()'.
> 
> well, my original complaint in PR85599 was that the second one is
> broken, and your reaction to that is to break the first one as well.
> 

Rose colored glasses.  You say 'break'.  I say 'fix'.

Fortunately, I'm done with this discussion.  Do what you want.

-- 
Steve

Go patch committed: Use build_variant_type_copy, not build_distinct_type_copy

[Ian Lance Taylor]

This patch to the Go frontend changes set_placeholder_struct_type to
use build_variant_type_copy rather than build_distinct_type_copy.
This is for PR 86343, which noted that the DECL_CONTEXT of the fields
in the copy were pointing to the wrong type.  Using
build_variant_type_copy puts the types in right relationship for the
DECL_CONTEXT setting.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian

2018-06-28  Ian Lance Taylor  

PR go/86343
* go-gcc.cc (Gcc_backend::set_placeholder_struct_type): Call
build_variant_type_copy rather than build_distinct_type_copy.
Index: go-gcc.cc
===
--- go-gcc.cc   (revision 262150)
+++ go-gcc.cc   (working copy)
@@ -1100,7 +1100,7 @@ Gcc_backend::set_placeholder_struct_type
   if (TYPE_NAME(t) != NULL_TREE)
 {
   // Build the data structure gcc wants to see for a typedef.
-  tree copy = build_distinct_type_copy(t);
+  tree copy = build_variant_type_copy(t);
   TYPE_NAME(copy) = NULL_TREE;
   DECL_ORIGINAL_TYPE(TYPE_NAME(t)) = copy;
 }

Re: [patch, fortran] Handling of .and. and .or. expressions

[Toon Moene]

On 06/28/2018 06:22 PM, Steve Kargl wrote:


You continue to miss my point or conveniently ignore it.
You want to special case the forced evaluation of the
operands in two specific logical expressions; namely, .and.
and .or.  If you want to force evaluation of operands, then
do it for all binary operators.


And - most interesting - that's how Fortran 77 formulated it (way before 
PURE/IMPURE functions entered the language):


"6.6.1 Evaluation of Operands

It is not necessary for a processor to evaluate all of the operands of 
an expression if the value of the expression can be determined 
otherwise. This principle is most often applicable to logical 
expressions, but it applies to all expressions. For example, in 
evaluating the logical expression

   X .GT. Y .OR. L(Z)
where X, Y, and Z are real, and L is a logical function, the function 
reference L(Z) need not be evaluated if X is greater than Y. If a 
statement contains a function reference in a part of an expression that 
need not be evaluated, all entities that would have become defined in 
the execution of that reference become undefined at the completion of
evaluation of the expression containing the function reference. In the 
example above, evaluation of the expression causes Z to become undefined 
if L defines its argument."


Kind regards,

--
Toon Moene - e-mail: t...@moene.org - phone: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands
At home: http://moene.org/~toon/; weather: http://moene.org/~hirlam/
Progress of GNU Fortran: http://gcc.gnu.org/wiki/GFortran#news

RE: [PATCH][GCC][AArch64] Add SIMD to REG pattern for movhf without armv8.2-a support (PR85769)

[Tamar Christina]

Hi Christophe,

> -Original Message-
> From: Christophe Lyon 
> Sent: Thursday, June 28, 2018 14:41
> To: James Greenhalgh 
> Cc: Tamar Christina ; Kyrill Tkachov
> ; gcc Patches ;
> nd ; Richard Earnshaw ;
> Marcus Shawcroft 
> Subject: Re: [PATCH][GCC][AArch64] Add SIMD to REG pattern for movhf
> without armv8.2-a support (PR85769)
> 
> On Tue, 26 Jun 2018 at 23:53, James Greenhalgh
>  wrote:
> >
> > On Wed, Jun 20, 2018 at 05:15:37AM -0500, Tamar Christina wrote:
> > > Hi Kyrill,
> > >
> > > Many thanks for the review!
> > >
> > > The 06/19/2018 16:47, Kyrill Tkachov wrote:
> > > > Hi Tamar,
> > > >
> > > > On 19/06/18 15:07, Tamar Christina wrote:
> > > > > Hi All,
> > > > >
> > > > > This fixes a regression where we don't have an instruction for
> > > > > pre Armv8.2-a to do a move of an fp16 value from a GP reg to a SIMD
> reg.
> > > > >
> > > > > This patch adds that pattern to movhf_aarch64 using a dup and
> > > > > only selectes it using a very low priority.
> > > > >
> > > > > This fixes an ICE at -O0.
> > > > >
> > > > > Regtested on aarch64-none-elf and no issues.
> > > > > Bootstrapped on aarch64-none-linux-gnu and no issues.
> > > > >
> > > > > Ok for master?
> >
> > OK,
> >
> 
> Hi,
> 
> I've noticed that the new test fails:
> FAIL:gcc.target/aarch64/f16_mov_immediate_3.c scan-assembler-times
> dup\\tv[0-9]+.4h, w[0-9]+ 1
> 
> It might be a matter of old binutils not supporting v8.2, and maybe your test
> should use an effective target to check that?

No, it was that Wilco had an unrelated patch that changed/fixed the costing in 
the register
allocator.  Due to it the target started generating load/stores again and so 
the constants
I was testing being so simple it didn't generate a dup anymore.

When I applied the patch on trunk it changed the behaviour. Wilco has fixed the 
test already
by making the constant more complex.

Regards,
Tamar

> 
> Thanks,
> 
> Christophe
> 
> > Thanks,
> > James
> >
> > > > >
> > > > > gcc/
> > > > > 2018-06-19  Tamar Christina  
> > > > >
> > > > > PR target/85769
> > > > > * config/aarch64/aarch64.md (*movhf_aarch64): Add dup v0.4h
> pattern.
> > > > >
> > > > > gcc/testsuite/
> > > > > 2018-06-19  Tamar Christina  
> > > > >
> > > > > PR target/85769
> > > > > * gcc.target/aarch64/f16_mov_immediate_3.c: New.
> > > > > Thanks,
> > > > > Tamar
> > > > >
> > > > > --
> > >

[PATCH] C++: less verbose error-recovery for version conflict markers

[David Malcolm]

We handle version conflict markers in source:

$ cat /tmp/test.cc

extern void f1 (void);
extern void f2 (void);
extern void f3 (void);
extern void f4 (void);

void test ()
{
  f1 ();
<<< HEAD
  f2 ();
===
  f3 ();
>>> 252be53... Some commit message
  f4 ();
}

The C frontend's output is mostly reasonable:

gcc -xc /tmp/test.cc
/tmp/test.cc: In function ‘test’:
/tmp/test.cc:9:1: error: version control conflict marker in file
 <<< HEAD
 ^~~
/tmp/test.cc:11:1: error: version control conflict marker in file
 ===
 ^~~
/tmp/test.cc:13:1: error: version control conflict marker in file
 >>> 252be53... Some commit message
 ^~~
/tmp/test.cc:13:9: error: invalid suffix "be53..." on integer constant
 >>> 252be53... Some commit message
 ^~

whereas in C++ the output can be very verbose:

/tmp/test.cc: In function ‘void test()’:
/tmp/test.cc:9:1: error: version control conflict marker in file
 <<< HEAD
 ^~~
/tmp/test.cc:9:3: error: expected primary-expression before ‘<<’ token
 <<< HEAD
   ^~
/tmp/test.cc:9:5: error: expected primary-expression before ‘<<’ token
 <<< HEAD
 ^~
/tmp/test.cc:9:7: error: expected primary-expression before ‘<’ token
 <<< HEAD
   ^
/tmp/test.cc:9:9: error: ‘HEAD’ was not declared in this scope
 <<< HEAD
 ^~~~
/tmp/test.cc:11:1: error: version control conflict marker in file
 ===
 ^~~
/tmp/test.cc:11:3: error: expected primary-expression before ‘==’ token
 ===
   ^~
/tmp/test.cc:11:5: error: expected primary-expression before ‘==’ token
 ===
 ^~
/tmp/test.cc:11:7: error: expected primary-expression before ‘=’ token
 ===
   ^
/tmp/test.cc:13:1: error: version control conflict marker in file
 >>> 252be53... Some commit message
 ^~~
/tmp/test.cc:13:3: error: expected primary-expression before ‘>>’ token
 >>> 252be53... Some commit message
   ^~
/tmp/test.cc:13:5: error: expected primary-expression before ‘>>’ token
 >>> 252be53... Some commit message
 ^~
/tmp/test.cc:13:7: error: expected primary-expression before ‘>’ token
 >>> 252be53... Some commit message
   ^
/tmp/test.cc:13:9: error: unable to find numeric literal operator 
‘operator""be53...’
 >>> 252be53... Some commit message
 ^~

The following patch eliminates this spew by consuming tokens until the
start of the next line after emitting such a message.

The C frontend and C++ with -std=c++98 both emit:
  error: invalid suffix "be53..." on integer constant
on the above testcase, but it's not fixable through this approach,
since that error is emitted by the lexer.


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

OK for trunk?

gcc/cp/ChangeLog:
* parser.c (cp_parser_error_1): After issuing a conflict marker
error, consume tokens until the end of the source line.

gcc/testsuite/ChangeLog:
* g++.dg/conflict-markers-2.C: New test.
---
 gcc/cp/parser.c   | 14 ++
 gcc/testsuite/g++.dg/conflict-markers-2.C | 17 +
 2 files changed, 31 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/conflict-markers-2.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 154729c..09d224f 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -2862,6 +2862,20 @@ cp_parser_error_1 (cp_parser* parser, const char* gmsgid,
   if (cp_lexer_peek_conflict_marker (parser->lexer, token->type, &loc))
{
  error_at (loc, "version control conflict marker in file");
+ expanded_location token_exploc = expand_location (token->location);
+ /* Consume tokens until the end of the source line.  */
+ while (1)
+   {
+ cp_lexer_consume_token (parser->lexer);
+ cp_token *next = cp_lexer_peek_token (parser->lexer);
+ if (next == NULL)
+   break;
+ expanded_location next_exploc = expand_location (next->location);
+ if (next_exploc.file != token_exploc.file)
+   break;
+ if (next_exploc.line != token_exploc.line)
+   break;
+   }
  return;
}
 }
diff --git a/gcc/testsuite/g++.dg/conflict-markers-2.C 
b/gcc/testsuite/g++.dg/conflict-markers-2.C
new file mode 100644
index 000..4fc3820
--- /dev/null
+++ b/gcc/testsuite/g++.dg/conflict-markers-2.C
@@ -0,0 +1,17 @@
+// { dg-do compile { target c++11 } }
+
+extern void f1 (void);
+extern void f2 (void);
+extern void f3 (void);
+extern void f4 (void);
+
+void test ()
+{
+  f1 ();
+<<< HEAD // { dg-error "conflict marker" }
+  f2 ();
+=== // { dg-error "conflict marker" }
+  f3 ();
+>>> 252be53... Some commit message // { dg-error "conflict marker" }
+  f4 ();
+}
-- 
1.8.5.3

[gomp5] Add #pragma omp depobj support

[Jakub Jelinek]

Hi!

This patch adds #pragma omp depobj support, so that the task dependencies
can be built at one spot and used later on from the omp_depend_t structure
that has all the necessary information (object address and dependence type)
later on.

In addition this fixes some problems in the depend clause handling, e.g.
handles references correctly, or e.g. C++ x ? y : z lvalues, --x lvalues
etc.  Mutexinoutset is also propagated to the library, which right now
still handles it conservatively as inout/out.

Tested on x86_64-linux, committed to gomp-5_0-branch.

2018-06-28  Jakub Jelinek  

* tree-core.h (enum omp_clause_depend_kind): Add
OMP_CLAUSE_DEPEND_UNSPECIFIED.
* gimplify.c (gimplify_omp_depend): If there are any
OMP_CLAUSE_DEPEND_UNSPECIFIED or OMP_CLAUSE_DEPEND_MUTEXINOUTSET
depend clauses, use a new array format.
* omp-low.c (lower_depend_clauses): Likewise.
* tree-pretty-print.c (dump_omp_clause): Handle
OMP_CLAUSE_DEPEND_UNSPECIFIED.
gcc/c-family/
* c-pragma.h (enum pragma_kind): Add PRAGMA_OMP_DEPOBJ.
* c-pragma.c (omp_pragmas): Likewise.
* c-common.h (c_omp_depend_t_p, c_finish_omp_depobj): Declare.
* c-omp.c (c_omp_depend_t_p, c_finish_omp_depobj): New functions.
gcc/c/
* c-parser.c (c_parser_omp_depobj): New function.
(c_parser_pragma): Handle PRAGMA_OMP_DEPOBJ.
(c_parser_omp_iterators): Return error_mark_node instead of NULL.
(c_parser_omp_clause_depend): Make dependence-type optional.
* c-typeck.c (c_finish_omp_clauses): Handle depend clause with
OMP_CLAUSE_DEPEND_UNSPECIFIED.  Diagnose bit-fields.  Require
omp_depend_t type for OMP_CLAUSE_DEPEND_UNSPECIFIED kinds and
some different type for other kinds.  Use build_unary_op with
ADDR_EXPR and build_indirect_ref instead of c_mark_addressable.
gcc/cp/
* parser.c (cp_parser_omp_var_list_no_open): Fix up depend clause
error recovery.
(cp_parser_omp_iterators): Return error_mark_node instead of NULL.
(cp_parser_omp_clause_depend): Make dependence-type optional.
(cp_parser_omp_depobj): New function.
(cp_parser_pragma): Handle PRAGMA_OMP_DEPOBJ.
* cp-tree.h (OMP_DEPOBJ_DEPOBJ, OMP_DEPOBJ_CLAUSES): Define.
(finish_omp_depobj): Declare.
* cp-tree.def (OMP_DEPOBJ): New tree code.
* semantics.c (finish_omp_clauses): Handle depend clause with
OMP_CLAUSE_DEPEND_UNSPECIFIED.  Diagnose bit-fields.  Require
omp_depend_t type for OMP_CLAUSE_DEPEND_UNSPECIFIED kinds and
some different type for other kinds.  Use cp_build_addr_expr
and cp_build_indirect_ref instead of cxx_mark_addressable.
(finish_omp_depobj): New function.
* pt.c (tsubst_expr): Handle OMP_DEPOBJ.
* cp-objcp-common.c (cp_common_init_ts): Likewise.
* constexpr.c (potential_constant_expression_1): Likewise.
* lex.c (cxx_init): Likewise.
* dump.c (cp_dump_tree): Likewise.
* cxx-pretty-print.c (cxx_pretty_printer::statement): Likewise.
gcc/testsuite/
* c-c++-common/gomp/depend-6.c: Add test for bit-field in depend
clause.
* c-c++-common/gomp/depend-iterator-2.c: Adjust for dependence-type
being optional.
* c-c++-common/gomp/depobj-1.c: New test.
* g++.dg/gomp/depend-iterator-2.C: Adjust for dependence-type being
optional.
* g++.dg/gomp/depobj-1.C: New test.
include/
* gomp-constants.h (GOMP_DEPEND_IN, GOMP_DEPEND_OUT,
GOMP_DEPEND_INOUT, GOMP_DEPEND_MUTEXINOUTSET): Define.
libgomp/
* omp.h.in (omp_depend_t): New typedef.
* task.c (gomp_task_handle_depend): Handle new depend array format
in addition to the old.  Handle mutexinoutset kinds the same as
inout for now, handle unspecified kinds.
(gomp_task_maybe_wait_for_dependencies): Likewise.
(gomp_create_target_task): Handle new depend array format count in
addition to the old.
(GOMP_task): Likewise.  Adjust function comment.
* testsuite/libgomp.c-c++-common/depend-iterator-2.c: New test.
* testsuite/libgomp.c-c++-common/depobj-1.c: New test.
* testsuite/libgomp.c++/depend-1.C: New test.
* testsuite/libgomp.c++/depobj-1.C: New test.

--- gcc/tree-core.h.jj  2018-06-05 15:00:56.357957856 +0200
+++ gcc/tree-core.h 2018-06-26 11:46:03.951421363 +0200
@@ -1416,6 +1416,7 @@ struct GTY(()) tree_constructor {
 
 enum omp_clause_depend_kind
 {
+  OMP_CLAUSE_DEPEND_UNSPECIFIED,
   OMP_CLAUSE_DEPEND_IN,
   OMP_CLAUSE_DEPEND_OUT,
   OMP_CLAUSE_DEPEND_INOUT,
--- gcc/gimplify.c.jj   2018-06-18 19:07:09.152186493 +0200
+++ gcc/gimplify.c  2018-06-27 11:33:45.353027093 +0200
@@ -7566,10 +7566,11 @@ gimplify_omp_depend (tree *list_p, gimpl
 {
   tree c;
   gimple *g;
-  size_t n[2] = { 0, 0 };
-  tree counts[2] = { NULL_TREE, NULL_TREE };
+  size_t n[4] = { 0, 0

Re: [patch, fortran] Handling of .and. and .or. expressions

[N.M. Maclaren]

On Jun 28 2018, Toon Moene wrote:


And - most interesting - that's how Fortran 77 formulated it (way before 
PURE/IMPURE functions entered the language):


"6.6.1 Evaluation of Operands

It is not necessary for a processor to evaluate all of the operands of 
an expression if the value of the expression can be determined 
otherwise. ...


Or even Fortran 66 :-)

6.4 Evaluation of Expressions.  A part of an expression need be evaluated
only if such action is necessary to establish the value of the expression.
... When two elements are combined by an operator, the order of evaluation
of the elements is optional.  ...

Regards,
Nick Maclaren.

[PATCH, committed] Fix insn length in pdp11 shift patterns

[Paul Koning]

This patch corrects the setting of the "length" attributes in the pdp11 target 
for certain shift cases.

Committed.

paul

ChangeLog:

2018-06-28  Paul Koning  

* config/pdp11/pdp11-protos.h (pdp11_shift_length): New function.
* config/pdp11/pdp11.c (pdp11_shift_length): New function.
* config/pdp11/pdp11.h (ADJUST_INSN_LENGTH): Remove.
* config/pdp11/pdp11.md: Correct "length" attribute calculation
for shift insn patterns.

Index: config/pdp11/pdp11-protos.h
===
--- config/pdp11/pdp11-protos.h (revision 262198)
+++ config/pdp11/pdp11-protos.h (working copy)
@@ -41,6 +41,7 @@ extern machine_mode pdp11_cc_mode (enum rtx_code,
 extern bool pdp11_expand_shift (rtx *, rtx (*) (rtx, rtx, rtx),
rtx (*) (rtx, rtx, rtx));
 extern const char * pdp11_assemble_shift (rtx *, machine_mode, int);
+extern int pdp11_shift_length (rtx *, machine_mode, int, bool);
 extern bool pdp11_small_shift (int);
 
 #endif /* RTX_CODE */
Index: config/pdp11/pdp11.c
===
--- config/pdp11/pdp11.c(revision 262198)
+++ config/pdp11/pdp11.c(working copy)
@@ -2020,6 +2020,35 @@ pdp11_assemble_shift (rtx *operands, machine_mode
   return "";
 }
 
+/* Figure out the length of the instructions that will be produced for
+   the given operands by pdp11_assemble_shift above.  */
+int
+pdp11_shift_length (rtx *operands, machine_mode m, int code, bool 
simple_operand_p)
+{
+  int shift_size;
+
+  /* Shift by 1 is 2 bytes if simple operand, 4 bytes if 2-word addressing 
mode.  */
+  shift_size = simple_operand_p ? 2 : 4;
+
+  /* In SImode, two shifts are needed per data item.  */
+  if (m == E_SImode)
+shift_size *= 2;
+
+  /* If shifting by a small constant, the loop is unrolled by the
+ shift count.  Otherwise, account for the size of the decrement
+ and branch.  */
+  if (CONSTANT_P (operands[2]) && pdp11_small_shift (INTVAL (operands[2])))
+shift_size *= INTVAL (operands[2]);
+  else
+shift_size += 4;
+
+  /* Logical right shift takes one more instruction (CLC).  */
+  if (code == LSHIFTRT)
+shift_size += 2;
+
+  return shift_size;
+}
+
 /* Worker function for TARGET_TRAMPOLINE_INIT.
 
trampoline - how should i do it in separate i+d ? 
Index: config/pdp11/pdp11.h
===
--- config/pdp11/pdp11.h(revision 262198)
+++ config/pdp11/pdp11.h(working copy)
@@ -123,22 +123,6 @@ extern const struct real_format pdp11_d_format;
 /* Define this if move instructions will actually fail to work
when given unaligned data.  */
 #define STRICT_ALIGNMENT 1
-
-/* Adjust the length of shifts by small constant amounts.  The base
-   value (in "length" on input) is the length of a shift by one, not
-   including the CLC in logical shifts.  */
-#define ADJUST_INSN_LENGTH(insn, length) \
-  if ((GET_CODE (insn) == ASHIFT || \
-   GET_CODE (insn) == ASHIFTRT || \
-   GET_CODE (insn) == LSHIFTRT) && \
-  GET_CODE (XEXP (insn, 2)) == CONST_INT && \
-  pdp11_small_shift (XINT (insn, 2))) \
-{\
-  if (GET_CODE (insn) == LSHIFTRT)   \
-   length = (length * XINT (insn, 2)) + 2; \
-  else \
-   length *= XINT (insn, 2); \
-}
 
 /* Standard register usage.  */
 
Index: config/pdp11/pdp11.md
===
--- config/pdp11/pdp11.md   (revision 262198)
+++ config/pdp11/pdp11.md   (working copy)
@@ -1297,11 +1297,6 @@
 ;; used to reduce the amount of very similar code.
 ;;
 ;; First the insns used for small constant shifts.
-;
-;; The "length" attribute values are modified by the ADJUST_INSN_LENGTH
-;; macro for the small constant shift case (first two alternatives).
-;; For those, the value coded in the length attribute is the cost of just
-;; the shift for a single shift.
 (define_insn "_sc"
   [(set (match_operand:QHSint 0 "nonimmediate_operand" "=rD,Q")
(SHF:QHSint (match_operand:QHSint 1 "general_operand" "0,0")
@@ -1308,7 +1303,9 @@
(match_operand:HI 2 "expand_shift_operand" "O,O")))]
   ""
   "* return pdp11_assemble_shift (operands, , );"
-  [(set_attr "length" "2,4")])
+  [(set (attr "length")
+   (symbol_ref "pdp11_shift_length (operands, , 
+ , which_alternative == 0)"))])
 
 ;; Next, shifts that are done as a loop on base (11/10 class) machines.
 ;; This applies to shift counts too large to unroll, or variable shift
@@ -1320,7 +1317,9 @@
(clobber (match_dup 2))]
   ""
   "* return pdp11_assemble_shift (operands, , );"
-  [(set_attr "length" "2,4")])
+  [(set (attr "length")
+   (symbol_ref "pdp11_shift_length (operands, , 
+ , which_alternative == 0)"))])
 
 ;; Next the insns that use 

[PATCH, testsuite/guality] Use line number vars in gdb-test

[Tom de Vries]

Hi,

I played around with pr45882.c and ran into FAILs.  It took me a while to
realize that the FAILs where due to the gdb-test (a dg-final action) using
absolute line numbers, and me adding lines before the gdb-test lines.

I've written this patch, which factors out the handling of relative line
numbers as well as line number variables from process-message, and reuses the
functionality in gdb-test.

This enables the line number variables functionality in gdb-test.  [ There's
one quirk: line number variables have a define-before-use semantics (with
matching used-before-defined error) but in the test-case the use in gdb-test
preceeds the definition in gdb-line.  This doesn't cause errors, because
the dg-final actions are executed after the definition has taken effect. ]

[ Relative line numbers still don't work in gdb-test, but that's due to an
orthogonal issue: gdb-test is a dg-final action, and while dg-final receives
the line number on which it occurred as it's first argument, it doesn't pass
on this line number to the argument list of the action. I'll submit a
follow-on rfc patch for this. ]

Tested pr45882.c.  Tested one test-case with relative line numbers, and
one with line number variables to make sure I didn't break process-message.

OK for trunk if bootstrap and reg-test succeeds?

Thanks,
- Tom

[testsuite/guality] Use line number vars in gdb-test

2018-06-28  Tom de Vries  

* gcc.dg/guality/pr45882.c (foo): Add line number var for breakpoint
line, and use it.
* lib/gcc-dg.exp (get-absolute-line): Factor out of ...
(process-message): ... here.
* lib/gcc-gdb-test.exp (gdb-test): Use get-absolute-line.

---
 gcc/testsuite/gcc.dg/guality/pr45882.c | 10 ++---
 gcc/testsuite/lib/gcc-dg.exp   | 73 +-
 gcc/testsuite/lib/gcc-gdb-test.exp |  3 +-
 3 files changed, 52 insertions(+), 34 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/guality/pr45882.c 
b/gcc/testsuite/gcc.dg/guality/pr45882.c
index ece35238a30..da9e2755590 100644
--- a/gcc/testsuite/gcc.dg/guality/pr45882.c
+++ b/gcc/testsuite/gcc.dg/guality/pr45882.c
@@ -9,11 +9,11 @@ volatile short int v;
 __attribute__((noinline,noclone,used)) int
 foo (int i, int j)
 {
-  int b = i;   /* { dg-final { gdb-test 16 "b" "7" } } */
-  int c = i + 4;   /* { dg-final { gdb-test 16 "c" "11" } } */
-  int d = a[i];/* { dg-final { gdb-test 16 "d" "112" } } */
-  int e = a[i + 6];/* { dg-final { gdb-test 16 "e" "142" } } */
-  ++v;
+  int b = i;   /* { dg-final { gdb-test bpline "b" "7" } } */
+  int c = i + 4;   /* { dg-final { gdb-test bpline "c" "11" } } */
+  int d = a[i];/* { dg-final { gdb-test bpline "d" "112" } } */
+  int e = a[i + 6];/* { dg-final { gdb-test bpline "e" "142" } } */
+  ++v; /* { dg-line bpline } */
   return ++j;
 }
 
diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index a15c5d5e2a6..22065c7e3fe 100644
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -1039,6 +1039,49 @@ proc dg-line { linenr varname } {
 }
 }
 
+# Get the absolute line number corresponding to:
+# - a relative line number (a non-null useline is required), or
+# - a line number variable reference.
+# Argument 0 is the line number on which line was used
+# Argument 1 is the relative line number or line number variable reference
+#
+proc get-absolute-line { useline line } {
+if { [regsub "^\.\[+-\](\[0-9\]+)$" $line "\\1" num] && $useline != "" } {
+   # Handle relative line specification, .+1 or .-1 etc.
+   set num [expr $useline [string index $line 1] $num]
+   return $num
+}
+
+if { ! [regsub "^(\[a-zA-Z\]\[a-zA-Z0-9_\]*)$" $line "\\1" varname] } {
+   return $line
+}
+
+# Handle linenr variable defined by dg-line
+set org_varname $varname
+set varname "saved_linenr_$varname"
+eval global $varname
+
+# Generate used-but-not-defined error.
+eval set var_defined [info exists $varname]
+if { ! $var_defined } {
+   if { "$useline" != "" } {
+   error "dg-line var $org_varname used at line $uselinenr, but not 
defined"
+   } else {
+   error "dg-line var $org_varname used, but not defined"
+   }
+   return
+}
+
+# Note that varname has been used.
+set varname_used "used_$varname"
+eval global $varname_used
+eval set $varname_used 1
+
+# Get line number from var and use it.
+eval set num \$$varname
+set line $num
+}
+
 # Modify the regular expression saved by a DejaGnu message directive to
 # include a prefix and to force the expression to match a single line.
 # MSGPROC is the procedure to call.
@@ -1049,34 +1092,8 @@ proc process-message { msgproc msgprefix dgargs } {
 upvar dg-messages dg-messages
 
 if { [llength $dgargs] == 5 } {
-   if { [regsub "^\.\[+-\](\[0-9\]+)$" [lindex $dgargs 4] "\\1" num] } {
-   # Handle re

[PATCH] Remove unused decl_scope_table

[Richard Biener]

Formerly used by scope_die_for this stack is now unused and just
pushed/popped to/from.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.

Richard.

2018-06-28  Richard Biener  

* dwarf2out.c (decl_scope_table): Remove.
(push_decl_scope): Likewise.
(pop_decl_scope): Likewise.
(gen_type_die_for_member): Do not call push/pop_decl_scope.
(gen_struct_or_union_type_die): Likewise.
(gen_tagged_type_die): Likewise.
(dwarf2out_init): Do not initialize decl_scope_table.
(dwarf2out_c_finalize): Do not free it.

Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 262217)
+++ gcc/dwarf2out.c (working copy)
@@ -150,13 +150,6 @@ static GTY(()) vec *used_rtx
it.  */
 static GTY(()) vec *incomplete_types;
 
-/* A pointer to the base of a table of references to declaration
-   scopes.  This table is a display which tracks the nesting
-   of declaration scopes at the current scope and containing
-   scopes.  This table is used to find the proper place to
-   define type declaration DIE's.  */
-static GTY(()) vec *decl_scope_table;
-
 /* Pointers to various DWARF2 sections.  */
 static GTY(()) section *debug_info_section;
 static GTY(()) section *debug_skeleton_info_section;
@@ -3835,8 +3828,6 @@ static void add_name_and_src_coords_attr
 static void add_discr_value (dw_die_ref, dw_discr_value *);
 static void add_discr_list (dw_die_ref, dw_discr_list_ref);
 static inline dw_discr_list_ref AT_discr_list (dw_attr_node *);
-static void push_decl_scope (tree);
-static void pop_decl_scope (void);
 static dw_die_ref scope_die_for (tree, dw_die_ref);
 static inline int local_scope_p (dw_die_ref);
 static inline int class_scope_p (dw_die_ref);
@@ -21361,22 +21352,6 @@ dwarf2out_vms_debug_main_pointer (void)
 }
 #endif /* VMS_DEBUGGING_INFO */
 
-/* Push a new declaration scope.  */
-
-static void
-push_decl_scope (tree scope)
-{
-  vec_safe_push (decl_scope_table, scope);
-}
-
-/* Pop a declaration scope.  */
-
-static inline void
-pop_decl_scope (void)
-{
-  decl_scope_table->pop ();
-}
-
 /* walk_tree helper function for uses_local_type, below.  */
 
 static tree
@@ -22359,7 +22334,6 @@ gen_type_die_for_member (tree type, tree
   dw_die_ref type_die;
   gcc_assert (!decl_ultimate_origin (member));
 
-  push_decl_scope (type);
   type_die = lookup_type_die_strip_naming_typedef (type);
   if (TREE_CODE (member) == FUNCTION_DECL)
gen_subprogram_die (member, type_die);
@@ -22381,8 +22355,6 @@ gen_type_die_for_member (tree type, tree
}
   else
gen_variable_die (member, NULL_TREE, type_die);
-
-  pop_decl_scope ();
 }
 }
 
@@ -25153,9 +25125,7 @@ gen_struct_or_union_type_die (tree type,
   if (type_die->die_parent == NULL)
add_child_die (scope_die, type_die);
 
-  push_decl_scope (type);
   gen_member_die (type, type_die);
-  pop_decl_scope ();
 
   add_gnat_descriptive_type_attribute (type_die, type, context_die);
   if (TYPE_ARTIFICIAL (type))
@@ -25309,14 +25279,12 @@ gen_tagged_type_die (tree type,
 dw_die_ref context_die,
 enum debug_info_usage usage)
 {
-  int need_pop;
-
   if (type == NULL_TREE
   || !is_tagged_type (type))
 return;
 
   if (TREE_ASM_WRITTEN (type))
-need_pop = 0;
+;
   /* If this is a nested type whose containing class hasn't been written
  out yet, writing it out will cover this one, too.  This does not apply
  to instantiations of member class templates; they need to be added to
@@ -25333,9 +25301,7 @@ gen_tagged_type_die (tree type,
return;
 
   /* If that failed, attach ourselves to the stub.  */
-  push_decl_scope (TYPE_CONTEXT (type));
   context_die = lookup_type_die (TYPE_CONTEXT (type));
-  need_pop = 1;
 }
   else if (TYPE_CONTEXT (type) != NULL_TREE
   && (TREE_CODE (TYPE_CONTEXT (type)) == FUNCTION_DECL))
@@ -25348,13 +25314,9 @@ gen_tagged_type_die (tree type,
 specification.  */
   if (context_die && is_declaration_die (context_die))
context_die = NULL;
-  need_pop = 0;
 }
   else
-{
-  context_die = declare_in_namespace (type, context_die);
-  need_pop = 0;
-}
+context_die = declare_in_namespace (type, context_die);
 
   if (TREE_CODE (type) == ENUMERAL_TYPE)
 {
@@ -25366,9 +25328,6 @@ gen_tagged_type_die (tree type,
   else
 gen_struct_or_union_type_die (type, context_die, usage);
 
-  if (need_pop)
-pop_decl_scope ();
-
   /* Don't set TREE_ASM_WRITTEN on an incomplete struct; we want to fix
  it up if it is ever completed.  gen_*_type_die will set it for us
  when appropriate.  */
@@ -28658,9 +28617,6 @@ dwarf2out_init (const char *filename ATT
   /* Allocate the cached_dw_loc_list_table.  */
   cached_dw_loc_list_table = hash_table::create_ggc (10);
 
-  /* Allocate the initial hunk

Re: [PATCH] Adjust subprogram DIE re-usal

[Richard Biener]

On Wed, 27 Jun 2018, Jason Merrill wrote:

> On Tue, Jun 26, 2018 at 8:43 AM, Richard Biener  wrote:
> >
> > A patch from Honza not LTO streaming DECL_ORIGINAL_TYPE ends up
> > ICEing during LTO bootstrap because we end up not re-using the
> > DIE we create late during LTRANS for a subprogram because its
> > parent is a namespace rather than a CU DIE (or a module).
> >
> > I'm wondering of the general reason why we enforce (inconsistently)
> > "We always want the DIE for this function that has the *_pc attributes to
> > be under comp_unit_die so the debugger can find it."
> > We indeed generate a specification DIE rooted at the CU in addition to the
> > declaration DIE inside the namespace for sth as simple as
> >
> > namespace Foo { void foo () {} }
> 
> The reason was to make it easier for debuggers to collect function
> definitions by scanning the top-level DIEs.  I don't know how
> useful/important that is to actual debuggers nowadays, since there are
> various accelerated lookup tables as well.

Ok.

> > anyhow - the comment also says "We also need to do this [re-use the DIE]
> > for abstract instances of inlines, since the spec requires the out-of-line
> > copy to have the same parent.".  Not sure what condition this part of
> > the comment applies to.
> 
> I think it's saying that we shouldn't re-use an in-class declaration
> die for the abstract instance of an inline, just like we shouldn't
> re-use it for a non-inline definition.
> 
> Incidentally, the same parent was only required by DWARF 2; DWARF 3+
> say it's typical but not required.

Ah, I see.

> > So my fix is to move the || get_AT (old_die, DW_AT_abstract_origin)
> > check I added for early LTO debug to a global override - forcing
> > DIE re-use for any DIE with an abstract origin set.  That is, all
> > concrete instances are fine where they are.  That also avoids
> > double-indirection DW_AT_specification -> DW_AT_abstract_origin -> DIE.
> >
> > But as said, I wonder about the overall condition, esp. the
> > DW_TAG_module special-casing (but not at the same time
> > special-casing DW_TAG_namespace or DW_TAG_partial_unit).
> 
> Hmm, the DW_TAG_module check seems to have come in with the
> early-debug merge.  I don't know what the rationale was for that; it
> certainly does weaken the case for treating CU scope specially.

The comment says it was done to fix some ICE.

> As for DW_TAG_partial_unit, probably the is_cu_die check should change
> to is_unit_die.

I'll bootstrap and test that change separately.

> > LTO bootstrap is in progress on x86_64-unknown-linux-gnu.
> >
> > OK if that succeeds?
> 
> OK.

It did, so applied.

Thanks,
Richard.

[PATCH][OBVIOUS] Add missing header file inclusion.

[Martin Liška]

Hi.

This fixes:

In file included from /space/rguenther/src/svn/trunk/gcc/brig/brigspec.c:26:
/space/rguenther/src/svn/trunk/gcc/gcc.h:60:3: error: ‘option_proposer’ does 
not name a type
   option_proposer m_option_proposer;

Installed as obvious.

Martin

gcc/brig/ChangeLog:

2018-06-28  Martin Liska  

* brigspec.c: Add missing header file inclusion.
---
 gcc/brig/brigspec.c | 1 +
 1 file changed, 1 insertion(+)


diff --git a/gcc/brig/brigspec.c b/gcc/brig/brigspec.c
index 35e2460b1e5..2c8a3faf8ff 100644
--- a/gcc/brig/brigspec.c
+++ b/gcc/brig/brigspec.c
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "system.h"
 #include "coretypes.h"
 #include "tm.h"
+#include "opt-suggestions.h"
 #include "gcc.h"
 #include "opts.h"
 

Re: [PATCH 2/N] IPA summaries use ::get in ipa-pure-const.c.

[Martin Liška]

On 06/23/2018 10:13 PM, Eric Botcazou wrote:
>> This is second part of IPA summaries clean-up. It removes some not needed
>>
>> ::get_create in ipa-pure.const.c.
> 
> It breaks the attached testcase compiled with "gnatmake lto_full -f -a -flto":
> 
> gnatbind -x lto_full.ali
> gnatlink lto_full.ali -flto
> during IPA pass: pure-const
> lto1: internal compiler error: Segmentation fault
> Please submit a full bug report,
> with preprocessed source if appropriate.
> See  for instructions.
> 

Hi.

Sorry for the breakage, according to back-track, it's very similar to:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86279

I'll fix that as well.

Martin

Re: [PATCH, testsuite/guality] Use line number vars in gdb-test

[Tom de Vries]

[ resending. It seems this did not get through to the mail archive. ]

On Thu, Jun 28, 2018 at 07:49:30PM +0200, Tom de Vries wrote:
> Hi,
> 
> I played around with pr45882.c and ran into FAILs.  It took me a while to
> realize that the FAILs where due to the gdb-test (a dg-final action) using
> absolute line numbers, and me adding lines before the gdb-test lines.
> 
> I've written this patch, which factors out the handling of relative line
> numbers as well as line number variables from process-message, and reuses the
> functionality in gdb-test.
> 
> This enables the line number variables functionality in gdb-test.  [ There's
> one quirk: line number variables have a define-before-use semantics (with
> matching used-before-defined error) but in the test-case the use in gdb-test
> preceeds the definition in gdb-line.  This doesn't cause errors, because
> the dg-final actions are executed after the definition has taken effect. ]
> 
> [ Relative line numbers still don't work in gdb-test, but that's due to an
> orthogonal issue: gdb-test is a dg-final action, and while dg-final receives
> the line number on which it occurred as it's first argument, it doesn't pass
> on this line number to the argument list of the action. I'll submit a
> follow-on rfc patch for this. ]
> 
> Tested pr45882.c.  Tested one test-case with relative line numbers, and
> one with line number variables to make sure I didn't break process-message.
> 
> OK for trunk if bootstrap and reg-test succeeds?
> 
> Thanks,
> - Tom
> 
> [testsuite/guality] Use line number vars in gdb-test
> 
> 2018-06-28  Tom de Vries  
> 
>   * gcc.dg/guality/pr45882.c (foo): Add line number var for breakpoint
>   line, and use it.
>   * lib/gcc-dg.exp (get-absolute-line): Factor out of ...
>   (process-message): ... here.
>   * lib/gcc-gdb-test.exp (gdb-test): Use get-absolute-line.
> 
> ---
>  gcc/testsuite/gcc.dg/guality/pr45882.c | 10 ++---
>  gcc/testsuite/lib/gcc-dg.exp   | 73 
> +-
>  gcc/testsuite/lib/gcc-gdb-test.exp |  3 +-
>  3 files changed, 52 insertions(+), 34 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.dg/guality/pr45882.c 
> b/gcc/testsuite/gcc.dg/guality/pr45882.c
> index ece35238a30..da9e2755590 100644
> --- a/gcc/testsuite/gcc.dg/guality/pr45882.c
> +++ b/gcc/testsuite/gcc.dg/guality/pr45882.c
> @@ -9,11 +9,11 @@ volatile short int v;
>  __attribute__((noinline,noclone,used)) int
>  foo (int i, int j)
>  {
> -  int b = i; /* { dg-final { gdb-test 16 "b" "7" } } */
> -  int c = i + 4; /* { dg-final { gdb-test 16 "c" "11" } } */
> -  int d = a[i];  /* { dg-final { gdb-test 16 "d" "112" } } */
> -  int e = a[i + 6];  /* { dg-final { gdb-test 16 "e" "142" } } */
> -  ++v;
> +  int b = i; /* { dg-final { gdb-test bpline "b" "7" } } */
> +  int c = i + 4; /* { dg-final { gdb-test bpline "c" "11" } } */
> +  int d = a[i];  /* { dg-final { gdb-test bpline "d" "112" } } */
> +  int e = a[i + 6];  /* { dg-final { gdb-test bpline "e" "142" } } */
> +  ++v;   /* { dg-line bpline } */
>return ++j;
>  }
>  
> diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
> index a15c5d5e2a6..22065c7e3fe 100644
> --- a/gcc/testsuite/lib/gcc-dg.exp
> +++ b/gcc/testsuite/lib/gcc-dg.exp
> @@ -1039,6 +1039,49 @@ proc dg-line { linenr varname } {
>  }
>  }
>  
> +# Get the absolute line number corresponding to:
> +# - a relative line number (a non-null useline is required), or
> +# - a line number variable reference.
> +# Argument 0 is the line number on which line was used
> +# Argument 1 is the relative line number or line number variable reference
> +#
> +proc get-absolute-line { useline line } {
> +if { [regsub "^\.\[+-\](\[0-9\]+)$" $line "\\1" num] && $useline != "" } 
> {
> + # Handle relative line specification, .+1 or .-1 etc.
> + set num [expr $useline [string index $line 1] $num]
> + return $num
> +}
> +
> +if { ! [regsub "^(\[a-zA-Z\]\[a-zA-Z0-9_\]*)$" $line "\\1" varname] } {
> + return $line
> +}
> +
> +# Handle linenr variable defined by dg-line
> +set org_varname $varname
> +set varname "saved_linenr_$varname"
> +eval global $varname
> +
> +# Generate used-but-not-defined error.
> +eval set var_defined [info exists $varname]
> +if { ! $var_defined } {
> + if { "$useline" != "" } {
> + error "dg-line var $org_varname used at line $uselinenr, but not 
> defined"
> + } else {
> + error "dg-line var $org_varname used, but not defined"
> + }
> + return
> +}
> +
> +# Note that varname has been used.
> +set varname_used "used_$varname"
> +eval global $varname_used
> +eval set $varname_used 1
> +
> +# Get line number from var and use it.
> +eval set num \$$varname
> +set line $num
> +}
> +
>  # Modify the regular expression saved by a DejaGnu message directive to
>  # include a pre

Re: [PATCH] Fix PR86321

[Richard Biener]

On Thu, 28 Jun 2018, Richard Biener wrote:

> 
> The fortran FE creates array descriptor types via build_distinct_type_copy
> which ends up re-using the TYPE_FIELDs chain of FIELD_DECLs between
> types in different type-variant chains.  While that seems harmless
> in practice it breaks once we try to generate C-like debug info for
> it because dwarf2out doesn't expect such sharing to occur (and I
> wouldn't be surprised of other odd behavior elsewhere that simply
> doesn't manifest in a as fatal way as PR86321).
> 
> We generate C-like debug info when you use LTO and -g0 at compile-time
> and -g at link-time (that's the way targets w/o debug-copy implementation
> end up wired).  For non-LTO we avoid directly generating debug for
> the array descriptor types by detecting them via a langhook.
> 
> The solution seems to be to adhere to the invariant that TYPE_FIELDs
> (and thus FIELD_DECL) sharing is only valid between variant types
> and their main variant.  Thus, copy the chain.
> 
> Bootstrap / regtest pending on x86_64-unknown-linux-gnu.
> 
> I suppose verify_type () could check proper ownership of the
> FIELD_DECLs (simply verify that DECL_CONTEXT is TYPE_MAIN_VARIANT).
> But I guess this may break in different ways.  Honza - did you
> originally try to verify that?  It currently says
> 
>   for (tree fld = TYPE_FIELDS (t); fld; fld = TREE_CHAIN (fld))
> {
>   /* TODO: verify properties of decls.  */
>   if (TREE_CODE (fld) == FIELD_DECL)
> ;
> ...

So at least for class Foo { enum Bar { BAZ }; }; the CONST_DECL for
BAZ appears in the TYPE_FIELDS for Foo and has DECL_CONTEXT Bar ...

Likely dwarf2out also has the following "hack" to deal with this:

gen_decl_die (tree decl, tree origin, struct vlr_context *ctx,
  dw_die_ref context_die)
{
...
case CONST_DECL:
  if (!is_fortran () && !is_ada ())
{
  /* The individual enumerators of an enum type get output when we 
output
 the Dwarf representation of the relevant enum type itself.  
*/
  break;
}

Richard.

> 
> OK for trunk?
> 
> Thanks,
> Richard.
> 
> 2018-06-28  Richard Biener  
> 
>   fortran/
>   PR lto/86321
>   * trans-types.c (gfc_get_array_type_bounds): Unshare TYPE_FIELDs
>   for the distinct type copy.
> 
> Index: gcc/fortran/trans-types.c
> ===
> --- gcc/fortran/trans-types.c (revision 262132)
> +++ gcc/fortran/trans-types.c (working copy)
> @@ -1923,6 +1923,14 @@ gfc_get_array_type_bounds (tree etype, i
>  
>base_type = gfc_get_array_descriptor_base (dimen, codimen, restricted);
>fat_type = build_distinct_type_copy (base_type);
> +  /* Unshare TYPE_FIELDs.  */
> +  for (tree *tp = &TYPE_FIELDS (fat_type); *tp; tp = &DECL_CHAIN (*tp))
> +{
> +  tree next = DECL_CHAIN (*tp);
> +  *tp = copy_node (*tp);
> +  DECL_CONTEXT (*tp) = fat_type;
> +  DECL_CHAIN (*tp) = next;
> +}
>/* Make sure that nontarget and target array type have the same canonical
>   type (and same stub decl for debug info).  */
>base_type = gfc_get_array_descriptor_base (dimen, codimen, false);
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

[PATCH] gen_subprogram_die followup.

[Richard Biener]

This does the is_cu_die->is_unit_die change and also removes the
stale comment I forgot to delete when moving the guarded condition.

Bootstrapped & tested on x86_64-unknown-linux-gnu, applied.

Richard.

2018-06-28  Richard Biener  

* dwarf2out.c (gen_subprogram_die): Use is_unit_die when
deciding whether to not re-use a DIE.

Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 262215)
+++ gcc/dwarf2out.c (working copy)
@@ -22780,7 +22780,7 @@ gen_subprogram_die (tree decl, dw_die_re
 apply; we just use the old DIE.  */
   expanded_location s = expand_location (DECL_SOURCE_LOCATION (decl));
   struct dwarf_file_data * file_index = lookup_filename (s.file);
-  if (((is_cu_die (old_die->die_parent)
+  if (((is_unit_die (old_die->die_parent)
/* This condition fixes the inconsistency/ICE with the
   following Fortran test (or some derivative thereof) while
   building libgfortran:
@@ -22796,9 +22796,6 @@ gen_subprogram_die (tree decl, dw_die_re
&& old_die->die_parent->die_tag == DW_TAG_module)
|| context_die == NULL)
   && (DECL_ARTIFICIAL (decl)
-  /* The location attributes may be in the abstract origin
- which in the case of LTO might be not available to
- look at.  */
   || (get_AT_file (old_die, DW_AT_decl_file) == file_index
   && (get_AT_unsigned (old_die, DW_AT_decl_line)
   == (unsigned) s.line)

[PATCH] Fix bit-test expansion for single cluster (PR tree-optimization/86263).

[Martin Liška]

Hi.

I'm sending patch for situation where we create a bit-test for
entire switch. In that case split BB must be redirected so that
the original switch is a dead code.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

gcc/ChangeLog:

2018-06-28  Martin Liska  

PR tree-optimization/86263
* tree-switch-conversion.c (switch_decision_tree::try_switch_expansion):
Make edge redirection.
---
 gcc/tree-switch-conversion.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)


diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c
index b79f2fdb6e6..4c9e7b9436b 100644
--- a/gcc/tree-switch-conversion.c
+++ b/gcc/tree-switch-conversion.c
@@ -1732,8 +1732,12 @@ switch_decision_tree::try_switch_expansion (vec &clusters)
   /* Do not do an extra work for a single cluster.  */
   if (clusters.length () == 1
   && clusters[0]->get_type () != SIMPLE_CASE)
-clusters[0]->emit (index_expr, index_type,
-		   gimple_switch_default_label (m_switch), m_default_bb);
+{
+  cluster *c = clusters[0];
+  c->emit (index_expr, index_type,
+	   gimple_switch_default_label (m_switch), m_default_bb);
+  redirect_edge_succ (single_succ_edge (bb), c->m_case_bb);
+}
   else
 {
   emit (bb, index_expr, default_edge->probability, index_type);

[PATCH] Fix PR86321

[Richard Biener]

The fortran FE creates array descriptor types via build_distinct_type_copy
which ends up re-using the TYPE_FIELDs chain of FIELD_DECLs between
types in different type-variant chains.  While that seems harmless
in practice it breaks once we try to generate C-like debug info for
it because dwarf2out doesn't expect such sharing to occur (and I
wouldn't be surprised of other odd behavior elsewhere that simply
doesn't manifest in a as fatal way as PR86321).

We generate C-like debug info when you use LTO and -g0 at compile-time
and -g at link-time (that's the way targets w/o debug-copy implementation
end up wired).  For non-LTO we avoid directly generating debug for
the array descriptor types by detecting them via a langhook.

The solution seems to be to adhere to the invariant that TYPE_FIELDs
(and thus FIELD_DECL) sharing is only valid between variant types
and their main variant.  Thus, copy the chain.

Bootstrap / regtest pending on x86_64-unknown-linux-gnu.

I suppose verify_type () could check proper ownership of the
FIELD_DECLs (simply verify that DECL_CONTEXT is TYPE_MAIN_VARIANT).
But I guess this may break in different ways.  Honza - did you
originally try to verify that?  It currently says

  for (tree fld = TYPE_FIELDS (t); fld; fld = TREE_CHAIN (fld))
{
  /* TODO: verify properties of decls.  */
  if (TREE_CODE (fld) == FIELD_DECL)
;
...

OK for trunk?

Thanks,
Richard.

2018-06-28  Richard Biener  

fortran/
PR lto/86321
* trans-types.c (gfc_get_array_type_bounds): Unshare TYPE_FIELDs
for the distinct type copy.

Index: gcc/fortran/trans-types.c
===
--- gcc/fortran/trans-types.c   (revision 262132)
+++ gcc/fortran/trans-types.c   (working copy)
@@ -1923,6 +1923,14 @@ gfc_get_array_type_bounds (tree etype, i
 
   base_type = gfc_get_array_descriptor_base (dimen, codimen, restricted);
   fat_type = build_distinct_type_copy (base_type);
+  /* Unshare TYPE_FIELDs.  */
+  for (tree *tp = &TYPE_FIELDS (fat_type); *tp; tp = &DECL_CHAIN (*tp))
+{
+  tree next = DECL_CHAIN (*tp);
+  *tp = copy_node (*tp);
+  DECL_CONTEXT (*tp) = fat_type;
+  DECL_CHAIN (*tp) = next;
+}
   /* Make sure that nontarget and target array type have the same canonical
  type (and same stub decl for debug info).  */
   base_type = gfc_get_array_descriptor_base (dimen, codimen, false);

[PATCH, i386]: Fix PR86348, ICE: in curr_insn_transform, unable to generate reloads for: vec_extractv4si_0_zext_sse4

[Uros Bizjak]

2018-06-28  Uros Bizjak  

PR target/86348
* config/i386/sse.md (*vec_extractv4si_0_zext_sse4): Use
alternative 0 in preferred_for_speed attribute.

testsuite/ChangeLog:

2018-06-28  Uros Bizjak  

PR target/86348
* gcc.target/i386/pr86348.c: New test.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN.

Uros.
Index: config/i386/sse.md
===
--- config/i386/sse.md  (revision 262224)
+++ config/i386/sse.md  (working copy)
@@ -13715,7 +13715,7 @@
   "#"
   [(set_attr "isa" "x64,*,avx512f")
(set (attr "preferred_for_speed")
- (cond [(eq_attr "alternative" "1")
+ (cond [(eq_attr "alternative" "0")
  (symbol_ref "TARGET_INTER_UNIT_MOVES_FROM_VEC")
   ]
   (symbol_ref "true")))])
Index: testsuite/gcc.target/i386/pr86348.c
===
--- testsuite/gcc.target/i386/pr86348.c (nonexistent)
+++ testsuite/gcc.target/i386/pr86348.c (working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile { target { ia32 } } } */
+/* { dg-options "-O -mtune=athlon -msse4" } */
+
+int i;
+unsigned __attribute__ ((__vector_size__ (16))) v;
+
+void
+foo (void)
+{
+  v *= i;
+  i = i > -(long long) v[0];
+}

Re: [patch, fortran] Handling of .and. and .or. expressions

[Jakub Jelinek]

On Thu, Jun 28, 2018 at 10:34:35AM -0700, Steve Kargl wrote:
> On Thu, Jun 28, 2018 at 07:03:05PM +0200, Jakub Jelinek wrote:
> > > In fact, I'll be submitting a bug report for a missed optimization.
> > > 
> > > subroutine foo(x,y)   subroutine foo(x,y)
> > >real x(10), y(10) real x(10), y(10)
> > >y = 0*sin(x)  y = 0
> > > end subroutine fooend subroutine foo
> > > 
> > > .L2:  pxor%xmm0, %xmm0
> > >callsinf   movq$0, 32(%rsi)
> > >pxor%xmm1, %xmm1   movups  %xmm0, (%rsi)
> > >mulss   %xmm1, %xmm0   movups  %xmm0, 16(%rsi)
> > >movss   %xmm0, 0(%rbp,%rbx)
> > >addq$4, %rbx
> > >cmpq$40, %rbx
> > >jne .L2
> > > 
> > > which I'm sure you'll just be thrilled with.
> > 
> > For floating point, you generally need -ffast-math to be able to
> > optimize such computations away, sinf already has const attribute (at least
> > in C/C++), so with -ffast-math it is optimized.
> 
> Doesn't -ffast-math allow the violaton of the integrity
> of parentheses?  That is, it allows more optimizations
> that violate the standard.

No, -ffast-math doesn't turn on -fno-protect-parens, only -Ofast does AFAIK.
Furthermore, to optimize away the sinf, you just need a subset of
-ffast-math, in particular -O2 -ffinite-math-only -fno-signed-zeros
optimizes it to zero stores too.  But without those 2 options it can't be
done, as if sinf returns e.g. negative value -1.0, then 0 * -1.0 is -0.0,
not 0.0, so optimizing it into 0.0 would be incorrect.  Similarly, if
the function would return Inf or -Inf or NaN (I know sinf should never
return Inf or -Inf, but it can return NaN, e.g. sinf (__builtin_inff ())
is NaN, or sinf (__builtin_nanf ("")) too), then 0 * NaN is NaN, not 0.

Jakub

[RFC, testsuite/guality] Use relative line numbers in gdb-test

[Tom de Vries]

[ was: [PATCH, testsuite/guality] Use line number vars in gdb-test ]

On Thu, Jun 28, 2018 at 07:49:30PM +0200, Tom de Vries wrote:
> Hi,
> 
> I played around with pr45882.c and ran into FAILs.  It took me a while to
> realize that the FAILs where due to the gdb-test (a dg-final action) using
> absolute line numbers, and me adding lines before the gdb-test lines.
> 
> I've written this patch, which factors out the handling of relative line
> numbers as well as line number variables from process-message, and reuses the
> functionality in gdb-test.
> 
> This enables the line number variables functionality in gdb-test.  [ There's
> one quirk: line number variables have a define-before-use semantics (with
> matching used-before-defined error) but in the test-case the use in gdb-test
> preceeds the definition in gdb-line.  This doesn't cause errors, because
> the dg-final actions are executed after the definition has taken effect. ]
> 
> [ Relative line numbers still don't work in gdb-test, but that's due to an
> orthogonal issue: gdb-test is a dg-final action, and while dg-final receives
> the line number on which it occurred as it's first argument, it doesn't pass
> on this line number to the argument list of the action. I'll submit a
> follow-on rfc patch for this. ]
>

This patch adds a dg-final override that passes it's first argument to the
gdb-test action.  This allows us to use relative line numbers in gdb-test.

Tested pr45882.c.

Any comments?
 
Thanks,
- Tom

[testsuite/guality] Use relative line numbers in gdb-test

2018-06-28  Tom de Vries  

* gcc.dg/guality/pr45882.c (foo): Use relative line numbers.
* lib/gcc-dg.exp (dg-final): New proc.
* lib/gcc-gdb-test.exp (gdb-test): Add and handle additional line number
argument.

---
 gcc/testsuite/gcc.dg/guality/pr45882.c | 10 +-
 gcc/testsuite/lib/gcc-dg.exp   | 20 
 gcc/testsuite/lib/gcc-gdb-test.exp |  4 ++--
 3 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/guality/pr45882.c 
b/gcc/testsuite/gcc.dg/guality/pr45882.c
index da9e2755590..02d74389ea0 100644
--- a/gcc/testsuite/gcc.dg/guality/pr45882.c
+++ b/gcc/testsuite/gcc.dg/guality/pr45882.c
@@ -9,11 +9,11 @@ volatile short int v;
 __attribute__((noinline,noclone,used)) int
 foo (int i, int j)
 {
-  int b = i;   /* { dg-final { gdb-test bpline "b" "7" } } */
-  int c = i + 4;   /* { dg-final { gdb-test bpline "c" "11" } } */
-  int d = a[i];/* { dg-final { gdb-test bpline "d" "112" } } */
-  int e = a[i + 6];/* { dg-final { gdb-test bpline "e" "142" } } */
-  ++v; /* { dg-line bpline } */
+  int b = i;   /* { dg-final { gdb-test .+4 "b" "7" } } */
+  int c = i + 4;   /* { dg-final { gdb-test .+3 "c" "11" } } */
+  int d = a[i];/* { dg-final { gdb-test .+2 "d" "112" } } */
+  int e = a[i + 6];/* { dg-final { gdb-test .+1 "e" "142" } } */
+  ++v;
   return ++j;
 }
 
diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index 22065c7e3fe..6f88ce2213e 100644
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -114,6 +114,26 @@ if [info exists ADDITIONAL_TORTURE_OPTIONS] {
[concat $DG_TORTURE_OPTIONS $ADDITIONAL_TORTURE_OPTIONS]
 }
 
+proc dg-final { args } {
+upvar dg-final-code final-code
+
+if { [llength $args] > 2 } {
+   error "[lindex $args 0]: too many arguments"
+}
+set line [lindex $args 0]
+set code [lindex $args 1]
+set directive [lindex $code 0]
+set withline \
+   [switch $directive {
+   gdb-test {expr {1}}
+   default  {expr {0}}
+   }]
+if { $withline == 1 } {
+   set code [linsert $code 1 $line]
+}
+append final-code "$code\n"
+}
+
 global orig_environment_saved
 
 # Deduce generated files from tool flags, return finalcode string
diff --git a/gcc/testsuite/lib/gcc-gdb-test.exp 
b/gcc/testsuite/lib/gcc-gdb-test.exp
index 5457e7a793e..c446f5b122d 100644
--- a/gcc/testsuite/lib/gcc-gdb-test.exp
+++ b/gcc/testsuite/lib/gcc-gdb-test.exp
@@ -26,7 +26,7 @@
 #   calling print on it in gdb. When asking for the type it is
 #   the literal string with extra whitespace removed.
 # Argument 3 handles expected failures and the like
-proc gdb-test { args } {
+proc gdb-test { useline args } {
 if { ![isnative] || [is_remote target] } { return }
 
 if { [llength $args] >= 4 } {
@@ -60,7 +60,7 @@ proc gdb-test { args } {
 set cmd_file "[file rootname [file tail $prog]].gdb"
 
 set fd [open $cmd_file "w"]
-set line [get-absolute-line "" [lindex $args 0]]
+set line [get-absolute-line $useline [lindex $args 0]]
 puts $fd "break $line"
 puts $fd "run"
 puts $fd "$command $var"

Re: [PATCH] C++: less verbose error-recovery for version conflict markers

[Jason Merrill]

OK.

On Thu, Jun 28, 2018 at 2:57 PM, David Malcolm  wrote:
> We handle version conflict markers in source:
>
> $ cat /tmp/test.cc
>
> extern void f1 (void);
> extern void f2 (void);
> extern void f3 (void);
> extern void f4 (void);
>
> void test ()
> {
>   f1 ();
> <<< HEAD
>   f2 ();
> ===
>   f3 ();
 252be53... Some commit message
>   f4 ();
> }
>
> The C frontend's output is mostly reasonable:
>
> gcc -xc /tmp/test.cc
> /tmp/test.cc: In function ‘test’:
> /tmp/test.cc:9:1: error: version control conflict marker in file
>  <<< HEAD
>  ^~~
> /tmp/test.cc:11:1: error: version control conflict marker in file
>  ===
>  ^~~
> /tmp/test.cc:13:1: error: version control conflict marker in file
>  >>> 252be53... Some commit message
>  ^~~
> /tmp/test.cc:13:9: error: invalid suffix "be53..." on integer constant
>  >>> 252be53... Some commit message
>  ^~
>
> whereas in C++ the output can be very verbose:
>
> /tmp/test.cc: In function ‘void test()’:
> /tmp/test.cc:9:1: error: version control conflict marker in file
>  <<< HEAD
>  ^~~
> /tmp/test.cc:9:3: error: expected primary-expression before ‘<<’ token
>  <<< HEAD
>^~
> /tmp/test.cc:9:5: error: expected primary-expression before ‘<<’ token
>  <<< HEAD
>  ^~
> /tmp/test.cc:9:7: error: expected primary-expression before ‘<’ token
>  <<< HEAD
>^
> /tmp/test.cc:9:9: error: ‘HEAD’ was not declared in this scope
>  <<< HEAD
>  ^~~~
> /tmp/test.cc:11:1: error: version control conflict marker in file
>  ===
>  ^~~
> /tmp/test.cc:11:3: error: expected primary-expression before ‘==’ token
>  ===
>^~
> /tmp/test.cc:11:5: error: expected primary-expression before ‘==’ token
>  ===
>  ^~
> /tmp/test.cc:11:7: error: expected primary-expression before ‘=’ token
>  ===
>^
> /tmp/test.cc:13:1: error: version control conflict marker in file
>  >>> 252be53... Some commit message
>  ^~~
> /tmp/test.cc:13:3: error: expected primary-expression before ‘>>’ token
>  >>> 252be53... Some commit message
>^~
> /tmp/test.cc:13:5: error: expected primary-expression before ‘>>’ token
>  >>> 252be53... Some commit message
>  ^~
> /tmp/test.cc:13:7: error: expected primary-expression before ‘>’ token
>  >>> 252be53... Some commit message
>^
> /tmp/test.cc:13:9: error: unable to find numeric literal operator 
> ‘operator""be53...’
>  >>> 252be53... Some commit message
>  ^~
>
> The following patch eliminates this spew by consuming tokens until the
> start of the next line after emitting such a message.
>
> The C frontend and C++ with -std=c++98 both emit:
>   error: invalid suffix "be53..." on integer constant
> on the above testcase, but it's not fixable through this approach,
> since that error is emitted by the lexer.
>
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/cp/ChangeLog:
> * parser.c (cp_parser_error_1): After issuing a conflict marker
> error, consume tokens until the end of the source line.
>
> gcc/testsuite/ChangeLog:
> * g++.dg/conflict-markers-2.C: New test.
> ---
>  gcc/cp/parser.c   | 14 ++
>  gcc/testsuite/g++.dg/conflict-markers-2.C | 17 +
>  2 files changed, 31 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/conflict-markers-2.C
>
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 154729c..09d224f 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -2862,6 +2862,20 @@ cp_parser_error_1 (cp_parser* parser, const char* 
> gmsgid,
>if (cp_lexer_peek_conflict_marker (parser->lexer, token->type, &loc))
> {
>   error_at (loc, "version control conflict marker in file");
> + expanded_location token_exploc = expand_location (token->location);
> + /* Consume tokens until the end of the source line.  */
> + while (1)
> +   {
> + cp_lexer_consume_token (parser->lexer);
> + cp_token *next = cp_lexer_peek_token (parser->lexer);
> + if (next == NULL)
> +   break;
> + expanded_location next_exploc = expand_location 
> (next->location);
> + if (next_exploc.file != token_exploc.file)
> +   break;
> + if (next_exploc.line != token_exploc.line)
> +   break;
> +   }
>   return;
> }
>  }
> diff --git a/gcc/testsuite/g++.dg/conflict-markers-2.C 
> b/gcc/testsuite/g++.dg/conflict-markers-2.C
> new file mode 100644
> index 000..4fc3820
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/conflict-markers-2.C
> @@ -0,0 +1,17 @@
> +// { dg-do compile { target c++11 } }
> +
> +extern void f1 (void);
> +extern void f2 (void);
> +extern void f3 (void);
> +extern void f4 (void);
> +
> +void test ()
> +{
> +  f1 ();
> +<<< HEAD // { dg-error "conflict marker" }
> +  f2 ();
> +=

C++ PATCH for c++/86342, -Wdeprecated-copy and system headers

[Jason Merrill]

-Wdeprecated-copy shouldn't warn about classes that the user can't change.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 8ca5b3176d0a79534b99bf0d53bb0467abb38800
Author: Jason Merrill 
Date:   Thu Jun 28 15:41:20 2018 -0400

PR c++/86342 - -Wdeprecated-copy and system headers.

* decl2.c (cp_warn_deprecated_use): Don't warn about declarations
in system headers.

diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 5fc6369d39d..e06ffa613b7 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -5208,8 +5208,10 @@ cp_warn_deprecated_use (tree decl, tsubst_flags_t complain)
   && DECL_NONSTATIC_MEMBER_FUNCTION_P (decl)
   && copy_fn_p (decl))
 {
-  warned = warning (OPT_Wdeprecated_copy,
-			"implicitly-declared %qD is deprecated", decl);
+  /* Don't warn about system library classes (c++/86342).  */
+  if (!DECL_IN_SYSTEM_HEADER (decl))
+	warned = warning (OPT_Wdeprecated_copy,
+			  "implicitly-declared %qD is deprecated", decl);
   if (warned)
 	{
 	  tree ctx = DECL_CONTEXT (decl);
diff --git a/gcc/testsuite/g++.dg/cpp0x/depr-copy2.C b/gcc/testsuite/g++.dg/cpp0x/depr-copy2.C
new file mode 100644
index 000..cef18b63d4a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/depr-copy2.C
@@ -0,0 +1,17 @@
+// PR c++/86342
+// { dg-options -Wdeprecated-copy }
+
+# 1 "deprcopy.cc"
+# 1 "deprcopy.h" 1 3
+
+struct X {
+  X() { }
+  ~X() { }
+};
+# 2 "deprcopy.cc" 2
+
+int main()
+{
+  X x;
+  X y = x;
+}

libgo patch committed: Don't stat a NULL filename

[Ian Lance Taylor]

The libgo code to create a backtrace state would sometimes call
stat(NULL).  This didn't matter, since the stat failure would not
cause any further problem, but it was pointless and sloppy.  The
problem was pointed out by Andreas Schwab in PR 86331.  This patch
removes it.  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 262120)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-baaaf1e0f1e9a54ea2dfe475154c85c83ec03740
+e1fcce0aec27b1f50ac0e736f39f4c806c2a5baa
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/runtime/go-caller.c
===
--- libgo/runtime/go-caller.c   (revision 261819)
+++ libgo/runtime/go-caller.c   (working copy)
@@ -116,7 +116,7 @@ __go_get_backtrace_state ()
 argv[0] (http://gcc.gnu.org/PR61895).  It would be nice to
 have a better check for whether this file is the real
 executable.  */
-  if (stat (filename, &s) < 0 || s.st_size < 1024)
+  if (filename != NULL && (stat (filename, &s) < 0 || s.st_size < 1024))
filename = NULL;
 
   back_state = backtrace_create_state (filename, 1, error_callback, NULL);

[PATCH] Make sure rs6000-modes.h is installed in plugin/include/config/rs6000/ subdir

[Jakub Jelinek]

Hi!

The newly added rs6000-modes.h is now included from rs6000.h, so it is
needed when building plugins that include tm.h, but it wasn't listed in the
Makefile fragments and therefore included among PLUGIN_HEADERS.

Fixed thusly, tested on powerpc64le-linux, ok for trunk and 8.2?

2018-06-28  Jakub Jelinek  

* config/rs6000/t-rs6000: Append rs6000-modes.h to TM_H.

--- gcc/config/rs6000/t-rs6000.jj   2018-05-06 23:13:00.205627800 +0200
+++ gcc/config/rs6000/t-rs6000  2018-06-28 23:31:41.426185017 +0200
@@ -20,6 +20,7 @@
 
 TM_H += $(srcdir)/config/rs6000/rs6000-builtin.def
 TM_H += $(srcdir)/config/rs6000/rs6000-cpus.def
+TM_H += $(srcdir)/config/rs6000/rs6000-modes.h
 PASSES_EXTRA += $(srcdir)/config/rs6000/rs6000-passes.def
 
 rs6000-c.o: $(srcdir)/config/rs6000/rs6000-c.c

Jakub

[testsuite/guality] Don't use attribute used in pr45882.c for -O0

[Tom de Vries]

[ was: Re: [testsuite] Fix guality/pr45882.c for flto ]

On Thu, Jun 21, 2018 at 02:52:31PM +0200, Richard Biener wrote:
> On Thu, 21 Jun 2018, Tom de Vries wrote:
> 
> > Hi,
> > 
> > Atm this test in pr45882.c:
> > ...
> >   int d = a[i];  /* { dg-final { gdb-test 16 "d" "112" } } */
> > ...
> > fails as follows with -flto:
> > ...
> > FAIL: gcc.dg/guality/pr45882.c   -O2 -flto -fuse-linker-plugin \
> >   -fno-fat-lto-objects  line 16 d == 112
> > ...
> > 
> > In more detail, gdb fails to print the value of d:
> > ...
> > Breakpoint 1, foo (i=i@entry=7, j=j@entry=7) at pr45882.c:16
> > 16++v;
> > $1 = 
> > $2 = 112
> >  != 112
> > ...
> > 
> > Variable d is a local variable in function foo, initialized from global 
> > array a.
> > When compiling, first cddce1 removes the initialization of d in foo, given
> > that d is not used afterwards.  Then ipa marks array a as write-only, and
> > removes the stores to array a in main.  This invalidates the location
> > expression for d, which points to a[i], so it is removed, which is why gdb
> > ends up printing  for d.
> > 
> > This patches fixes the fail by adding attribute used to array a, preventing
> > array a from being marked as write-only.
> > 
> > Tested on x86_64.
> > 
> > OK for trunk?
> 
> OK.
> 

I committed this patch, but thought about it some more, and realized that this
is the right solution for optimized code, but not for non-optimized code.

For optimized code, we generate debug info, but as best effort, and we might
need to inhibit the optimizers here and there by adding attribute used and
volatile etc in testcases, in order to allow sufficient debug information to
be generated.

But for non-optimized code (-O0), we shouldn't need such additions in the
testcases, since we're not supposed to optimize in the first place.

So, this patch conditionalizes attribute used in pr45882.c, depending on
whether we're optimizing or not.

This patch as is makes the test-case fail due to adding lines in front of
gdb-test, and gdb-test using absolute line numbers.  So, we either need
"[testsuite/guality] Use line number vars in gdb-test" (
https://gcc.gnu.org/ml/gcc-patches/2018-06/msg01825.html ), or the absolute
line numbers need to be updated.

OK for trunk if bootstrap and reg-test succeeds?

Thanks,
- Tom

[testsuite/guality] Don't use attribute used in pr45882.c for -O0

2018-06-28  Tom de Vries  

* gcc.dg/guality/guality.exp (guality_transform_options): New proc.
(toplevel): Apply guality_transform_options on DG_TORTURE_OPTIONS and
LTO_TORTURE_OPTIONS.
* gcc.dg/guality/prevent-optimization.h: New file.
* gcc.dg/guality/pr45882.c: Include prevent-optimization.h.
(a): Replace __attribute__((used)) with ATTRIBUTE_USED.

---
 gcc/testsuite/gcc.dg/guality/guality.exp   | 23 ++
 gcc/testsuite/gcc.dg/guality/pr45882.c |  4 +++-
 .../gcc.dg/guality/prevent-optimization.h  | 28 ++
 3 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/guality/guality.exp 
b/gcc/testsuite/gcc.dg/guality/guality.exp
index 04e889caa2f..d9994341477 100644
--- a/gcc/testsuite/gcc.dg/guality/guality.exp
+++ b/gcc/testsuite/gcc.dg/guality/guality.exp
@@ -48,6 +48,28 @@ if ![info exists ::env(GUALITY_GDB_NAME)] {
 }
 report_gdb $::env(GUALITY_GDB_NAME) [info script]
 
+proc guality_transform_options { args } {
+set res [list]
+foreach opt [lindex $args 0] {
+   #
+   if { ! [regexp -- "-O0" $opt] } {
+   set opt "$opt -DPREVENT_OPTIMIZATION"
+   }
+   lappend res $opt
+}
+
+return $res
+}
+
+global DG_TORTURE_OPTIONS
+set guality_dg_torture_options [guality_transform_options $DG_TORTURE_OPTIONS]
+set guality_lto_torture_options [guality_transform_options 
$LTO_TORTURE_OPTIONS]
+torture-init
+set-torture-options \
+$guality_dg_torture_options \
+[list {}] \
+$guality_lto_torture_options
+
 if {[check_guality "
   #include \"$srcdir/$subdir/guality.h\"
   volatile long int varl = 6;
@@ -65,4 +87,5 @@ if [info exists guality_gdb_name] {
 unsetenv GUALITY_GDB_NAME
 }
 
+torture-finish
 dg-finish
diff --git a/gcc/testsuite/gcc.dg/guality/pr45882.c 
b/gcc/testsuite/gcc.dg/guality/pr45882.c
index ece35238a30..47572f6a7a4 100644
--- a/gcc/testsuite/gcc.dg/guality/pr45882.c
+++ b/gcc/testsuite/gcc.dg/guality/pr45882.c
@@ -2,8 +2,10 @@
 /* { dg-do run } */
 /* { dg-options "-g" } */
 
+#include "prevent-optimization.h"
+
 extern void abort (void);
-int a[1024] __attribute__((used));
+int a[1024] ATTRIBUTE_USED;
 volatile short int v;
 
 __attribute__((noinline,noclone,used)) int
diff --git a/gcc/testsuite/gcc.dg/guality/prevent-optimization.h 
b/gcc/testsuite/gcc.dg/guality/prevent-optimization.h
new file mode 100644
index 000..0ef84a3d9c8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/guality/prevent-optimization.h
@@ -0,0 +1,28 @@
+/* Copyright (C) 2018 Free Software Foundation, Inc.
+

Re: [PATCH] Make sure rs6000-modes.h is installed in plugin/include/config/rs6000/ subdir

[Jeff Law]

On 06/28/2018 04:52 PM, Jakub Jelinek wrote:
> Hi!
> 
> The newly added rs6000-modes.h is now included from rs6000.h, so it is
> needed when building plugins that include tm.h, but it wasn't listed in the
> Makefile fragments and therefore included among PLUGIN_HEADERS.
> 
> Fixed thusly, tested on powerpc64le-linux, ok for trunk and 8.2?
> 
> 2018-06-28  Jakub Jelinek  
> 
>   * config/rs6000/t-rs6000: Append rs6000-modes.h to TM_H.
OK.
jeff

Re: [patch, i386] false dependencies fix

[Jeff Law]

On 06/28/2018 01:16 AM, Uros Bizjak wrote:
> Hello!
> 
>>> --- i386.md (revision 259756)
>>> +++ i386.md (working copy)
>>> @@ -3547,7 +3547,7 @@
>>>   {
>>>   case MODE_DF:
>>>if (TARGET_AVX && REG_P (operands[0]) && REG_P (operands[1]))
>>> -return "vmovsd\t{%1, %0, %0|%0, %0, %1}";
>>> +return "%vmovsd\t{%d1, %0|%0, %d1}";
>>>return "%vmovsd\t{%1, %0|%0, %1}";
>>>
>>>   case MODE_V4SF:
>>> @@ -3748,7 +3748,7 @@
>>>   {
>>>   case MODE_SF:
>>>if (TARGET_AVX && REG_P (operands[0]) && REG_P (operands[1]))
>>> -return "vmovss\t{%1, %0, %0|%0, %0, %1}";
>>> +return "%vmovss\t{%d1, %0|%0, %d1}";
>>>return "%vmovss\t{%1, %0|%0, %1}";
>> So what I'm confused about is in the original output template operand 0
>> is duplicated. In the new template operand 1 is duplicated.
>>
>> Presumably what you're trying to accomplish is avoiding a false read on
>> operand 0 (the destination)?  Can you please confirm?
> 
>> Knowing that should also help me evaluate the changes to recp and rsqrt
>> since they're being changed to the same style encoding when operating
>> strictly on registers.
> 
> Please don't change "v" -> "%v" for TARGET_AVX templates. We know that
> in this case, all insn mnemonics are prefixed with "v".
I didn't realize you'd already acked the patch without the v->%v bits
and Sebastian committed the changes about a month ago...

Oh well, at least I know a bit more about the inner workings of the AVX
unit than I did a last week.

Sorry for the noise.

jeff

Re: [testsuite/guality] Don't use attribute used in pr45882.c for -O0

[Jeff Law]

On 06/28/2018 05:00 PM, Tom de Vries wrote:
> [ was: Re: [testsuite] Fix guality/pr45882.c for flto ]
> 
> On Thu, Jun 21, 2018 at 02:52:31PM +0200, Richard Biener wrote:
>> On Thu, 21 Jun 2018, Tom de Vries wrote:
>>
>>> Hi,
>>>
>>> Atm this test in pr45882.c:
>>> ...
>>>   int d = a[i];  /* { dg-final { gdb-test 16 "d" "112" } } */
>>> ...
>>> fails as follows with -flto:
>>> ...
>>> FAIL: gcc.dg/guality/pr45882.c   -O2 -flto -fuse-linker-plugin \
>>>   -fno-fat-lto-objects  line 16 d == 112
>>> ...
>>>
>>> In more detail, gdb fails to print the value of d:
>>> ...
>>> Breakpoint 1, foo (i=i@entry=7, j=j@entry=7) at pr45882.c:16
>>> 16++v;
>>> $1 = 
>>> $2 = 112
>>>  != 112
>>> ...
>>>
>>> Variable d is a local variable in function foo, initialized from global 
>>> array a.
>>> When compiling, first cddce1 removes the initialization of d in foo, given
>>> that d is not used afterwards.  Then ipa marks array a as write-only, and
>>> removes the stores to array a in main.  This invalidates the location
>>> expression for d, which points to a[i], so it is removed, which is why gdb
>>> ends up printing  for d.
>>>
>>> This patches fixes the fail by adding attribute used to array a, preventing
>>> array a from being marked as write-only.
>>>
>>> Tested on x86_64.
>>>
>>> OK for trunk?
>>
>> OK.
>>
> 
> I committed this patch, but thought about it some more, and realized that this
> is the right solution for optimized code, but not for non-optimized code.
> 
> For optimized code, we generate debug info, but as best effort, and we might
> need to inhibit the optimizers here and there by adding attribute used and
> volatile etc in testcases, in order to allow sufficient debug information to
> be generated.
> 
> But for non-optimized code (-O0), we shouldn't need such additions in the
> testcases, since we're not supposed to optimize in the first place.
> 
> So, this patch conditionalizes attribute used in pr45882.c, depending on
> whether we're optimizing or not.
> 
> This patch as is makes the test-case fail due to adding lines in front of
> gdb-test, and gdb-test using absolute line numbers.  So, we either need
> "[testsuite/guality] Use line number vars in gdb-test" (
> https://gcc.gnu.org/ml/gcc-patches/2018-06/msg01825.html ), or the absolute
> line numbers need to be updated.
> 
> OK for trunk if bootstrap and reg-test succeeds?
> 
> Thanks,
> - Tom
> 
> [testsuite/guality] Don't use attribute used in pr45882.c for -O0
> 
> 2018-06-28  Tom de Vries  
> 
>   * gcc.dg/guality/guality.exp (guality_transform_options): New proc.
>   (toplevel): Apply guality_transform_options on DG_TORTURE_OPTIONS and
>   LTO_TORTURE_OPTIONS.
>   * gcc.dg/guality/prevent-optimization.h: New file.
>   * gcc.dg/guality/pr45882.c: Include prevent-optimization.h.
>   (a): Replace __attribute__((used)) with ATTRIBUTE_USED.
OK.
Jeff

Re: [PATCH, testsuite/guality] Use line number vars in gdb-test

[Jeff Law]

On 06/28/2018 11:49 AM, Tom de Vries wrote:
> Hi,
> 
> I played around with pr45882.c and ran into FAILs.  It took me a while to
> realize that the FAILs where due to the gdb-test (a dg-final action) using
> absolute line numbers, and me adding lines before the gdb-test lines.
> 
> I've written this patch, which factors out the handling of relative line
> numbers as well as line number variables from process-message, and reuses the
> functionality in gdb-test.
> 
> This enables the line number variables functionality in gdb-test.  [ There's
> one quirk: line number variables have a define-before-use semantics (with
> matching used-before-defined error) but in the test-case the use in gdb-test
> preceeds the definition in gdb-line.  This doesn't cause errors, because
> the dg-final actions are executed after the definition has taken effect. ]
> 
> [ Relative line numbers still don't work in gdb-test, but that's due to an
> orthogonal issue: gdb-test is a dg-final action, and while dg-final receives
> the line number on which it occurred as it's first argument, it doesn't pass
> on this line number to the argument list of the action. I'll submit a
> follow-on rfc patch for this. ]
> 
> Tested pr45882.c.  Tested one test-case with relative line numbers, and
> one with line number variables to make sure I didn't break process-message.
> 
> OK for trunk if bootstrap and reg-test succeeds?
> 
> Thanks,
> - Tom
> 
> [testsuite/guality] Use line number vars in gdb-test
> 
> 2018-06-28  Tom de Vries  
> 
>   * gcc.dg/guality/pr45882.c (foo): Add line number var for breakpoint
>   line, and use it.
>   * lib/gcc-dg.exp (get-absolute-line): Factor out of ...
>   (process-message): ... here.
>   * lib/gcc-gdb-test.exp (gdb-test): Use get-absolute-line.
I prefer the relative line numbers, but either approach is fine with me.
 Your call.

jeff

Re: [patch, fortran] Handling of .and. and .or. expressions

[Steve Kargl]

> 2018-06-27 10:09 GMT+02:00 Janne Blomqvist :
> >
> > How about committing a patch changing to use TRUTH_{AND|OR}_EXPR, and then
> > check benchmark results (polyhedron, spec etc.)? If performance worsens, we
> > revert, if it improves, great, lets keep it?
> 
> I would definitely support that. I cannot imagine that it will have a
> large impact on benchmarks, but it's certainly a good idea to check.
> (After all: ifort, which is usually perceived as being slightly ahead
> of GCC in terms of performance, does not do this kind of
> short-circuiting either.)

Polyhedron benchmark

Compiler options:
  -static -ffpe-summary=none -O3 -pipe -mtune=native
  -march=native -ffast-math -ftree-vectorize

   Benchmark  Unpatched   Patched
Name   (secs)  (sec)
   -      
  ac8.068.08   0.3%
  aermod   20.17   20.88   3.5%
 air3.573.58   0.3%
capacita   42.00   42.66   1.6%
 channel1.841.88   2.2%
   doduc   20.53   20.65   0.6%
 fatigue4.324.38   1.4%
 gas_dyn2.202.15  (2.3%)
  induct6.196.20   0.2%
   linpk7.237.79   7.8%
mdbx7.187.26   1.1%
  nf8.128.15   0.4%
 protein   26.72   26.98   1.0%
  rnflow   35.69   35.51  (0.5%)
test_fpu5.936.01   1.4%
tfft1.821.90   4.4%

14 of 16 tests experience a slow down.  As this is unscientific,
anything within 1% to 2% may not be significant.  However, linpk
is 7.8% slower, tfft is 4.4% slower, and aermod is 3.5% slower.

More importantly, with TRUTH_ANDIF_EXPR and TRUTH_ORIF_EXPR
(the status quo) on x86_64-*-freebsd, I see

=== gfortran Summary ===

# of expected passes47564
# of expected failures  104
# of unsupported tests  85
/safe/sgk/gcc/obj/gcc/gfortran  version 9.0.0 20180626 (experimental) (GCC) 

while with TRUTH_AND_EXPR and TRUTH_OR_EXPR, I get


=== gfortran Summary ===

# of expected passes47558
# of unexpected failures6
# of expected failures  104
# of unsupported tests  85

FAIL: gfortran.dg/actual_pointer_function_1.f90   -O0  execution test
FAIL: gfortran.dg/actual_pointer_function_1.f90   -O1  execution test
FAIL: gfortran.dg/actual_pointer_function_1.f90   -O2  execution test
FAIL: gfortran.dg/actual_pointer_function_1.f90   -O3 -fomit-frame-pointer 
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gfortran.dg/actual_pointer_function_1.f90   -O3 -g  execution test
FAIL: gfortran.dg/actual_pointer_function_1.f90   -Os  execution test

Execution timeout is: 300
spawn [open ...]

Program received signal SIGSEGV: Segmentation fault - invalid memory reference.

Backtrace for this error:
#0  0x71a2 in ???
#1  0x400c09 in ???
#2  0x400b91 in ???
#3  0x400c51 in ???
#4  0x400854 in _start
at /usr/src/lib/csu/amd64/crt1.c:74
#5  0x200627fff in ???

-- 
Steve

Re: [ABSU_EXPR] Add some of the missing patterns in match,pd

[Kugan Vivekanandarajah]

Hi Marc,

Thanks for the review.

On 28 June 2018 at 14:11, Marc Glisse  wrote:
> (why is there no mention of ABSU_EXPR in doc/* ?)

I will fix this in a separate patch.
>
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -571,10 +571,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (copysigns (op @0) @1)
> (copysigns @0 @1
>
> -/* abs(x)*abs(x) -> x*x.  Should be valid for all types.  */
> -(simplify
> - (mult (abs@1 @0) @1)
> - (mult @0 @0))
> +/* abs(x)*abs(x) -> x*x.  Should be valid for all types.
> +   also for absu(x)*absu(x) -> x*x.  */
> +(for op (abs absu)
> + (simplify
> +  (mult (op@1 @0) @1)
> +  (mult @0 @0)))
>
> 1) the types are wrong, it should be (convert (mult @0 @0)), or maybe
> view_convert if vectors also use absu.
> 2) this is only valid with -fwrapv (TYPE_OVERFLOW_WRAPS(TREE_TYPE(@0))),
> otherwise you are possibly replacing a multiplication on unsigned with a
> multiplication on signed that may overflow. So maybe it is actually supposed
> to be
> (mult (convert @0) (convert @0))
Indeed, I missed it. I have changed it in the attached patch.
>
>  /* cos(copysign(x, y)) -> cos(x).  Similarly for cosh.  */
>  (for coss (COS COSH)
> @@ -1013,15 +1015,24 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>&& tree_nop_conversion_p (type, TREE_TYPE (@2)))
>(bit_xor (convert @1) (convert @2
>
> -(simplify
> - (abs (abs@1 @0))
> - @1)
> -(simplify
> - (abs (negate @0))
> - (abs @0))
> -(simplify
> - (abs tree_expr_nonnegative_p@0)
> - @0)
> +/* Convert abs (abs (X)) into abs (X).
> +   also absu (absu (X)) into absu (X).  */
> +(for op (abs absu)
> + (simplify
> +  (op (op@1 @0))
> +  @1))
>
> You cannot have (absu (absu @0)) since absu takes a signed integer and
> returns an unsigned one. (absu (abs @0)) may indeed be equivalent to
> (convert (abs @0)). Possibly (op (convert (absu @0))) could also be
> simplified if convert is a NOP.
>
> +/* Convert abs[u] (-X) -> abs[u] (X).  */
> +(for op (abs absu)
> + (simplify
> +  (op (negate @0))
> +  (op @0)))
> +
> +/* Convert abs[u] (X)  where X is nonnegative -> (X).  */
> +(for op (abs absu)
> + (simplify
> +  (op tree_expr_nonnegative_p@0)
> +  @0))
>
> Missing convert again.
>
> Where are the testcases?
I have fixed the above and added test-cases.

>
>> Bootstrap and regression testing on x86_64-linux-gnu. Is this OK if no
>> regressions.
>
>
> Does it mean you have run the tests or intend to run them in the future? It
> is confusing.
Sorry for not being clear.

I have bootstrapped and regression tested with no new regression.

Is this OK?

Thanks,
Kugan

>
> --
> Marc Glisse
From a2606af5d9ed814cbbbce19dafbb780405ca8a06 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah 
Date: Fri, 29 Jun 2018 10:52:46 +1000
Subject: [PATCH] add absu patterns V2

Change-Id: I002312bf23a5fb225b7ff18e98ad22822baa6bef
---
 gcc/match.pd   | 23 +++
 gcc/testsuite/gcc.dg/gimplefe-30.c | 16 
 gcc/testsuite/gcc.dg/gimplefe-31.c | 16 
 gcc/testsuite/gcc.dg/gimplefe-32.c | 14 ++
 gcc/testsuite/gcc.dg/gimplefe-33.c | 16 
 5 files changed, 85 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/gimplefe-30.c
 create mode 100644 gcc/testsuite/gcc.dg/gimplefe-31.c
 create mode 100644 gcc/testsuite/gcc.dg/gimplefe-32.c
 create mode 100644 gcc/testsuite/gcc.dg/gimplefe-33.c

diff --git a/gcc/match.pd b/gcc/match.pd
index c1e0963..7959787 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -576,6 +576,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  (mult (abs@1 @0) @1)
  (mult @0 @0))
 
+/* Convert absu(x)*absu(x) -> x*x.  */
+(simplify
+ (mult (absu@1 @0) @1)
+ (mult (convert @0) (convert @0)))
+
 /* cos(copysign(x, y)) -> cos(x).  Similarly for cosh.  */
 (for coss (COS COSH)
  copysigns (COPYSIGN)
@@ -1013,16 +1018,34 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   && tree_nop_conversion_p (type, TREE_TYPE (@2)))
   (bit_xor (convert @1) (convert @2
 
+/* Convert abs (abs (X)) into abs (X).
+   also absu (absu (X)) into absu (X).  */
 (simplify
  (abs (abs@1 @0))
  @1)
+
+(simplify
+ (absu (nop_convert (absu@1 @0)))
+ @1)
+
+/* Convert abs[u] (-X) -> abs[u] (X).  */
 (simplify
  (abs (negate @0))
  (abs @0))
+
+(simplify
+ (absu (negate @0))
+ (absu @0))
+
+/* Convert abs[u] (X)  where X is nonnegative -> (X).  */
 (simplify
  (abs tree_expr_nonnegative_p@0)
  @0)
 
+(simplify
+ (absu tree_expr_nonnegative_p@0)
+ (convert @0))
+
 /* A few cases of fold-const.c negate_expr_p predicate.  */
 (match negate_expr_p
  INTEGER_CST
diff --git a/gcc/testsuite/gcc.dg/gimplefe-30.c b/gcc/testsuite/gcc.dg/gimplefe-30.c
new file mode 100644
index 000..6c25106
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/gimplefe-30.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fgimple -fdump-tree-optimized" } */
+
+unsigned int __GIMPLE() f(int a)
+{
+  unsigned int t0;
+  unsigned int t1;
+  unsigned int t2;
+  t0 = __ABSU a;
+  t1 = __ABSU a;
+  t2 = t0 * t1;
+  retur

Re: [PATCH] Add experimental::sample and experimental::shuffle from N4531

[Christophe Lyon]

On Mon, 25 Jun 2018 at 18:23, Jonathan Wakely  wrote:
>
> The additions to  were added in 2015 but the new
> algorithms in  were not. This adds them.
>
> * include/experimental/algorithm (sample, shuffle): Add new overloads
> using per-thread random number engine.
> * testsuite/experimental/algorithm/sample.cc: Simpify and reduce
> dependencies by using __gnu_test::test_container.
> * testsuite/experimental/algorithm/sample-2.cc: New.
> * testsuite/experimental/algorithm/shuffle.cc: New.
>
> Tested x86_64-linux, committed to trunk.
>
> This would be safe to backport, but nobody has noticed the algos are
> missing or complained, so it doesn't seem very important to backport.
>
>

Hi,

On bare-metal targets (aarch64 and arm + newlib), I've noticed that
the two new tests fail:
PASS: experimental/algorithm/shuffle.cc (test for excess errors)
spawn 
/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-eabi/gcc3/utils/bin/qemu-wrapper.sh
./shuffle.exe
terminate called after throwing an instance of 'std::runtime_error'
  what():  random_device::random_device(const std::string&)

*** EXIT code 4242
FAIL: experimental/algorithm/shuffle.cc execution test

PASS: experimental/algorithm/sample-2.cc (test for excess errors)
spawn 
/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-eabi/gcc3/utils/bin/qemu-wrapper.sh
./sample-2.exe
terminate called after throwing an instance of 'std::runtime_error'
  what():  random_device::random_device(const std::string&)

*** EXIT code 4242
FAIL: experimental/algorithm/sample-2.cc execution test

Does this ring a bell?

Thanks,

Christophe