[COMMITTED, PATCH] gcc.target/i386/pr89229-3c.c: Include "pr89229-3a.c"

[H.J. Lu]

On Thu, Mar 5, 2020 at 3:47 PM Jeff Law  wrote:
>
> On Sat, 2020-02-29 at 06:16 -0800, H.J. Lu wrote:
> > On x86, when AVX and AVX512 are enabled, vector move instructions can
> > be encoded with either 2-byte/3-byte VEX (AVX) or 4-byte EVEX (AVX512):
> >
> >0: c5 f9 6f d1 vmovdqa %xmm1,%xmm2
> >4: 62 f1 fd 08 6f d1   vmovdqa64 %xmm1,%xmm2
> >
> > We prefer VEX encoding over EVEX since VEX is shorter.  Also AVX512F
> > only supports 512-bit vector moves.  AVX512F + AVX512VL supports 128-bit
> > and 256-bit vector moves.  xmm16-xmm31 and ymm16-ymm31 are disallowed in
> > 128-bit and 256-bit modes when AVX512VL is disabled.  Mode attributes on
> > x86 vector move patterns indicate target preferences of vector move
> > encoding.  For scalar register to register move, we can use 512-bit
> > vector move instructions to move 32-bit/64-bit scalar if AVX512VL isn't
> > available.  With AVX512F and AVX512VL, we should use VEX encoding for
> > 128-bit/256-bit vector moves if upper 16 vector registers aren't used.
> > This patch adds a function, ix86_output_ssemov, to generate vector moves:
> >
> > 1. If zmm registers are used, use EVEX encoding.
> > 2. If xmm16-xmm31/ymm16-ymm31 registers aren't used, SSE or VEX encoding
> > will be generated.
> > 3. If xmm16-xmm31/ymm16-ymm31 registers are used:
> >a. With AVX512VL, AVX512VL vector moves will be generated.
> >b. Without AVX512VL, xmm16-xmm31/ymm16-ymm31 register to register
> >   move will be done with zmm register move.
> >
> > There is no need to set mode attribute to XImode explicitly since
> > ix86_output_ssemov can properly encode xmm16-xmm31/ymm16-ymm31 registers
> > with and without AVX512VL.
> >
> > Tested on AVX2 and AVX512 with and without --with-arch=native.
> >
> > gcc/
> >
> >   PR target/89229
> >   PR target/89346
> >   * config/i386/i386-protos.h (ix86_output_ssemov): New prototype.
> >   * config/i386/i386.c (ix86_get_ssemov): New function.
> >   (ix86_output_ssemov): Likewise.
> >   * config/i386/sse.md (VMOVE:mov_internal): Call
> >   ix86_output_ssemov for TYPE_SSEMOV.  Remove TARGET_AVX512VL
> >   check.
> >   (*movxi_internal_avx512f): Call ix86_output_ssemov for TYPE_SSEMOV.
> >   (*movoi_internal_avx): Call ix86_output_ssemov for TYPE_SSEMOV.
> >   Remove ext_sse_reg_operand and TARGET_AVX512VL check.
> >   (*movti_internal): Likewise.
> >   (*movtf_internal): Call ix86_output_ssemov for TYPE_SSEMOV.
> >
> > gcc/testsuite/
> >
> >   PR target/89229
> >   PR target/89346
> >   * gcc.target/i386/avx512vl-vmovdqa64-1.c: Updated.
> >   * gcc.target/i386/pr89346.c: New test.
> >
> > gcc/testsuite/
> >
> >   PR target/89229
> >   * gcc.target/i386/pr89229-2a.c: New test.
> >   * gcc.target/i386/pr89229-2b.c: Likewise.
> >   * gcc.target/i386/pr89229-2c.c: Likewise.
> >   * gcc.target/i386/pr89229-3a.c: Likewise.
> >   * gcc.target/i386/pr89229-3b.c: Likewise.
> >   * gcc.target/i386/pr89229-3c.c: Likewise.
> OK.  Let's get this one installed, let the various testers out there chew on 
> it
> for a day, then we'll iterate through the rest.
>
> Thanks again for your patience.
>

I checked in this patch to fix

FAIL: gcc.target/i386/pr89229-3c.c (test for excess errors)

Thanks.

-- 
H.J.

Re: [Patch, fortran] PR93581 - [9/10 Regression] ICE in gfc_get_dataptr_offset, at fortran/trans-array.c:6951

[Paul Richard Thomas]

***ping***

On Sun, 1 Mar 2020 at 16:00, Paul Richard Thomas
 wrote:
>
> This is a straightforward patch, especially for the bug in the PR! The
> additional fix ensures that expr%LEN always returns a scalar. Please
> note the comment in resolve.c about bounds checking.
>
> Regtests on trunk - OK for 9- and 10-branches?
>
> Paul
>
> 2020-03-01  Paul Thomas  
>
> PR fortran/93581
> * resolve.c (gfc_resolve_ref): Modify array refs to be elements
> if the ref chain ends in INQUIRY_LEN.
> * trans-array.c (gfc_get_dataptr_offset): Provide the offsets
> for INQUIRY_RE and INQUIRY_IM.
>
> 2020-03-01  Paul Thomas  
>
> PR fortran/93581
> * gfortran.dg/inquiry_type_ref_6.f90 : New test.



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

[Patch, fortran] PR93833 - [8/9/10 Regression] ICE in trans_array_constructor, at fortran/trans-array.c:2566

[Paul Richard Thomas]

This is yet another case, where a deferred character length variable
loses the character length backend_decl. As previously, converting the
expression and using the string_length recovers it successfully.

Regtested on FC31/x86_64 - OK for trunk, followed by 8- and 9-branches
after a week or two?

Paul

2020-03-08  Paul Thomas  

PR fortran/93833
* trans-array.c (get_array_ctor_var_strlen): If the character
length backend_decl cannot be found, convert the expression and
use the string length. Clear up some minor white space issues in
the rest of the file.

2020-03-08  Paul Thomas  

PR fortran/93833
* gfortran.dg/deferred_character_36.f90 : New test.

Re: [Patch, fortran] PR93581 - [9/10 Regression] ICE in gfc_get_dataptr_offset, at fortran/trans-array.c:6951

[Thomas Koenig via Gcc-patches]

Hi Paul,


***ping***


OK. Thanks for the patch!

Regards

Thomas

Re: [PATCH][gcc] libgccjit: introduce version entry points

[Andrea Corallo]

Hi,

thanks for reviewing, I've totally missed this multi-thread aspect.

David Malcolm  writes:

> On Thu, 2020-03-05 at 21:34 -0500, David Malcolm wrote:
>> On Thu, 2020-01-16 at 11:11 +, Andrea Corallo wrote:
>
> Responding to my own ideas about thread-safety.
>
> [...]
>
>> My first thought here was that we should have a way to get all three
>> at
>> once, but it turns out that parse_basever does its own caching
>> internally.
>>
>> I don't think the current implementation is thread-safe;
>> parse_basever
>> has:
>>
>>   static int s_major = -1, s_minor, s_patchlevel;
>>
>>   if (s_major == -1)
>> if (sscanf (BASEVER, "%d.%d.%d", &s_major, &s_minor,
>> &s_patchlevel) != 3)
>>   {
>>  sscanf (BASEVER, "%d.%d", &s_major, &s_minor);
>>  s_patchlevel = 0;
>>   }
>>
>> I think there's a race here: if two threads call parse_basever at the
>> same time, it looks like:
>>  (1) thread A could set s_major
>>  (2) thread B could read s_major, find it's set
>>  (3) thread B could read the uninitialized s_minor
>>  (4) thread A sets s_minor
>> and various similar issues.
>>
>> One fix might be to add a version mutex to libgccjit.c; maybe
>> something
>> like the following (caveat: I haven't tried compiling this):
>>
>> /* A mutex around the cached state in parse_basever.
>>Ideally this would be within parse_basever, but the mutex is only
>> needed
>>by libgccjit.  */
>>
>> static pthread_mutex_t version_mutex = PTHREAD_MUTEX_INITIALIZER;
>>
>> struct version_info
>> {
>>   /* Default constructor.  Populate via parse_basever,
>>  guarded by version_mutex.  */
>>   version_info ()
>>   {
>> pthread_mutex_lock (&version_mutex);
>> parse_basever (&major, &minor, &patchlevel);
>> pthread_mutex_unlock (&version_mutex);
>>   }
>>
>>   int major;
>>   int minor;
>>   int patchlevel;
>> };
>>
>> int
>> gcc_jit_version_major (void)
>> {
>>   version_info vi;
>>   return vi.major;
>> }
>>
>> int
>> gcc_jit_version_minor (void)
>> {
>>   version_info vi;
>>   return vi.minor;
>> }
>>
>> int
>> gcc_jit_version_patchlevel (void)
>> {
>>   version_info vi;
>>   return vi.patchlevel;
>> }
>>
>> Is adding a mutex a performance issue?  How frequently are these
>> going
>> to be called?
>>
>> Alternatively, maybe make these functions take a gcc_jit_context and
>> cache the version information within the context? (since the API
>> requires multithreaded programs to use their own locking if threads
>> share a context)
>
> In retrospect, I don't think this other approach would work: the state
> is within parse_basever, so if two threads both determine they need to
> access it at about the same time, then they will race.

Agree

>> Or some kind of caching in libgccjit.c?  (perhaps simply by making
>> the
>> version_info instances above static?  my memory of C++ function-
>> static
>> init rules and what we can rely on on our minimal compiler is a
>> little
>> hazy)
>
> I'd hoped that we could rely on static init being thread-safe, but in
> general it isn't, according to:
> https://eli.thegreenplace.net/2011/08/30/construction-of-function-static-variables-in-c-is-not-thread-safe
> (apparently GCC 4 onwards makes it so, but other compilers don't)
>
>
> From what I can tell parse_basever is only called once for the regular
> compiler use-case.  So maybe it makes sense to remove the caching from
> it, and move the caching to libgccjit.c where we can have a mutex
> (AFAIK none of the rest of the host code uses mutexes).
>
> Or split out the actual parsing logic into a parse_basever_uncached
> that libgccjit.c can use, and manage its own caching with a pthread
> mutex like in my suggested version_info code above.
>
> Thoughts?

What would be the advantage in splitting the cached and uncached
versions at this stage?  If we accept that parse_basever is not thread
safe we can just protect it with the mutex, we'll have to use one anyway
also if we keep the cache in the jit code.

Coming back on your question I assume client code will call very rarely
this function (especially if we compare against the compile times
involved) so I don't think we can have a perf issue here.

My vote is to go for the mutex solution you've first suggested.

  Andrea

New Danish PO file for 'cpplib' (version 10.1-b20200209)

[Translation Project Robot]

Hello, gentle maintainer.

This is a message from the Translation Project robot.

A revised PO file for textual domain 'cpplib' has been submitted
by the Danish team of translators.  The file is available at:

https://translationproject.org/latest/cpplib/da.po

(This file, 'cpplib-10.1-b20200209.da.po', has just now been sent to you in
a separate email.)

All other PO files for your package are available in:

https://translationproject.org/latest/cpplib/

Please consider including all of these in your next release, whether
official or a pretest.

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

The following HTML page has been updated:

https://translationproject.org/domain/cpplib.html

If any question arises, please contact the translation coordinator.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.

Contents of PO file 'cpplib-10.1-b20200209.da.po'

[Translation Project Robot]

The Translation Project robot, in the
name of your translation coordinator.

Re: [PATCH v2] generate EH info for volatile asm statements (PR93981)

[J.W. Jagersma]

On 2020-03-08 00:03, Segher Boessenkool wrote:
> On Sat, Mar 07, 2020 at 10:06:46PM +0100, J.W. Jagersma wrote:
>> Thanks Marek and Gerald, I hadn't read that page yet.  So do I resubmit
>> the final patch during stage 1, or will it end up in a queue to be
>> included when stage 1 for gcc 11 (or 10.2) begins?
> 
> It will be handled for GCC 11.  Please ping the patch when GCC 11 has
> opened (if necessary)?  We can of course keep discussing it now (if
> there are any open questions left.

Okay thanks, I'll keep an eye on it.

The only question I have left now is if my proposed change to the
documentation is acceptable or should be expanded/reworded.  Any other
feedback is welcome too, of course.
The test cases I think I've sorted out now.  I added a target-
independent test that scans the assembly for a catch block.  I think
that is a reliable method to confirm that EH information is being
generated.  The target-dependent test is set to xfail on anything
except *-linux-gnu, and only runs on x86/amd64 targets.

There is also still the question of whether non-volatile asms should be
allowed to throw or not.  I don't know if that should be discussed here
or on the PR.

Re: Minor regression due to recent IRA changes

[Jeff Law]

On Sun, 2020-03-08 at 15:53 +0900, Oleg Endo wrote:
> On Thu, 2020-03-05 at 08:51 -0700, Jeff Law wrote:
> > FWIW I've got an sh4/sh4eb bootstrap and regression test running with
> > HONOR_REG_ALLOC_ORDER defined.  As Vlad mentioned, that may be a
> > viable workaround.
> > 
> 
> I've had a look at the good old CSiBE code size results and poked at
> some of the cases.  Overall, it seems to help code quality when
> HONOR_REG_ALLOC_ORDER is defined on SH.
> 
> sum:  3383449 -> 3379629-3820 / -0.112903 %
> avg: -212.22 / -0.271573 %
> max: flex-2.5.31  253514 -> 253718+204 / +0.080469 %
> min: bzip2-1.0.2   67202 -> 65938-1264 / -1.880896 %
> 
> 
> However, even with HONOR_REG_ALLOC_ORDER defined, the simple test case
> from PR 81426 https://gcc.gnu.org/bugzilla/attachment.cgi?id=47159
> fails to compile without -mlra (use options -m4 -matomic-model=soft-gusa on
> regular non-linux sh-elf cross compiler).
> 
> How about the bootstrap, Jeff?  Did it help anything?
Bootstrapped just fine. It neither regressed nor fixed anything in the GCC
testsuite -- so it doesn't appear to address the c-torture/compile/sync-1
regression.

Jeff

Re: [PING^2][PATCH] Fix documentation of -mpoke-function-name ARM option

[blatinox]

Hi Sandra,

On Mon, Feb 24, 2020 at 02:19:21PM -0700, Sandra Loosemore wrote:
> On 2/20/20 3:08 PM, Jérémy Lefaure wrote:
> > Hello,
> > 
> > Ping for https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01081.html.
> > 
> > Thank you,
> > Jérémy
> > 
> > On Sun, Dec 15, 2019 at 07:20:26PM +0100, Jérémy Lefaure wrote:
> > > Hi!
> > > 
> > > Since in ARM state the value of PC is the address of the current
> > > instruction plus 8 bytes, the code inspecting the value of PC stored at
> > > FP + 0 should look at location PC - 16 : PC - 8 points to the stmfd
> > > instruction, PC - 16 points two words before, where the top 8 bits are
> > > set.
> > > 
> > > gcc/
> > > 2019-12-14  Jérémy Lefaure 
> > > 
> > >   * config/arm/arm.c (-mpoke-function-name): Fix documentation in comment.
> > >   * doc/invoke.texi (-mpoke-function-name): Fix documentation.
> > > 
> > 
> 
> The patch looks OK to me from a documentation point of view, but I'm no
> expert about the technical correctness of the change.  Maybe an ARM
> maintainer can review this?
> 

Thank you for taking the time to review this patch.
I hope that an ARM maintainer will have time to review it too.


Thank you,
Jérémy

Re: [PATCH v2] c++: Fix ABI issue with alignas on armv7hl [PR94050]

[Marek Polacek]

On Sun, Mar 08, 2020 at 01:25:17AM -0500, Jason Merrill wrote:
> On 3/7/20 6:02 PM, Marek Polacek wrote:
> > On Sat, Mar 07, 2020 at 01:21:41AM -0500, Jason Merrill wrote:
> > > On 3/6/20 8:12 PM, Marek Polacek wrote:
> > > > On Fri, Mar 06, 2020 at 05:49:07PM -0500, Jason Merrill wrote:
> > > > > On 3/5/20 2:40 PM, Marek Polacek wrote:
> > > > > > The static_assert in the following test was failing on armv7hl 
> > > > > > because
> > > > > > we were disregarding the alignas specifier on Cell.  BaseShape's 
> > > > > > data
> > > > > > takes up 20B on 32-bit architectures, but we failed to round up its
> > > > > > TYPE_SIZE.  This happens since the
> > > > > > 
> > > > > > patch: here, in layout_class_type for TenuredCell, we see that the 
> > > > > > size
> > > > > > of TenuredCell and its CLASSTYPE_AS_BASE match, so we set
> > > > > > 
> > > > > >  CLASSTYPE_AS_BASE (t) = t;
> > > > > > 
> > > > > > But while TYPE_USER_ALIGN of TenuredCell was 0, TYPE_USER_ALIGN of 
> > > > > > its
> > > > > > CLASSTYPE_AS_BASE was 1.
> > > > > 
> > > > > Surely the bug is that TYPE_USER_ALIGN isn't set for TenuredCell?
> > > > 
> > > > That's my understanding.
> > > 
> > > So why is that?  Where is setting TYPE_USER_ALIGN on as-base TenuredCell,
> > > and why isn't it setting it on the main type as well?  Or is it getting
> > > cleared on the main type for some reason?
> > 
> > Yeah, it's getting cleared in finalize_type_size, called from
> >   6703   /* Let the back end lay out the type.  */
> >   6704   finish_record_layout (rli, /*free_p=*/true);
> > because finalize_type_size has
> > 1930   /* Don't override a larger alignment requirement coming from a 
> > user
> > 1931  alignment of one of the fields.  */
> > 1932   if (mode_align >= TYPE_ALIGN (type))
> > 1933 {
> > 1934   SET_TYPE_ALIGN (type, mode_align);
> > 1935   TYPE_USER_ALIGN (type) = 0;
> > 1936 }
> > (for aggregates it is only done on STRICT_ALIGNMENT platforms which is why 
> > we
> >   won't see this problem on e.g. i686).
> > 
> > Here's the story of TYPE_USER_ALIGN:
> > - first we set TYPE_USER_ALIGN on Cell in common_handle_aligned_attribute,
> >as expected
> > - in layout_class_type for Cell we copy T_U_A to its as-base type:
> >TYPE_USER_ALIGN (base_t) = TYPE_USER_ALIGN (t);
> > - then we call finish_record_layout and T_U_A is cleared on Cell, but not 
> > its
> >as-base type.  In finalize_type_size mode_align == TYPE_ALIGN (type) == 
> > 64.
> > - so in layout_empty_base_or_field we do this:
> >if (CLASSTYPE_USER_ALIGN (type))
> >  {
> >rli->record_align = MAX (rli->record_align, CLASSTYPE_ALIGN (type));
> >if (warn_packed)
> >  rli->unpacked_align = MAX (rli->unpacked_align, CLASSTYPE_ALIGN 
> > (type));
> >TYPE_USER_ALIGN (rli->t) = 1;
> >  }
> >type is Cell and rli->t is TenuredCell
> > - then in layout_class_type we copy T_U_A from TenuredCell to its as-base 
> > type,
> >then finish_record_layout clears it from the main type
> > - then we perform the new optimization by replacing the as-base type, making
> >T_U_A set to 0
> > - and so BaseShape's T_U_A is never set.
> > 
> > Does this explain things better?
> 
> Yes, thanks.  So the problem is the interaction of finalize_type_size
> deciding that we don't need TYPE_USER_ALIGN if it's the same alignment we
> would get anyway, and layout_empty_base_or_field only adjusting the
> alignment if TYPE_USER_ALIGN is set, which happened to work before because
> CLASSTYPE_USER_ALIGN was accidentally(?) still set.

I wish we had a comment to that effect -- then we'd know for sure that it
wasn't just an accident.

> I think the right behavior is probably to update rli->*_align regardless of
> CLASSTYPE_USER_ALIGN (type); if an empty base doesn't have user-specified
> aligment, its alignment will be 1, so it shouldn't ever be harmful.  When I
> added that code I wasn't aware of the finalize_type_size behavior.

And given it only triggers on STRICT_ALIGNMENT, no wonder we haven't seen this
before.

> But I'm not confident that won't be an ABI break itself, so let's go ahead
> with your approach for GCC 10, except

Yeah, it's tricky stuff :/

> > +  /* If T's CLASSTYPE_AS_BASE is TYPE_USER_ALIGN, but T is not,
> > +replacing the as-base type would change CLASSTYPE_USER_ALIGN,
> > +causing us to lose the user-specified alignment as in PR94050.  */
> > +  && !CLASSTYPE_USER_ALIGN (t)
> 
> How about
> 
> && CLASSTYPE_USER_ALIGN (t) == CLASSTYPE_USER_ALIGN (CLASSTYPE_AS_BASE (t))

Presumably you mean TYPE_USER_ALIGN, but sure.  Thanks,

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

-- >8 --
The static_assert in the following test was failing on armv7hl because
we were disregarding the alignas specifier on Cell.  BaseShape's data
takes up 20B on 32-bit architectures, but we failed to round up its
TYPE_SIZE. 

Re: [PATCH] [rs6000] Fix a wrong GC issue

[binbin]

Hi Segher,

On 2020/3/5 下午6:42, Segher Boessenkool wrote:

Hi Bin Bin,

On Thu, Mar 05, 2020 at 10:31:39AM +0800, binbin wrote:

On 2020/3/5 上午2:35, Segher Boessenkool wrote:

On Wed, Mar 04, 2020 at 03:08:41PM +0800, binbin wrote:

* config/rs6000/rs6000.h (MAX_MACHINE_MODE): Include the header file
for MAX_MACHINE_MODE.


The changelog entry should say *what* file is included, and under what
condition.  It doesn't have to say why (that belongs in the commit
message).

But, can't you just include it unconditionally?  Don't we already,
anyway, via coretypes.h -> machmode.h -> insn-modes.h?


OK, change it to uncondition.  Thanks for your suggestion.


What about the second part?  Shouldn't it already be included anyway?


If "insn-modes.h" is not included in rs6000.h, it reports error showing
MAX_MACHINE_MODE’ undeclared here (not in a function) in file included
from
../../host-powerpc64le-unknown-linux-gnu/gcc/tm.h:25
from ../.././libgcc/libgcc2.c:29
../.././libgcc/../gcc/config/rs6000/rs6000.h:2495:42.  Thanks.


Sure.  But why do we need to include that file at all here?  It should
already be included via coretypes.h (which includes machmode.h, which
includes insn-modes.h).


+extern GTY(()) tree builtin_mode_to_type[MAX_MACHINE_MODE][2];
+extern GTY(()) tree altivec_builtin_mask_for_load;
  
  #ifndef USED_FOR_TARGET


Maybe it should be moved into here?  Or some other #ifdef trickery?


Segher



OK, changed the code.  Bootstrap and regression tests were done on
powerpc64le-linux-gnu (LE) with no regressions.  Thanks for your good
suggestion.
gcc/ChangeLog

2020-03-09  Bin Bin Lv  

* config/rs6000/rs6000-internal.h (altivec_builtin_mask_for_load,
builtin_mode_to_type[MAX_MACHINE_MODE][2]): Remove the declaration.
* config/rs6000/rs6000.h (altivec_builtin_mask_for_load,
builtin_mode_to_type[MAX_MACHINE_MODE][2]): Add an extern GTY(())
declaration.
* config/rs6000/rs6000.c (altivec_builtin_mask_for_load,
builtin_mode_to_type[MAX_MACHINE_MODE][2]): Remove the GTY(())
declaration and add the definition.
---
 gcc/config/rs6000/rs6000-internal.h | 2 --
 gcc/config/rs6000/rs6000.c  | 4 ++--
 gcc/config/rs6000/rs6000.h  | 2 ++
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/gcc/config/rs6000/rs6000-internal.h 
b/gcc/config/rs6000/rs6000-internal.h
index a23e956..d331b9e 100644
--- a/gcc/config/rs6000/rs6000-internal.h
+++ b/gcc/config/rs6000/rs6000-internal.h
@@ -187,7 +187,5 @@ extern bool rs6000_passes_long_double;
 extern bool rs6000_passes_vector;
 extern bool rs6000_returns_struct;
 extern bool cpu_builtin_p;
-extern GTY(()) tree builtin_mode_to_type[MAX_MACHINE_MODE][2];
-extern GTY(()) tree altivec_builtin_mask_for_load;
 
 #endif
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 9910b27..0faf44b 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -99,7 +99,7 @@
 #endif
 
 /* Support targetm.vectorize.builtin_mask_for_load.  */
-GTY(()) tree altivec_builtin_mask_for_load;
+tree altivec_builtin_mask_for_load;
 
 #ifdef USING_ELFOS_H
 /* Counter for labels which are to be placed in .fixup.  */
@@ -196,7 +196,7 @@ enum reg_class rs6000_constraints[RS6000_CONSTRAINT_MAX];
 int rs6000_vector_align[NUM_MACHINE_MODES];
 
 /* Map selected modes to types for builtins.  */
-GTY(()) tree builtin_mode_to_type[MAX_MACHINE_MODE][2];
+tree builtin_mode_to_type[MAX_MACHINE_MODE][2];
 
 /* What modes to automatically generate reciprocal divide estimate (fre) and
reciprocal sqrt (frsqrte) for.  */
diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
index 1697186..724085b 100644
--- a/gcc/config/rs6000/rs6000.h
+++ b/gcc/config/rs6000/rs6000.h
@@ -2490,6 +2490,8 @@ extern GTY(()) tree rs6000_builtin_types[RS6000_BTI_MAX];
 extern GTY(()) tree rs6000_builtin_decls[RS6000_BUILTIN_COUNT];
 
 #ifndef USED_FOR_TARGET
+extern GTY(()) tree builtin_mode_to_type[MAX_MACHINE_MODE][2];
+extern GTY(()) tree altivec_builtin_mask_for_load;
 /* A C structure for machine-specific, per-function data.
This is added to the cfun structure.  */
 typedef struct GTY(()) machine_function
-- 
1.8.3.1

Re: Bind to std::equal plumbing in ranges::equal

[François Dumont]

On 3/6/20 11:12 AM, Jonathan Wakely wrote:

On 06/03/20 10:09 +, Jonathan Wakely wrote:

On 06/03/20 07:06 +0100, François Dumont wrote:
I started to work on ranges::equal to find out if what I am trying 
to do is totally silly.


With this patch ranges::equal is in pare with std::equal 
specializations that is to say that it correctly deals with Debug 
mode or std::deque iterators.


Once below patch is in:

https://gcc.gnu.org/ml/libstdc++/2019-12/msg00032.html

We will even be able to call std::__equal_aux1 directly using 
__niter_base to get rid of the Debug safe iterator layer. And even 
in this case get rid of the branch __use_memcmp and leave it to 
__equal_aux1.


I mainly fear the usage of std::iterator_traits in __equal_aux1 to 
be a problem. Is it in this context of sized_sentinel ?


I don't understand the question. No sentinel of type _Sent1 or _Sent2
gets passed to __equal_aux1 with your patch, you only pass iterators.


I just thought that std::iterator_traits was becoming somehow obsolete 
is the more recent Standard and that it was the reason for not using 
existing std algos.





But I think the patch is wrong:


+  return !std::__memcmp(__first1, __first2, __d1);
+    else
+  return std::__equal_aux(__first1, __first1 + __d1, 
__first2);


This last line will only compile if _Iter1 is random access, but all
we know at this point is that _Sent1 is a sized sentinel for _Iter1.
That doesn't mean it's necessarily random access.


Please try the example at https://wg21.link/counted.iterator#2 which
uses counted_iterator::iterator> and default_sentinel.
The sized_sentinel_for concept is satisfied, but you can't do first1+d1.


Thanks for this example, now I know what has to be supported. I'll see 
if I can find a solution before you do.


François