Re:

Re:

[Ziemowit Laski]

Mark, Zack,

Please see my responses below.

> > This is by far the biggest chunk of the ObjC++ integration work; its
> > purpose is to set the stage
> > for ObjC++ proper.
> >
> > I've broken down the patch into two parts, A and B.  Part A (this
> > one) touches common parts of the compiler, and therefore requires
> > global approval.
>
> You need to explain, in detail, what each change does and why.
>
> You have broken up this patch along the wrong lines.  Never break a
> patch into pieces which could not be committed independently.  Changes

The reason I broke the patch up is simply to ease the review process, 
since
I can take maintainer responsibility for a large chunk of it.  I 
apologize if
this is confusing; this _is_ really all one giant patch.

> which are strictly mechanical - e.g. renaming functions - should
> always be submitted separately from other changes.  Prototypes for new
> functions should always be submitted alongside the new functions
> themselves.  In this case it probably makes sense for you to send a
> patch consisting solely of mechanical changes; a patch consisting
> solely of addition of new functions (it's okay to add new functions
> before the code that uses them); a patch consisting of miscellaneous
> changes to common code, *along with* the minimal set of associated
> changes to the Objective C front end; and finally the remainder of the
> changes to the Objective C front end.  You are expected to test,
> submit for approval, and (if approved) commit each of these patches
> independently.

This is the part that I find problematic. :-(  The work contained in the
two patches I posted last night, in addition to a couple of patches I
committed previously (and a few more I have yet to offer) is all part
of my ObjC++ integration (approved by the Steering Committee for 3.5
integration).  Furthermore, these bits already live in
objc-improvements-branch, available for the bootstrapping pleasure of 
all.

Therefore, I would kindly request that my ongoing work be treated as a
branch integration project.  Breaking this patch up into numerous
tiny pieces that when put together will reconstitute the original patch
does not offer any benefits that I can think of, and does in fact have
two major drawbacks: (1) it takes a lot more time and (2) it is more
susceptible to pilot error due to its fragmented nature.

Mark, it would be very helpful if you could opine as to how I should
proceed.  Of course, all comments and criticisms (such as those below)
are more than welcome, and I shall address them.  I'd just just that
I'd much prefer to (continue to) receive such comments regarding
the patches as I posted them. :-)

Now on to Zack's nits:

> > +/****** OBJECTIVE-C / OBJECTIVE-C++ ENTRY POINTS ******/
>
> No SCREAMING, no extra asterisks:
>
>   /* Objective-C and/or Objective-C++ entry points.  */

Ok; didn't mean to yell at anyone. :-)
>
> > +/* The following ObjC/ObjC++ functions are called by the C and/or 
> C++
> > +   front-ends; they all must have corresponding stubs in 
> stub-objc.c.
> > */
>
> Your mailer is still word-wrapping patches.  Fix it.

Hm... it looks fine from here; perhaps I shall explicitly attach patches
instead of pasting them into the window from now on. :-)

>
> > @@ -6662,6 +6664,12 @@ build_cdtor (int method_type, tree cdtor
> >  {
> >    tree body = 0;
> >
> > +  /* The Objective-C metadata initializer (if any) must be run
> > +     _before_ all other static constructors.  */
> > +  if (c_dialect_objc () && (method_type == 'I')
> > +      && objc_static_init_needed_p ())
> > +    cdtors = objc_generate_static_init_call (cdtors);
> > +
> >    if (!cdtors)
> >      return;
>
> This chunk of logic belongs in the caller of build_cdtor.  It can be
> expressed much more cleanly there.

Ok; I'll take a look.

> > -  if (!objc_is_public (datum, component))
> > +  if (c_dialect_objc () && !objc_is_public (datum, component))
>
> Most objc_* functions are designed not to need guarding with
> c_dialect_objc(), therefore I think it best if all of them don't; in
> other words, please cause objc_is_public to always return true when
> !c_dialect_objc, so this change will be unnecessary.

Again, perhaps Mark can issue a ruling here.  I thought that 
c_dialect_objc()
should be used because (1) it offers a clear demarcation point (i.e., 
"this is
ObjC-specific functionality") and (2) it improves performance (checking 
a bit
is a lot quicker than calling a function).

>
> > -  {".i", "@cpp-output", 0, 1, 0},
> > -  {"@cpp-output",
> > +  {".i", "@c-cpp-output", 0, 1, 0},
> > +  {"@c-cpp-output",
>
> This is a gratuitous change to the user interface which may break
> scripts.  You may not make this change.  (You may add -x c-cpp-output,
> but -x cpp-output must continue to have its existing meaning.)

Yes, you're right.  Truth be told, this is the one piece I actually
_should_ yank out my patch, since it is a logically separate animal.

Thanks,

--Zem
--------------------------------------------------------------
Ziemowit Laski                 1 Infinite Loop, MS 301-2K
Mac OS X Compiler Group        Cupertino, CA USA  95014-2083
Apple Computer, Inc.           +1.408.974.6229  Fax .5477

Re:

[Zack Weinberg]

Ziemowit Laski <zlaski@apple.com> writes:

>> You need to explain, in detail, what each change does and why.
>>
>> You have broken up this patch along the wrong lines.  Never break a
>> patch into pieces which could not be committed independently.  Changes
>
> The reason I broke the patch up is simply to ease the review
> process, since I can take maintainer responsibility for a large
> chunk of it.  I apologize if this is confusing; this _is_ really all
> one giant patch.

I realize that it's one giant patch and you broke it up to ease the
review process.  However, you chose to break it along lines which made
it _harder_ to review, because they didn't divide the patch into
independently reviewable changes.  In fact, you separated components
of the same change, such that I would have had to read both part A and
part B in order to review part A completely.

...
> Therefore, I would kindly request that my ongoing work be treated as
> a branch integration project.  Breaking this patch up into numerous
> tiny pieces that when put together will reconstitute the original
> patch does not offer any benefits that I can think of, and does in
> fact have two major drawbacks: (1) it takes a lot more time and (2)
> it is more susceptible to pilot error due to its fragmented nature.

Being a branch integration project does _not_ excuse you from breaking
up the patch into independent least-change units.  Look at the way the
LNO branch merge is being handled.

I realize that this makes substantially more work for you, but please
understand that there is no way we can review the patch as you posted
it.  It's just too hard to understand.  The chance of pilot error goes
up, yes, but that is overshadowed by the reduced chance that we will
miss something in our review, and the increased easiness of correcting
a small patch if a problem is discovered after it's committed.

zw

Re:

[Mark Mitchell]

> This is the part that I find problematic. :-(  The work contained in the
> two patches I posted last night, in addition to a couple of patches I
> committed previously (and a few more I have yet to offer) is all part
> of my ObjC++ integration (approved by the Steering Committee for 3.5
> integration).  Furthermore, these bits already live in
> objc-improvements-branch, available for the bootstrapping pleasure of all.

I think that it's up to you to decide to take Zack's advice, or not. 
Merges from a branch still need to be reviewed by someone with 
appropriate write privileges.  I think that, given Zack's message, he'll 
not be inclined to review your patch in its current state.  I think that 
the reason he's asking you to break it up is so that it is easier to 
review.  If someone else will review it as it stands, then you don't 
need to break it up.

You might want to try to make a deal with Zack whereby he will commit to 
reviewing the patches, if you will break them up, so that you know that 
you will at least be assured of getting a review if you do the work of 
breaking it up.  I have no idea whether Zack would agree to that deal or 
not, though; that is up to him.

>> > -  if (!objc_is_public (datum, component))
>> > +  if (c_dialect_objc () && !objc_is_public (datum, component))
>>
>> Most objc_* functions are designed not to need guarding with
>> c_dialect_objc(), therefore I think it best if all of them don't; in
>> other words, please cause objc_is_public to always return true when
>> !c_dialect_objc, so this change will be unnecessary.
> 
> 
> Again, perhaps Mark can issue a ruling here.  

The C front end maintainers are probably the best people to ask for a 
final decision.  I don't really know the C front end well enough to know 
what the historical decisions about style have been, or how strongly 
people think we should continue to follow the historical precedent.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com

Re:

[Zack Weinberg]

Mark Mitchell <mark@codesourcery.com> writes:

> You might want to try to make a deal with Zack whereby he will commit to 
> reviewing the patches, if you will break them up, so that you know that 
> you will at least be assured of getting a review if you do the work of 
> breaking it up.  I have no idea whether Zack would agree to that deal or 
> not, though; that is up to him.

Part of why I'm asking for it to be broken up is, I feel competent to
review changes to the C front end, but not to expr.c.  I am willing to
commit to reviewing changes that touch only the C front end, if
properly broken up and explained.

zw

Re: Re:

[Ziemowit Laski]

On 16 Aug, 2004, at 13.34, Zack Weinberg wrote:

> Mark Mitchell <mark@codesourcery.com> writes:
>
>> You might want to try to make a deal with Zack whereby he will commit 
>> to
>> reviewing the patches, if you will break them up, so that you know 
>> that
>> you will at least be assured of getting a review if you do the work of
>> breaking it up.  I have no idea whether Zack would agree to that deal 
>> or
>> not, though; that is up to him.
>
> Part of why I'm asking for it to be broken up is, I feel competent to
> review changes to the C front end, but not to expr.c.  I am willing to
> commit to reviewing changes that touch only the C front end, if
> properly broken up and explained.

OK, I'll try and separate that out.  Can anyone commit to review the 
generic changes (gcc.c, expr.c, gengtype.c)
once I've isolated those? :-)

Thank you,

--Zem
--------------------------------------------------------------
Ziemowit Laski                 1 Infinite Loop, MS 301-2K
Mac OS X Compiler Group        Cupertino, CA USA  95014-2083
Apple Computer, Inc.           +1.408.974.6229  Fax .5477

Re:

[Geoffrey Keating]

Ziemowit Laski <zlaski@apple.com> writes:

> On 16 Aug, 2004, at 13.34, Zack Weinberg wrote:
> 
> > Mark Mitchell <mark@codesourcery.com> writes:
> >
> >> You might want to try to make a deal with Zack whereby he will
> >> commit to
> >> reviewing the patches, if you will break them up, so that you know
> >> that
> >> you will at least be assured of getting a review if you do the work of
> >> breaking it up.  I have no idea whether Zack would agree to that
> >> deal or
> >> not, though; that is up to him.
> >
> > Part of why I'm asking for it to be broken up is, I feel competent to
> > review changes to the C front end, but not to expr.c.  I am willing to
> > commit to reviewing changes that touch only the C front end, if
> > properly broken up and explained.
> 
> OK, I'll try and separate that out.  Can anyone commit to review the
> generic changes (gcc.c, expr.c, gengtype.c)
> once I've isolated those? :-)

Yes, I'll do that.

Re:

[Richard Henderson]

On Mon, Aug 16, 2004 at 12:07:50PM -0700, Ziemowit Laski wrote:
> Again, perhaps Mark can issue a ruling here.  I thought that 
> c_dialect_objc()> should be used because (1) it offers a clear
> demarcation point (i.e., "this is ObjC-specific functionality")

I think "objc_is_foo" already does that.

> and (2) it improves performance (checking a bit
> is a lot quicker than calling a function).

Premature optimization is the root of all evil.


r~

Re:

[Joseph S. Myers]

On Mon, 16 Aug 2004, Ziemowit Laski wrote:

> This is the part that I find problematic. :-(  The work contained in the
> two patches I posted last night, in addition to a couple of patches I
> committed previously (and a few more I have yet to offer) is all part
> of my ObjC++ integration (approved by the Steering Committee for 3.5
> integration).  Furthermore, these bits already live in
> objc-improvements-branch, available for the bootstrapping pleasure of all.

If you don't like the proposed split, one you could try would be 
separating out the patch to stop the ObjC front end depending on 
particular structures for declarators, which is the part of the merge 
currently blocking a replacement of those structures.

I have looked at the C front end changes and believe that they only affect 
ObjC, and have no comments on them beyond those Zack has made, but don't 
think changes only affecting ObjC are for me to review.

> > Most objc_* functions are designed not to need guarding with
> > c_dialect_objc(), therefore I think it best if all of them don't; in
> > other words, please cause objc_is_public to always return true when
> > !c_dialect_objc, so this change will be unnecessary.
> 
> Again, perhaps Mark can issue a ruling here.  I thought that c_dialect_objc()
> should be used because (1) it offers a clear demarcation point (i.e., "this is
> ObjC-specific functionality") and (2) it improves performance (checking a bit
> is a lot quicker than calling a function).

Where is the evidence that this is, or is likely to be, of any measurable 
performance difference, compiling any source code whatever?  It looks like 
premature micro-optimization without such evidence.

-- 
Joseph S. Myers               http://www.srcf.ucam.org/~jsm28/gcc/
    jsm@polyomino.org.uk (personal mail)
    jsm28@gcc.gnu.org (Bugzilla assignments and CCs)

Re: Re:

[Ziemowit Laski]

On 16 Aug, 2004, at 13.53, Joseph S. Myers wrote:

> On Mon, 16 Aug 2004, Ziemowit Laski wrote:
>
>> This is the part that I find problematic. :-(  The work contained in 
>> the
>> two patches I posted last night, in addition to a couple of patches I
>> committed previously (and a few more I have yet to offer) is all part
>> of my ObjC++ integration (approved by the Steering Committee for 3.5
>> integration).  Furthermore, these bits already live in
>> objc-improvements-branch, available for the bootstrapping pleasure of 
>> all.
>
> If you don't like the proposed split, one you could try would be
> separating out the patch to stop the ObjC front end depending on
> particular structures for declarators, which is the part of the merge
> currently blocking a replacement of those structures.

Yeah, I've been sitting here and ruminating about how to break up this
behemoth.  As you may imagine, the various features are not only deeply
intertwined, but have not ever had a separate existence, since all of
this stuff was being worked on simultaneously on 
objc-improvements-branch.... :-(

> I have looked at the C front end changes and believe that they only 
> affect
> ObjC, and have no comments on them beyond those Zack has made, but 
> don't
> think changes only affecting ObjC are for me to review.

Does this mean that, after I address the issues raised by Zack, my 
patch would be OK to commit? :-)
Not entirely sure how to parse your message here...
>
> Where is the evidence that this is, or is likely to be, of any 
> measurable
> performance difference, compiling any source code whatever?  It looks 
> like
> premature micro-optimization without such evidence.

I can certainly remove the c_dialect_objc() conjunct if you like.

Thanks,

--Zem
--------------------------------------------------------------
Ziemowit Laski                 1 Infinite Loop, MS 301-2K
Mac OS X Compiler Group        Cupertino, CA USA  95014-2083
Apple Computer, Inc.           +1.408.974.6229  Fax .5477

Re: Re:

[Joseph S. Myers]

On Mon, 16 Aug 2004, Ziemowit Laski wrote:

> > I have looked at the C front end changes and believe that they only affect
> > ObjC, and have no comments on them beyond those Zack has made, but don't
> > think changes only affecting ObjC are for me to review.
> 
> Does this mean that, after I address the issues raised by Zack, my patch would
> be OK to commit? :-)

No, it means that my message should not be taken as a review of this 
patch, as I consider changes that are physically in C front end files but 
are purely relevant for ObjC to be outside C front end maintainership.  
This is not a judgement on whether they are within ObjC front end 
maintainership, as the boundaries are ill-defined.  In any case there are 
the changes not in either front end, and maintainers will generally seek 
consensus on changes likely to be controversial or to which objections 
have been raised rather than just checking them in.

-- 
Joseph S. Myers               http://www.srcf.ucam.org/~jsm28/gcc/
    jsm@polyomino.org.uk (personal mail)
    jsm28@gcc.gnu.org (Bugzilla assignments and CCs)

Re: Re: Re:

[Ziemowit Laski]

On 16 Aug, 2004, at 14.33, Joseph S. Myers wrote:

> On Mon, 16 Aug 2004, Ziemowit Laski wrote:
>
>>> I have looked at the C front end changes and believe that they only 
>>> affect
>>> ObjC, and have no comments on them beyond those Zack has made, but 
>>> don't
>>> think changes only affecting ObjC are for me to review.
>>
>> Does this mean that, after I address the issues raised by Zack, my 
>> patch would
>> be OK to commit? :-)
>
> No, it means that my message should not be taken as a review of this
> patch, as I consider changes that are physically in C front end files 
> but
> are purely relevant for ObjC to be outside C front end maintainership.
> This is not a judgement on whether they are within ObjC front end
> maintainership, as the boundaries are ill-defined.  In any case there 
> are
> the changes not in either front end, and maintainers will generally 
> seek
> consensus on changes likely to be controversial or to which objections
> have been raised rather than just checking them in.

OK; thanks for the clarification.

--Zem
--------------------------------------------------------------
Ziemowit Laski                 1 Infinite Loop, MS 301-2K
Mac OS X Compiler Group        Cupertino, CA USA  95014-2083
Apple Computer, Inc.           +1.408.974.6229  Fax .5477

Re:

[Stan Shebs]

Ziemowit Laski wrote:

> On 16 Aug, 2004, at 13.53, Joseph S. Myers wrote:
>
>> On Mon, 16 Aug 2004, Ziemowit Laski wrote:
>>
>>> This is the part that I find problematic. :-(  The work contained in 
>>> the
>>> two patches I posted last night, in addition to a couple of patches I
>>> committed previously (and a few more I have yet to offer) is all part
>>> of my ObjC++ integration (approved by the Steering Committee for 3.5
>>> integration).  Furthermore, these bits already live in
>>> objc-improvements-branch, available for the bootstrapping pleasure 
>>> of all.
>>
>>
>> If you don't like the proposed split, one you could try would be
>> separating out the patch to stop the ObjC front end depending on
>> particular structures for declarators, which is the part of the merge
>> currently blocking a replacement of those structures.
>
>
> Yeah, I've been sitting here and ruminating about how to break up this
> behemoth.  As you may imagine, the various features are not only deeply
> intertwined, but have not ever had a separate existence, since all of
> this stuff was being worked on simultaneously on 
> objc-improvements-branch.... :-(

The hazards of branch development... I didn't see a whole lot of
mechanical-type changes myself, those ought to be easy (if annoying)
to undo. Temporary stubouts should be acceptable too, and you reduce
your risk of having to back out one of the series because some random
bootstrap breaks.

Look on the bright side, at least there are no mullahs issuing
fatwas - GCC development could advance to a whole new level of
excitement! ("Death to the ssa-loving gimplifiers! Tear out their rtls
and hit them with grokdeclarator!")

:-)

Stan

(no subject)

[Kito Cheng]

From: Kito Cheng <kito.cheng@sifive.com>

Reply-To:

Subject: [PATCH v1 0/4] RISC-V target attribute

In-Reply-To:

This patch set implement target attribute for RISC-V target, which is similar to other target like x86 or ARM, let user able to set some local setting per function without changing global settings.

We support arch, tune and cpu first, and we will support other target attribute later, this version DOES NOT include multi-version function support yet, that is future work, probably work for GCC 15.

The full proposal is put in RISC-V C-API document[1], which has discussed with RISC-V LLVM community, so we have consistent syntax and semantics. 

[1] https://github.com/riscv-non-isa/riscv-c-api-doc/pull/35

Re:

[Kito Cheng]

Ooop, I screwed up when writing my cover letter of the target
attribute patch set...

On Tue, Oct 3, 2023 at 5:10 PM Kito Cheng <kito.cheng@sifive.com> wrote:
>
> From: Kito Cheng <kito.cheng@sifive.com>
>
> Reply-To:
>
> Subject: [PATCH v1 0/4] RISC-V target attribute
>
> In-Reply-To:
>
> This patch set implement target attribute for RISC-V target, which is similar to other target like x86 or ARM, let user able to set some local setting per function without changing global settings.
>
> We support arch, tune and cpu first, and we will support other target attribute later, this version DOES NOT include multi-version function support yet, that is future work, probably work for GCC 15.
>
> The full proposal is put in RISC-V C-API document[1], which has discussed with RISC-V LLVM community, so we have consistent syntax and semantics.
>
> [1] https://github.com/riscv-non-isa/riscv-c-api-doc/pull/35
>
>

(no subject)

[Eddy Young Tie Yang]

From d57ac4f9a095a2f616863efd524ac2d87276becb Mon Sep 17 00:00:00 2001
From: Eddy Young <jeyoung@priscimon.com>
Date: Sun, 13 Aug 2023 19:59:12 +0100
Subject: [PATCH] gcc/reload.h: Change type of x_spill_indirect_levels

---
 ChangeLog    | 5 +++++
 gcc/reload.h | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 3dd1ce544af..442aa9192a9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2015-08-13 Eddy Young <jeyoung@priscimon.com>
+
+	* gcc/reload.h: Change type of x_spill_indirect_levels of struct
+	target_reload to support C++17 build.
+
 2015-06-23  Release Manager
 
 	* GCC 4.8.5 released.
diff --git a/gcc/reload.h b/gcc/reload.h
index 7a13ad30e82..1e94d8ea93b 100644
--- a/gcc/reload.h
+++ b/gcc/reload.h
@@ -166,7 +166,7 @@ struct target_reload {
      value indicates the level of indirect addressing supported, e.g., two
      means that (MEM (MEM (REG n))) is also valid if (REG n) does not get
      a hard register.  */
-  bool x_spill_indirect_levels;
+  unsigned char x_spill_indirect_levels;
 
   /* True if caller-save has been reinitialized.  */
   bool x_caller_save_initialized_p;
-- 
2.39.2

Re:

[Andrew Pinski]

On Sun, Aug 13, 2023 at 12:05 PM Eddy Young Tie Yang
<jeyoung@priscimon.com> wrote:
>
> From d57ac4f9a095a2f616863efd524ac2d87276becb Mon Sep 17 00:00:00 2001
> From: Eddy Young <jeyoung@priscimon.com>
> Date: Sun, 13 Aug 2023 19:59:12 +0100
> Subject: [PATCH] gcc/reload.h: Change type of x_spill_indirect_levels
>
> ---
>  ChangeLog    | 5 +++++
>  gcc/reload.h | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 3dd1ce544af..442aa9192a9 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2015-08-13 Eddy Young <jeyoung@priscimon.com>
> +
> +       * gcc/reload.h: Change type of x_spill_indirect_levels of struct
> +       target_reload to support C++17 build.

This was done back in d57c99458933 for GCC 6.
https://gcc.gnu.org/r6-535-gd57c99458933a2 .
Why are you posting a patch against a branch which has not been
supported for years now?

Thanks,
Andrew Pinski

> +
>  2015-06-23  Release Manager
>
>         * GCC 4.8.5 released.
> diff --git a/gcc/reload.h b/gcc/reload.h
> index 7a13ad30e82..1e94d8ea93b 100644
> --- a/gcc/reload.h
> +++ b/gcc/reload.h
> @@ -166,7 +166,7 @@ struct target_reload {
>       value indicates the level of indirect addressing supported, e.g., two
>       means that (MEM (MEM (REG n))) is also valid if (REG n) does not get
>       a hard register.  */
> -  bool x_spill_indirect_levels;
> +  unsigned char x_spill_indirect_levels;
>
>    /* True if caller-save has been reinitialized.  */
>    bool x_caller_save_initialized_p;
> --
> 2.39.2
>

Re:

[d-ni]

[-- Attachment #1: Type: text/plain, Size: 103 bytes --]

Hello i have a word to share with you , kindly contact :  mrslingli373@gmail.com        for more info

Re: [PATCH][i386] Split not+broadcast+pand to broadcast+pandn.

[Segher Boessenkool]

> 	PR target/100711
> 	* simplify-rtx.c (simplify_unary_operation_1):
> 	Simplify (vec_duplicate (not op)) to (not (vec_duplicate op)).

This is not a simplification.  If we want to do this we need to document
this canonicalisation (in md.texi, "Insn Canonicalizations").

> +    /* Prefer (not (vec_duplicate (nonimmedaite_operand)))
> +       to (vec_duplicate (not (nonimmedaite_operand))).  */

What Andrew said here (also, it's misspelled :-) )

> +    case VEC_DUPLICATE:
> +      if (GET_CODE (op) == NOT)
> +	return gen_rtx_NOT (mode, gen_rtx_VEC_DUPLICATE (mode, XEXP (op, 0)));
> +      break;

If it isn't a canonicalisation you need to simplify the result, and then
only do it if it does in fact simplify.  You risk "simplification" loops
if you don't.


Segher

(no subject)

[liuhongt]

This is the updated patch.

Re:

[Hongtao Liu]

Please discard this one, sorry for disturbing.
Obviously I'm new to git send-email.

On Wed, Jun 2, 2021 at 1:40 PM liuhongt via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This is the updated patch.
>
>


-- 
BR,
Hongtao

[PATCH RFC] bootstrap: Update requirement to C++11.

[Jason Merrill]

There seemed to be general agreement last November that we would move to
allowing C++11 features to be used in GCC 11; this patch implements that
direction.  Are changes needed anywhere else?

ChangeLog
2020-05-14  Jason Merrill  <jason@redhat.com>

	* configure.ac: Update bootstrap dialect to -std=gnu++11.

gcc/ChangeLog
2020-05-14  Jason Merrill  <jason@redhat.com>

	* doc/install.texi (Prerequisites): Update boostrap compiler
	requirement to C++11/GCC 4.8.
---
 gcc/doc/install.texi | 14 ++++++++------
 configure.ac         |  6 +++---
 ChangeLog            |  4 ++++
 configure            |  6 +++---
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 876b04f9c45..f47e3c76f73 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -238,15 +238,17 @@ described below.
 
 @heading Tools/packages necessary for building GCC
 @table @asis
-@item ISO C++98 compiler
-Necessary to bootstrap GCC, although versions of GCC prior
-to 4.8 also allow bootstrapping with a ISO C89 compiler and versions
-of GCC prior to 3.4 also allow bootstrapping with a traditional
-(K&R) C compiler.
+@item ISO C++11 compiler
+Necessary to bootstrap GCC.
+
+Versions of GCC prior to 11 also allow bootstrapping with an ISO C++98
+compiler, versions of GCC prior to 4.8 also allow bootstrapping with a
+ISO C89 compiler, and versions of GCC prior to 3.4 also allow
+bootstrapping with a traditional (K&R) C compiler.
 
 To build all languages in a cross-compiler or other configuration where
 3-stage bootstrap is not performed, you need to start with an existing
-GCC binary (version 3.4 or later) because source code for language
+GCC binary (version 4.8 or later) because source code for language
 frontends other than C might use GCC extensions.
 
 Note that to bootstrap GCC with versions of GCC earlier than 3.4, you
diff --git a/configure.ac b/configure.ac
index c78d9cbea62..63d92b73061 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1462,10 +1462,10 @@ case "$have_compiler:$host:$target:$enable_bootstrap" in
     ;;
 esac
 
-# When bootstrapping with GCC, build stage 1 in C++98 mode to ensure that a
-# C++98 compiler can still start the bootstrap.
+# When bootstrapping with GCC, build stage 1 in C++11 mode to ensure that a
+# C++11 compiler can still start the bootstrap.
 if test "$enable_bootstrap:$GXX" = "yes:yes"; then
-  CXX="$CXX -std=gnu++98"
+  CXX="$CXX -std=gnu++11"
 fi
 
 # Used for setting $lt_cv_objdir
diff --git a/ChangeLog b/ChangeLog
index a7fcf77b9b2..1d281855a3e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2020-05-14  Jason Merrill  <jason@redhat.com>
+
+	* configure.ac: Update bootstrap dialect to -std=gnu++11.
+
 2020-04-29  Thomas Schwinge  <thomas@codesourcery.com>
 
 	PR target/92713
diff --git a/configure b/configure
index 4cc938ebb7d..9b39035bbcc 100755
--- a/configure
+++ b/configure
@@ -5523,10 +5523,10 @@ $as_echo "$as_me: WARNING: trying to bootstrap a cross compiler" >&2;}
     ;;
 esac
 
-# When bootstrapping with GCC, build stage 1 in C++98 mode to ensure that a
-# C++98 compiler can still start the bootstrap.
+# When bootstrapping with GCC, build stage 1 in C++11 mode to ensure that a
+# C++11 compiler can still start the bootstrap.
 if test "$enable_bootstrap:$GXX" = "yes:yes"; then
-  CXX="$CXX -std=gnu++98"
+  CXX="$CXX -std=gnu++11"
 fi
 
 # Used for setting $lt_cv_objdir

base-commit: 4e1592f8e1d6366699e05c0824fc3dc39ca7314b
-- 
2.18.1

Re: [PATCH RFC] bootstrap: Update requirement to C++11.

[Richard Biener]

On Thu, May 14, 2020 at 11:53 PM Jason Merrill via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> There seemed to be general agreement last November that we would move to
> allowing C++11 features to be used in GCC 11; this patch implements that
> direction.  Are changes needed anywhere else?
>
> ChangeLog
> 2020-05-14  Jason Merrill  <jason@redhat.com>
>
>         * configure.ac: Update bootstrap dialect to -std=gnu++11.
>
> gcc/ChangeLog
> 2020-05-14  Jason Merrill  <jason@redhat.com>
>
>         * doc/install.texi (Prerequisites): Update boostrap compiler
>         requirement to C++11/GCC 4.8.
> ---
>  gcc/doc/install.texi | 14 ++++++++------
>  configure.ac         |  6 +++---
>  ChangeLog            |  4 ++++
>  configure            |  6 +++---
>  4 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
> index 876b04f9c45..f47e3c76f73 100644
> --- a/gcc/doc/install.texi
> +++ b/gcc/doc/install.texi
> @@ -238,15 +238,17 @@ described below.
>
>  @heading Tools/packages necessary for building GCC
>  @table @asis
> -@item ISO C++98 compiler
> -Necessary to bootstrap GCC, although versions of GCC prior
> -to 4.8 also allow bootstrapping with a ISO C89 compiler and versions
> -of GCC prior to 3.4 also allow bootstrapping with a traditional
> -(K&R) C compiler.
> +@item ISO C++11 compiler
> +Necessary to bootstrap GCC.
> +
> +Versions of GCC prior to 11 also allow bootstrapping with an ISO C++98
> +compiler, versions of GCC prior to 4.8 also allow bootstrapping with a
> +ISO C89 compiler, and versions of GCC prior to 3.4 also allow
> +bootstrapping with a traditional (K&R) C compiler.
>
>  To build all languages in a cross-compiler or other configuration where
>  3-stage bootstrap is not performed, you need to start with an existing
> -GCC binary (version 3.4 or later) because source code for language
> +GCC binary (version 4.8 or later) because source code for language
>  frontends other than C might use GCC extensions.
>
>  Note that to bootstrap GCC with versions of GCC earlier than 3.4, you
> diff --git a/configure.ac b/configure.ac
> index c78d9cbea62..63d92b73061 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1462,10 +1462,10 @@ case "$have_compiler:$host:$target:$enable_bootstrap" in
>      ;;
>  esac
>
> -# When bootstrapping with GCC, build stage 1 in C++98 mode to ensure that a
> -# C++98 compiler can still start the bootstrap.
> +# When bootstrapping with GCC, build stage 1 in C++11 mode to ensure that a
> +# C++11 compiler can still start the bootstrap.
>  if test "$enable_bootstrap:$GXX" = "yes:yes"; then
> -  CXX="$CXX -std=gnu++98"
> +  CXX="$CXX -std=gnu++11"

So I just spotted this - since we're requiring a ISO C++11 compiler shouldn't
we build stage1 with -std=c++11 rather than gnu++11 (whatever the detailed
differences are here)?  Also not sure what level of -pedantic we'd need to
avoid GNU extensions even with -std=c++11.  Of course there are (I hope)
a lot less GNU extensions for C++ than there were for C and hopefully
no extra in gnu++11 compared to gnu++98 which we checked previously.

Note I think what's missing is some general blurb in our coding conventions
as to how much of C++11 we are supposed to use in non-infrastructure parts
of GCC (I expect things like hash-table.h to use more C++ features than,
say, tree-ssa-alias.c).

There also does not seem to be a configure check which may present
users with a more useful error message than later cryptic fail of build?
I suppose we cannot simply check __cplusplus for this, can we?  Do
other common host compilers need additional options to enable C++11?
Should we try to second guess such flags via configury?  For example
GCC 4.8 defaults to -std=gnu++98 and the above only seems to apply
to the bootstrap case so GCC 4.8 cannot be used to build cross compilers
without adjusting CC and CXX?

Thanks,
Richard.

>  fi
>
>  # Used for setting $lt_cv_objdir
> diff --git a/ChangeLog b/ChangeLog
> index a7fcf77b9b2..1d281855a3e 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,7 @@
> +2020-05-14  Jason Merrill  <jason@redhat.com>
> +
> +       * configure.ac: Update bootstrap dialect to -std=gnu++11.
> +
>  2020-04-29  Thomas Schwinge  <thomas@codesourcery.com>
>
>         PR target/92713
> diff --git a/configure b/configure
> index 4cc938ebb7d..9b39035bbcc 100755
> --- a/configure
> +++ b/configure
> @@ -5523,10 +5523,10 @@ $as_echo "$as_me: WARNING: trying to bootstrap a cross compiler" >&2;}
>      ;;
>  esac
>
> -# When bootstrapping with GCC, build stage 1 in C++98 mode to ensure that a
> -# C++98 compiler can still start the bootstrap.
> +# When bootstrapping with GCC, build stage 1 in C++11 mode to ensure that a
> +# C++11 compiler can still start the bootstrap.
>  if test "$enable_bootstrap:$GXX" = "yes:yes"; then
> -  CXX="$CXX -std=gnu++98"
> +  CXX="$CXX -std=gnu++11"
>  fi
>
>  # Used for setting $lt_cv_objdir
>
> base-commit: 4e1592f8e1d6366699e05c0824fc3dc39ca7314b
> --
> 2.18.1
>

Re: [PATCH RFC] bootstrap: Update requirement to C++11.

[Richard Sandiford]

Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Note I think what's missing is some general blurb in our coding conventions
> as to how much of C++11 we are supposed to use in non-infrastructure parts
> of GCC (I expect things like hash-table.h to use more C++ features than,
> say, tree-ssa-alias.c).

I guess there are two aspects to that:

- Are there any specific features we should avoid using at all?
  TBH I hope not.  Trying to police this based on C++ feature sounds
  difficulty and might be counterproductive.

  IMO it should just (continue to) be based on principles like avoiding
  abstraction for abstraction's sake, and keeping compiler performance
  and code size in mind.  Even tree-ssa-alias.c should be able to use
  any C++ feature if there's a justification.

- Coding conventions for when features should be used.  "auto" is an
  obvious one.  Maybe also lambdas (which should help avoid the horrible
  "void *" callback parameters we have all over the place now).

Maybe also guidelines to actively use certain features, e.g.

- use "= default" where possible

- prefer range-based for loops to macros

- mark "operator bool()" conversions as explicit

- use "override" where applicable

(all obvious I guess), etc.

Thanks,
Richard

Re: [PATCH RFC] bootstrap: Update requirement to C++11.

[Richard Biener]

On Fri, May 15, 2020 at 10:30 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > Note I think what's missing is some general blurb in our coding conventions
> > as to how much of C++11 we are supposed to use in non-infrastructure parts
> > of GCC (I expect things like hash-table.h to use more C++ features than,
> > say, tree-ssa-alias.c).
>
> I guess there are two aspects to that:
>
> - Are there any specific features we should avoid using at all?
>   TBH I hope not.  Trying to police this based on C++ feature sounds
>   difficulty and might be counterproductive.
>
>   IMO it should just (continue to) be based on principles like avoiding
>   abstraction for abstraction's sake, and keeping compiler performance
>   and code size in mind.  Even tree-ssa-alias.c should be able to use
>   any C++ feature if there's a justification.
>
> - Coding conventions for when features should be used.  "auto" is an
>   obvious one.  Maybe also lambdas (which should help avoid the horrible
>   "void *" callback parameters we have all over the place now).
>
> Maybe also guidelines to actively use certain features, e.g.
>
> - use "= default" where possible
>
> - prefer range-based for loops to macros
>
> - mark "operator bool()" conversions as explicit
>
> - use "override" where applicable
>
> (all obvious I guess), etc.

I think the most important thing is that refactoring for the sake
of refactoring is bad iff it does not improve on consistency
throughout the code base.  We should really try hard to use
C++ features consistently - this makes the code base easier
to understand.

We've moved more and more to stronly-typed data structures
so I'd not like to see 'auto' everywhere - it should be still
obvious what kind of objects we're working with where they
matter.  IMHO they do not matter for example for iterators.
I don't care about the iterator type but about the type of
the object and the container.

Richard.

> Thanks,
> Richard

Re: [PATCH RFC] bootstrap: Update requirement to C++11.

[Richard Sandiford]

Richard Biener <richard.guenther@gmail.com> writes:
> On Fri, May 15, 2020 at 10:30 AM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> > Note I think what's missing is some general blurb in our coding conventions
>> > as to how much of C++11 we are supposed to use in non-infrastructure parts
>> > of GCC (I expect things like hash-table.h to use more C++ features than,
>> > say, tree-ssa-alias.c).
>>
>> I guess there are two aspects to that:
>>
>> - Are there any specific features we should avoid using at all?
>>   TBH I hope not.  Trying to police this based on C++ feature sounds
>>   difficulty and might be counterproductive.
>>
>>   IMO it should just (continue to) be based on principles like avoiding
>>   abstraction for abstraction's sake, and keeping compiler performance
>>   and code size in mind.  Even tree-ssa-alias.c should be able to use
>>   any C++ feature if there's a justification.
>>
>> - Coding conventions for when features should be used.  "auto" is an
>>   obvious one.  Maybe also lambdas (which should help avoid the horrible
>>   "void *" callback parameters we have all over the place now).
>>
>> Maybe also guidelines to actively use certain features, e.g.
>>
>> - use "= default" where possible
>>
>> - prefer range-based for loops to macros
>>
>> - mark "operator bool()" conversions as explicit
>>
>> - use "override" where applicable
>>
>> (all obvious I guess), etc.
>
> I think the most important thing is that refactoring for the sake
> of refactoring is bad iff it does not improve on consistency
> throughout the code base.  We should really try hard to use
> C++ features consistently - this makes the code base easier
> to understand.

Agreed.  One of the reasons I'm keen to have something in the
coding standards is that things became less consistent after
the C->C++ switch.

Maybe we should reconsider some of the existing coding standards too.
E.g.:

  Define all members outside the class definition.
  That is, there are no function bodies or member initializers
  inside the class definition.

isn't widely followed.  That's a bit of a tangent though.
(FTR, I've no attachment to the current conventions and didn't
contribute anything to them.  More consistency would be good though.)

> We've moved more and more to stronly-typed data structures
> so I'd not like to see 'auto' everywhere - it should be still
> obvious what kind of objects we're working with where they
> matter.  IMHO they do not matter for example for iterators.
> I don't care about the iterator type but about the type of
> the object and the container.

Also agreed. :-)  How about this as a starting point:

---------------------------------------------------------------
Use auto for:

- the result of casts or other expressions that give the type
  explicitly.  E.g.:

    if (auto *table = dyn_cast <rtx_jump_table_data *> (insn))

  instead of:

    if (rtx_jump_table_data *table = dyn_cast <rtx_jump_table_data *> (insn))

- iterator types.  E.g.:

    auto it = foo.begin ();

  instead of:

    foo_type::iterator it = foo.begin ();

- expressions that provide an alternative view of something,
  when the expression is bound to a read-only temporary.  E.g.:

    auto val1 = wi::to_wide (...);
    auto val2 = wi::uhwi (12, 16);

  instead of:

    wide_int val1 = wi::to_wide (...);
    wide_int val2 = wi::uhwi (12, 16);

  (Using "wide_int" is less efficient than using the natural type of
  the expression.)

- the type of a lambda expression.  E.g.:

    auto f = [] (int x) { return x + 1; };

Do not use auto otherwise.
---------------------------------------------------------------

One thing I wondered about is whether we should encourage auto
for normal integer results.  It would avoid problems with values
being silently truncated to a narrower type (e.g. HOST_WIDE_INT->int).
On the other hand, that kind of truncation will still happen in things
like function calls and member variable assignments.  Using "auto" could
also hide important signedness choices.  So not using "auto" is probably
better there...

Richard

Re: [PATCH RFC] bootstrap: Update requirement to C++11.

[Richard Biener]

On Fri, May 15, 2020 at 11:58 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Richard Biener <richard.guenther@gmail.com> writes:
> > On Fri, May 15, 2020 at 10:30 AM Richard Sandiford
> > <richard.sandiford@arm.com> wrote:
> >>
> >> Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> >> > Note I think what's missing is some general blurb in our coding conventions
> >> > as to how much of C++11 we are supposed to use in non-infrastructure parts
> >> > of GCC (I expect things like hash-table.h to use more C++ features than,
> >> > say, tree-ssa-alias.c).
> >>
> >> I guess there are two aspects to that:
> >>
> >> - Are there any specific features we should avoid using at all?
> >>   TBH I hope not.  Trying to police this based on C++ feature sounds
> >>   difficulty and might be counterproductive.
> >>
> >>   IMO it should just (continue to) be based on principles like avoiding
> >>   abstraction for abstraction's sake, and keeping compiler performance
> >>   and code size in mind.  Even tree-ssa-alias.c should be able to use
> >>   any C++ feature if there's a justification.
> >>
> >> - Coding conventions for when features should be used.  "auto" is an
> >>   obvious one.  Maybe also lambdas (which should help avoid the horrible
> >>   "void *" callback parameters we have all over the place now).
> >>
> >> Maybe also guidelines to actively use certain features, e.g.
> >>
> >> - use "= default" where possible
> >>
> >> - prefer range-based for loops to macros
> >>
> >> - mark "operator bool()" conversions as explicit
> >>
> >> - use "override" where applicable
> >>
> >> (all obvious I guess), etc.
> >
> > I think the most important thing is that refactoring for the sake
> > of refactoring is bad iff it does not improve on consistency
> > throughout the code base.  We should really try hard to use
> > C++ features consistently - this makes the code base easier
> > to understand.
>
> Agreed.  One of the reasons I'm keen to have something in the
> coding standards is that things became less consistent after
> the C->C++ switch.

Yeah.  I'm really hoping that we can unify all of the various iteration
styles used.  In general, even if I don't like it too much personally,
I'm leaning towards STL style of thing because of the idea
"C++ attracts more developers".  Now C++ range-for makes me
like it much more so that's one good reason for adopting C++11,
turn all of them over to range-for style (only!).

> Maybe we should reconsider some of the existing coding standards too.
> E.g.:
>
>   Define all members outside the class definition.
>   That is, there are no function bodies or member initializers
>   inside the class definition.
>
> isn't widely followed.  That's a bit of a tangent though.
> (FTR, I've no attachment to the current conventions and didn't
> contribute anything to them.  More consistency would be good though.)
>
> > We've moved more and more to stronly-typed data structures
> > so I'd not like to see 'auto' everywhere - it should be still
> > obvious what kind of objects we're working with where they
> > matter.  IMHO they do not matter for example for iterators.
> > I don't care about the iterator type but about the type of
> > the object and the container.
>
> Also agreed. :-)  How about this as a starting point:
>
> ---------------------------------------------------------------
> Use auto for:
>
> - the result of casts or other expressions that give the type
>   explicitly.  E.g.:
>
>     if (auto *table = dyn_cast <rtx_jump_table_data *> (insn))
>
>   instead of:
>
>     if (rtx_jump_table_data *table = dyn_cast <rtx_jump_table_data *> (insn))
>
> - iterator types.  E.g.:
>
>     auto it = foo.begin ();
>
>   instead of:
>
>     foo_type::iterator it = foo.begin ();
>
> - expressions that provide an alternative view of something,
>   when the expression is bound to a read-only temporary.  E.g.:
>
>     auto val1 = wi::to_wide (...);
>     auto val2 = wi::uhwi (12, 16);
>
>   instead of:
>
>     wide_int val1 = wi::to_wide (...);
>     wide_int val2 = wi::uhwi (12, 16);
>
>   (Using "wide_int" is less efficient than using the natural type of
>   the expression.)
>
> - the type of a lambda expression.  E.g.:
>
>     auto f = [] (int x) { return x + 1; };

Those are all good examples.  Mind putting that into a patch
for the coding conventions?

> Do not use auto otherwise.
> ---------------------------------------------------------------
>
> One thing I wondered about is whether we should encourage auto
> for normal integer results.  It would avoid problems with values
> being silently truncated to a narrower type (e.g. HOST_WIDE_INT->int).
> On the other hand, that kind of truncation will still happen in things
> like function calls and member variable assignments.  Using "auto" could
> also hide important signedness choices.  So not using "auto" is probably
> better there...

Agreed.

Richard.

> Richard

(no subject)

[Richard Sandiford]

[-- Attachment #1: Type: text/plain, Size: 1706 bytes --]

>> > We've moved more and more to stronly-typed data structures
>> > so I'd not like to see 'auto' everywhere - it should be still
>> > obvious what kind of objects we're working with where they
>> > matter.  IMHO they do not matter for example for iterators.
>> > I don't care about the iterator type but about the type of
>> > the object and the container.
>>
>> Also agreed. :-)  How about this as a starting point:
>>
>> ---------------------------------------------------------------
>> Use auto for:
>>
>> - the result of casts or other expressions that give the type
>>   explicitly.  E.g.:
>>
>>     if (auto *table = dyn_cast <rtx_jump_table_data *> (insn))
>>
>>   instead of:
>>
>>     if (rtx_jump_table_data *table = dyn_cast <rtx_jump_table_data *> (insn))
>>
>> - iterator types.  E.g.:
>>
>>     auto it = foo.begin ();
>>
>>   instead of:
>>
>>     foo_type::iterator it = foo.begin ();
>>
>> - expressions that provide an alternative view of something,
>>   when the expression is bound to a read-only temporary.  E.g.:
>>
>>     auto val1 = wi::to_wide (...);
>>     auto val2 = wi::uhwi (12, 16);
>>
>>   instead of:
>>
>>     wide_int val1 = wi::to_wide (...);
>>     wide_int val2 = wi::uhwi (12, 16);
>>
>>   (Using "wide_int" is less efficient than using the natural type of
>>   the expression.)
>>
>> - the type of a lambda expression.  E.g.:
>>
>>     auto f = [] (int x) { return x + 1; };
>
> Those are all good examples.  Mind putting that into a patch
> for the coding conventions?

How's this?  I added "new" expressions as another example of the
first category.

I'm sure I've missed other good uses, but we can always add to the
list later if necessary.

Thanks,
Richard


[-- Attachment #2: 0001-Describe-coding-conventions-surrounding-auto.patch --]
[-- Type: text/plain, Size: 4621 bytes --]

From 10b27e367de0fa9d5bf91544385401cdcbdb8c00 Mon Sep 17 00:00:00 2001
From: Richard Sandiford <richard.sandiford@arm.com>
Date: Fri, 15 May 2020 14:58:46 +0100
Subject: [PATCH] Describe coding conventions surrounding "auto"

---
 htdocs/codingconventions.html | 53 +++++++++++++++++++++++++++++++++++
 htdocs/codingrationale.html   | 17 +++++++++++
 2 files changed, 70 insertions(+)

diff --git a/htdocs/codingconventions.html b/htdocs/codingconventions.html
index f4732ef6..ae49fb91 100644
--- a/htdocs/codingconventions.html
+++ b/htdocs/codingconventions.html
@@ -51,6 +51,7 @@ the conventions separately from any other changes to the code.</p>
     <li><a href="#Cxx_Language">Language Use</a>
         <ul>
         <li><a href="#Variable">Variable Definitions</a></li>
+        <li><a href="#Auto">Use of <code>auto</code></a></li>
         <li><a href="#Struct_Use">Struct Definitions</a></li>
         <li><a href="#Class_Use">Class Definitions</a></li>
         <li><a href="#Constructors">Constructors and Destructors</a></li>
@@ -884,6 +885,58 @@ Variables may be simultaneously defined and tested in control expressions.
 <a href="codingrationale.html#variables">Rationale and Discussion</a>
 </p>
 
+<h4 id="Auto">Use of <code>auto</code></h4>
+
+<p><code>auto</code> should be used in the following circumstances:
+<ul>
+  <li><p>when the expression gives the C++ type explicitly.  For example</p>
+
+    <blockquote>
+<pre>if (<b>auto *</b>table = dyn_cast &lt;<b>rtx_jump_table_data *</b>&gt; (insn))                 // OK
+  ...
+if (rtx_jump_table_data *table = dyn_cast &lt;rtx_jump_table_data *&gt; (insn))  // Avoid
+  ...
+<b>auto *</b>map = new <b>hash_map &lt;tree, size_t&gt;</b>;                    // OK
+hash_map &lt;tree, size_t&gt; *map = new hash_map &lt;tree, size_t&gt;; // Avoid</pre></blockquote>
+
+    <p>This rule does not apply to abbreviated type names embedded in
+    an identifier, such as the result of <code>tree_to_shwi</code>.</p>
+  </li>
+  <li>
+    <p>when the expression simply provides an alternative view of an object
+    and is bound to a read-only temporary.  For example:</p>
+
+    <blockquote>
+<pre><b>auto</b> wioff = <b>wi::to_wide (off);</b>         // OK
+wide_int wioff = wi::to_wide (off);     // Avoid if wioff is read-only
+<b>auto</b> minus1 = <b>std::shwi (-1, prec);</b>     // OK
+wide_int minus1 = std::shwi (-1, prec); // Avoid if minus1 is read-only</pre></blockquote>
+
+    <p>In principle this rule applies to other views of an object too,
+    such as a reversed view of a list, or a sequential view of a
+    <code>hash_set</code>.  It does not apply to general temporaries.</p>
+  </li>
+  <li>
+    <p>the type of an iterator.  For example:</p>
+
+    <blockquote>
+<pre><b>auto</b> it = <b>std::find (names.begin (), names.end (), needle)</b>;        // OK
+vector &lt;name_map&gt;::iterator it = std::find (names.begin (),
+                                            names.end (), needle); // Avoid</pre></blockquote>
+  </li>
+  <li>
+    <p>the type of a lambda expression.  For example:</p>
+
+    <blockquote>
+<pre><b>auto</b> f = <b>[] (int x) { return x + 1; }</b>; // OK</pre></blockquote>
+  </li>
+</ul></p>
+
+<p><code>auto</code> should <b>not</b> be used in other contexts.</p>
+
+<p>
+<a href="codingrationale.html#auto">Rationale and Discussion</a>
+</p>
 
 <h4 id="Struct_Use">Struct Definitions</h4>
 
diff --git a/htdocs/codingrationale.html b/htdocs/codingrationale.html
index 0b44f1da..a919023c 100644
--- a/htdocs/codingrationale.html
+++ b/htdocs/codingrationale.html
@@ -50,6 +50,23 @@ if (info *q = get_any_available_info ()) {
 }
 </code></pre></blockquote>
 
+<h4 id="auto">Use of <code>auto</code></h4>
+
+<p>The reason for preferring <code>auto</code> in expressions like:
+<blockquote><pre>auto wioff = wi::to_wide (off);</pre></blockquote>
+is that using the natural type of the expression is more efficient than
+converting it to types like <code>wide_int</code>.</p>
+
+<p>The reason for excluding other uses of <code>auto</code> is that
+in most other cases the type carries useful information.  For example:
+<blockquote><pre>for (const std::pair &lt;const char *, tree&gt; &amp;elt : indirect_pool)
+  ...</pre></blockquote>
+makes it obvious that <code>elt</code> is a pair and gives the types of
+<code>elt.first</code> and <code>elt.second</code>.  In contrast:
+<blockquote><pre>for (const auto &amp;elt : indirect_pool)
+  ...</pre></blockquote>
+gives no immediate indication what <code>elt</code> is or what can
+be done with it.</p>
 
 <h4 id="struct">Struct Definitions</h4>
 
-- 
2.17.1

Re:

[Martin Sebor]

On 5/15/20 8:08 AM, Richard Sandiford wrote:
>>>> We've moved more and more to stronly-typed data structures
>>>> so I'd not like to see 'auto' everywhere - it should be still
>>>> obvious what kind of objects we're working with where they
>>>> matter.  IMHO they do not matter for example for iterators.
>>>> I don't care about the iterator type but about the type of
>>>> the object and the container.
>>> Also agreed. :-)  How about this as a starting point:
>>>
>>> ---------------------------------------------------------------
>>> Use auto for:
>>>
>>> - the result of casts or other expressions that give the type
>>>    explicitly.  E.g.:
>>>
>>>      if (auto *table = dyn_cast <rtx_jump_table_data *> (insn))
>>>
>>>    instead of:
>>>
>>>      if (rtx_jump_table_data *table = dyn_cast <rtx_jump_table_data *> (insn))
>>>
>>> - iterator types.  E.g.:
>>>
>>>      auto it = foo.begin ();
>>>
>>>    instead of:
>>>
>>>      foo_type::iterator it = foo.begin ();
>>>
>>> - expressions that provide an alternative view of something,
>>>    when the expression is bound to a read-only temporary.  E.g.:
>>>
>>>      auto val1 = wi::to_wide (...);
>>>      auto val2 = wi::uhwi (12, 16);
>>>
>>>    instead of:
>>>
>>>      wide_int val1 = wi::to_wide (...);
>>>      wide_int val2 = wi::uhwi (12, 16);
>>>
>>>    (Using "wide_int" is less efficient than using the natural type of
>>>    the expression.)
>>>
>>> - the type of a lambda expression.  E.g.:
>>>
>>>      auto f = [] (int x) { return x + 1; };
>> Those are all good examples.  Mind putting that into a patch
>> for the coding conventions?
> How's this?  I added "new" expressions as another example of the
> first category.
> 
> I'm sure I've missed other good uses, but we can always add to the
> list later if necessary.
> 
> Thanks,
> Richard
> 
> 
> 0001-Describe-coding-conventions-surrounding-auto.patch
> 
>  From 10b27e367de0fa9d5bf91544385401cdcbdb8c00 Mon Sep 17 00:00:00 2001
> From: Richard Sandiford<richard.sandiford@arm.com>
> Date: Fri, 15 May 2020 14:58:46 +0100
> Subject: [PATCH] Describe coding conventions surrounding "auto"
> 
> ---
>   htdocs/codingconventions.html | 53 +++++++++++++++++++++++++++++++++++
>   htdocs/codingrationale.html   | 17 +++++++++++
>   2 files changed, 70 insertions(+)
> 
> diff --git a/htdocs/codingconventions.html b/htdocs/codingconventions.html
> index f4732ef6..ae49fb91 100644
> --- a/htdocs/codingconventions.html
> +++ b/htdocs/codingconventions.html
> @@ -51,6 +51,7 @@ the conventions separately from any other changes to the code.</p>
>       <li><a href="#Cxx_Language">Language Use</a>
>           <ul>
>           <li><a href="#Variable">Variable Definitions</a></li>
> +        <li><a href="#Auto">Use of <code>auto</code></a></li>
>           <li><a href="#Struct_Use">Struct Definitions</a></li>
>           <li><a href="#Class_Use">Class Definitions</a></li>
>           <li><a href="#Constructors">Constructors and Destructors</a></li>
> @@ -884,6 +885,58 @@ Variables may be simultaneously defined and tested in control expressions.
>   <a href="codingrationale.html#variables">Rationale and Discussion</a>
>   </p>
>   
> +<h4 id="Auto">Use of <code>auto</code></h4>
> +
> +<p><code>auto</code> should be used in the following circumstances:
> +<ul>
> +  <li><p>when the expression gives the C++ type explicitly.  For example</p>
> +
> +    <blockquote>
> +<pre>if (<b>auto *</b>table = dyn_cast &lt;<b>rtx_jump_table_data *</b>&gt; (insn))                 // OK
> +  ...
> +if (rtx_jump_table_data *table = dyn_cast &lt;rtx_jump_table_data *&gt; (insn))  // Avoid
> +  ...
> +<b>auto *</b>map = new <b>hash_map &lt;tree, size_t&gt;</b>;                    // OK
> +hash_map &lt;tree, size_t&gt; *map = new hash_map &lt;tree, size_t&gt;; // Avoid</pre></blockquote>
> +
> +    <p>This rule does not apply to abbreviated type names embedded in
> +    an identifier, such as the result of <code>tree_to_shwi</code>.</p>
> +  </li>
> +  <li>
> +    <p>when the expression simply provides an alternative view of an object
> +    and is bound to a read-only temporary.  For example:</p>
> +
> +    <blockquote>
> +<pre><b>auto</b> wioff = <b>wi::to_wide (off);</b>         // OK
> +wide_int wioff = wi::to_wide (off);     // Avoid if wioff is read-only
> +<b>auto</b> minus1 = <b>std::shwi (-1, prec);</b>     // OK
> +wide_int minus1 = std::shwi (-1, prec); // Avoid if minus1 is read-only</pre></blockquote>
> +
> +    <p>In principle this rule applies to other views of an object too,
> +    such as a reversed view of a list, or a sequential view of a
> +    <code>hash_set</code>.  It does not apply to general temporaries.</p>
> +  </li>
> +  <li>
> +    <p>the type of an iterator.  For example:</p>
> +
> +    <blockquote>
> +<pre><b>auto</b> it = <b>std::find (names.begin (), names.end (), needle)</b>;        // OK
> +vector &lt;name_map&gt;::iterator it = std::find (names.begin (),
> +                                            names.end (), needle); // Avoid</pre></blockquote>
> +  </li>
> +  <li>
> +    <p>the type of a lambda expression.  For example:</p>
> +
> +    <blockquote>
> +<pre><b>auto</b> f = <b>[] (int x) { return x + 1; }</b>; // OK</pre></blockquote>
> +  </li>
> +</ul></p>
> +
> +<p><code>auto</code> should <b>not</b> be used in other contexts.</p>

This seems like a severe (and unnecessary) restriction...

> +
> +<p>
> +<a href="codingrationale.html#auto">Rationale and Discussion</a>
> +</p>
>   
>   <h4 id="Struct_Use">Struct Definitions</h4>
>   
> diff --git a/htdocs/codingrationale.html b/htdocs/codingrationale.html
> index 0b44f1da..a919023c 100644
> --- a/htdocs/codingrationale.html
> +++ b/htdocs/codingrationale.html
> @@ -50,6 +50,23 @@ if (info *q = get_any_available_info ()) {
>   }
>   </code></pre></blockquote>
>   
> +<h4 id="auto">Use of <code>auto</code></h4>
> +
> +<p>The reason for preferring <code>auto</code> in expressions like:
> +<blockquote><pre>auto wioff = wi::to_wide (off);</pre></blockquote>
> +is that using the natural type of the expression is more efficient than
> +converting it to types like <code>wide_int</code>.</p>
> +
> +<p>The reason for excluding other uses of <code>auto</code> is that
> +in most other cases the type carries useful information.  For example:
> +<blockquote><pre>for (const std::pair &lt;const char *, tree&gt; &amp;elt : indirect_pool)
> +  ...</pre></blockquote>
> +makes it obvious that <code>elt</code> is a pair and gives the types of
> +<code>elt.first</code> and <code>elt.second</code>.  In contrast:
> +<blockquote><pre>for (const auto &amp;elt : indirect_pool)
> +  ...</pre></blockquote>
> +gives no immediate indication what <code>elt</code> is or what can
> +be done with it.</p>

...there are countless constructs in C++ 98 as well in C where there
is no such indication yet we don't (and very well can't) try to avoid
using them.  Examples include macros, members of structures defined
far away from the point of their use, results of ordinary function
calls, results of overloaded functions or templates, default function
arguments, default template parameters, etc.

By way of a random example from genrecog.c:

         int_set::iterator end
	= std::set_union (trans1->labels.begin (), trans1->labels.end (),
			  combined->begin (), combined->end (),
			  next->begin ());

There is no immediate indication precisely what type int_set::iterator
is.  All we can tell is that that it's some sort of an iterator, and
that should be good enough.  It lets us (even forces us to) write code
that satisfies the requirements of the abstraction (whatever it happens
to be), and avoid tying it closely to the implementation.  That's
a good thing.

Unless there is a sound technical reason for avoiding it (e.g.,
unacceptable inefficiency or known safety problems) I'd say leave
it to everyone's judgment what convenience features to use. If
something turns out to be a problem we'll deal with it then.

Martin

Re:

[Jason Merrill]

On Fri, May 15, 2020 at 9:47 PM Martin Sebor <msebor@gmail.com> wrote:

> On 5/15/20 8:08 AM, Richard Sandiford wrote:
> >>>> We've moved more and more to stronly-typed data structures
> >>>> so I'd not like to see 'auto' everywhere - it should be still
> >>>> obvious what kind of objects we're working with where they
> >>>> matter.  IMHO they do not matter for example for iterators.
> >>>> I don't care about the iterator type but about the type of
> >>>> the object and the container.
> >>> Also agreed. :-)  How about this as a starting point:
> >>>
> >>> ---------------------------------------------------------------
> >>> Use auto for:
> >>>
> >>> - the result of casts or other expressions that give the type
> >>>    explicitly.  E.g.:
> >>>
> >>>      if (auto *table = dyn_cast <rtx_jump_table_data *> (insn))
> >>>
> >>>    instead of:
> >>>
> >>>      if (rtx_jump_table_data *table = dyn_cast <rtx_jump_table_data *>
> (insn))
> >>>
> >>> - iterator types.  E.g.:
> >>>
> >>>      auto it = foo.begin ();
> >>>
> >>>    instead of:
> >>>
> >>>      foo_type::iterator it = foo.begin ();
> >>>
> >>> - expressions that provide an alternative view of something,
> >>>    when the expression is bound to a read-only temporary.  E.g.:
> >>>
> >>>      auto val1 = wi::to_wide (...);
> >>>      auto val2 = wi::uhwi (12, 16);
> >>>
> >>>    instead of:
> >>>
> >>>      wide_int val1 = wi::to_wide (...);
> >>>      wide_int val2 = wi::uhwi (12, 16);
> >>>
> >>>    (Using "wide_int" is less efficient than using the natural type of
> >>>    the expression.)
> >>>
> >>> - the type of a lambda expression.  E.g.:
> >>>
> >>>      auto f = [] (int x) { return x + 1; };
> >> Those are all good examples.  Mind putting that into a patch
> >> for the coding conventions?
> > How's this?  I added "new" expressions as another example of the
> > first category.
> >
> > I'm sure I've missed other good uses, but we can always add to the
> > list later if necessary.
> >
> > Thanks,
> > Richard
> >
> >
> > 0001-Describe-coding-conventions-surrounding-auto.patch
> >
> >  From 10b27e367de0fa9d5bf91544385401cdcbdb8c00 Mon Sep 17 00:00:00 2001
> > From: Richard Sandiford<richard.sandiford@arm.com>
> > Date: Fri, 15 May 2020 14:58:46 +0100
> > Subject: [PATCH] Describe coding conventions surrounding "auto"
> >
> > ---
> >   htdocs/codingconventions.html | 53 +++++++++++++++++++++++++++++++++++
> >   htdocs/codingrationale.html   | 17 +++++++++++
> >   2 files changed, 70 insertions(+)
> >
> > diff --git a/htdocs/codingconventions.html
> b/htdocs/codingconventions.html
> > index f4732ef6..ae49fb91 100644
> > --- a/htdocs/codingconventions.html
> > +++ b/htdocs/codingconventions.html
> > @@ -51,6 +51,7 @@ the conventions separately from any other changes to
> the code.</p>
> >       <li><a href="#Cxx_Language">Language Use</a>
> >           <ul>
> >           <li><a href="#Variable">Variable Definitions</a></li>
> > +        <li><a href="#Auto">Use of <code>auto</code></a></li>
> >           <li><a href="#Struct_Use">Struct Definitions</a></li>
> >           <li><a href="#Class_Use">Class Definitions</a></li>
> >           <li><a href="#Constructors">Constructors and
> Destructors</a></li>
> > @@ -884,6 +885,58 @@ Variables may be simultaneously defined and tested
> in control expressions.
> >   <a href="codingrationale.html#variables">Rationale and Discussion</a>
> >   </p>
> >
> > +<h4 id="Auto">Use of <code>auto</code></h4>
> > +
> > +<p><code>auto</code> should be used in the following circumstances:
> > +<ul>
> > +  <li><p>when the expression gives the C++ type explicitly.  For
> example</p>
> > +
> > +    <blockquote>
> > +<pre>if (<b>auto *</b>table = dyn_cast &lt;<b>rtx_jump_table_data
> *</b>&gt; (insn))                 // OK
> > +  ...
> > +if (rtx_jump_table_data *table = dyn_cast &lt;rtx_jump_table_data *&gt;
> (insn))  // Avoid
> > +  ...
> > +<b>auto *</b>map = new <b>hash_map &lt;tree, size_t&gt;</b>;
>         // OK
> > +hash_map &lt;tree, size_t&gt; *map = new hash_map &lt;tree, size_t&gt;;
> // Avoid</pre></blockquote>
> > +
> > +    <p>This rule does not apply to abbreviated type names embedded in
> > +    an identifier, such as the result of <code>tree_to_shwi</code>.</p>
> > +  </li>
> > +  <li>
> > +    <p>when the expression simply provides an alternative view of an
> object
> > +    and is bound to a read-only temporary.  For example:</p>
> > +
> > +    <blockquote>
> > +<pre><b>auto</b> wioff = <b>wi::to_wide (off);</b>         // OK
> > +wide_int wioff = wi::to_wide (off);     // Avoid if wioff is read-only
> > +<b>auto</b> minus1 = <b>std::shwi (-1, prec);</b>     // OK
> > +wide_int minus1 = std::shwi (-1, prec); // Avoid if minus1 is
> read-only</pre></blockquote>
> > +
> > +    <p>In principle this rule applies to other views of an object too,
> > +    such as a reversed view of a list, or a sequential view of a
> > +    <code>hash_set</code>.  It does not apply to general
> temporaries.</p>
> > +  </li>
> > +  <li>
> > +    <p>the type of an iterator.  For example:</p>
> > +
> > +    <blockquote>
> > +<pre><b>auto</b> it = <b>std::find (names.begin (), names.end (),
> needle)</b>;        // OK
> > +vector &lt;name_map&gt;::iterator it = std::find (names.begin (),
> > +                                            names.end (), needle); //
> Avoid</pre></blockquote>
> > +  </li>
> > +  <li>
> > +    <p>the type of a lambda expression.  For example:</p>
> > +
> > +    <blockquote>
> > +<pre><b>auto</b> f = <b>[] (int x) { return x + 1; }</b>; //
> OK</pre></blockquote>
> > +  </li>
> > +</ul></p>
> > +
> > +<p><code>auto</code> should <b>not</b> be used in other contexts.</p>
>
> This seems like a severe (and unnecessary) restriction...
>
> > +
> > +<p>
> > +<a href="codingrationale.html#auto">Rationale and Discussion</a>
> > +</p>
> >
> >   <h4 id="Struct_Use">Struct Definitions</h4>
> >
> > diff --git a/htdocs/codingrationale.html b/htdocs/codingrationale.html
> > index 0b44f1da..a919023c 100644
> > --- a/htdocs/codingrationale.html
> > +++ b/htdocs/codingrationale.html
> > @@ -50,6 +50,23 @@ if (info *q = get_any_available_info ()) {
> >   }
> >   </code></pre></blockquote>
> >
> > +<h4 id="auto">Use of <code>auto</code></h4>
> > +
> > +<p>The reason for preferring <code>auto</code> in expressions like:
> > +<blockquote><pre>auto wioff = wi::to_wide (off);</pre></blockquote>
> > +is that using the natural type of the expression is more efficient than
> > +converting it to types like <code>wide_int</code>.</p>
> > +
> > +<p>The reason for excluding other uses of <code>auto</code> is that
> > +in most other cases the type carries useful information.  For example:
> > +<blockquote><pre>for (const std::pair &lt;const char *, tree&gt;
> &amp;elt : indirect_pool)
> > +  ...</pre></blockquote>
> > +makes it obvious that <code>elt</code> is a pair and gives the types of
> > +<code>elt.first</code> and <code>elt.second</code>.  In contrast:
> > +<blockquote><pre>for (const auto &amp;elt : indirect_pool)
> > +  ...</pre></blockquote>
> > +gives no immediate indication what <code>elt</code> is or what can
> > +be done with it.</p>
>
> ...there are countless constructs in C++ 98 as well in C where there
> is no such indication yet we don't (and very well can't) try to avoid
> using them.  Examples include macros, members of structures defined
> far away from the point of their use, results of ordinary function
> calls, results of overloaded functions or templates, default function
> arguments, default template parameters, etc.
>
> By way of a random example from genrecog.c:
>
>          int_set::iterator end
>         = std::set_union (trans1->labels.begin (), trans1->labels.end (),
>                           combined->begin (), combined->end (),
>                           next->begin ());
>
> There is no immediate indication precisely what type int_set::iterator
> is.  All we can tell is that that it's some sort of an iterator, and
> that should be good enough.  It lets us (even forces us to) write code
> that satisfies the requirements of the abstraction (whatever it happens
> to be), and avoid tying it closely to the implementation.  That's
> a good thing.
>
> Unless there is a sound technical reason for avoiding it (e.g.,
> unacceptable inefficiency or known safety problems) I'd say leave
> it to everyone's judgment what convenience features to use. If
> something turns out to be a problem we'll deal with it then.
>

I agree with this.  If using 'auto' makes the code harder to read, that's a
good comment for code review, but it's hard to write a general rule for
this sort of thing.

Jason

Re:

[Xinliang David Li]

On Thu, Feb 14, 2013 at 10:18 AM, Matt <matt@use.net> wrote:
> The attached patches do two things:
> 1. Backports a fix from trunk that eliminates bogus warning traces. On my
> current codebase which links ~40MB of C++ with LTO, the bogus warning traces
> are literally hundreds of lines.
>

What is the trunk revision?

David

> I verified the backport fixed our issue by doing doing a profiledbootstrap
> using the bootstrap-lto.mk config with -O3 added. I used the resulting
> compiler on the proprietary codebase, C++Benchmark, scummvm, and a few other
> open source projects to validate.
>
> 2. Our primary development platform is RHEL6.1-based, and the recent
> autoconf requirement bump locked us out. I lowered the version, and saw no
> difference in ability to configure/bootstrap.
>
> Thanks!
>
>
> --
> tangled strands of DNA explain the way that I behave.
> http://www.clock.org/~matt

Re:

[Matt Hargett]

On Feb 14, 2013, at 10:40 AM, Xinliang David Li <davidxl@google.com> wrote:

> On Thu, Feb 14, 2013 at 10:18 AM, Matt <matt@use.net> wrote:
>> The attached patches do two things:
>> 1. Backports a fix from trunk that eliminates bogus warning traces. On my
>> current codebase which links ~40MB of C++ with LTO, the bogus warning traces
>> are literally hundreds of lines.
> 
> What is the trunk revision?

Richard's original patch was committed to trunk in r195884.


>> I verified the backport fixed our issue by doing doing a profiledbootstrap
>> using the bootstrap-lto.mk config with -O3 added. I used the resulting
>> compiler on the proprietary codebase, C++Benchmark, scummvm, and a few other
>> open source projects to validate.
>> 
>> 2. Our primary development platform is RHEL6.1-based, and the recent
>> autoconf requirement bump locked us out. I lowered the version, and saw no
>> difference in ability to configure/bootstrap.
>> 
>> Thanks!
>> 
>> 
>> --
>> tangled strands of DNA explain the way that I behave.
>> http://www.clock.org/~matt

Re:

[Xinliang David Li]

Ok for the google branch -- please provide the patch details in svn
commit message (note that ChangeLog is not needed any more for the
branch).

David



On Thu, Feb 14, 2013 at 11:53 AM, Matt Hargett <matt@use.net> wrote:
> On Feb 14, 2013, at 10:40 AM, Xinliang David Li <davidxl@google.com> wrote:
>
>> On Thu, Feb 14, 2013 at 10:18 AM, Matt <matt@use.net> wrote:
>>> The attached patches do two things:
>>> 1. Backports a fix from trunk that eliminates bogus warning traces. On my
>>> current codebase which links ~40MB of C++ with LTO, the bogus warning traces
>>> are literally hundreds of lines.
>>
>> What is the trunk revision?
>
> Richard's original patch was committed to trunk in r195884.
>
>
>>> I verified the backport fixed our issue by doing doing a profiledbootstrap
>>> using the bootstrap-lto.mk config with -O3 added. I used the resulting
>>> compiler on the proprietary codebase, C++Benchmark, scummvm, and a few other
>>> open source projects to validate.
>>>
>>> 2. Our primary development platform is RHEL6.1-based, and the recent
>>> autoconf requirement bump locked us out. I lowered the version, and saw no
>>> difference in ability to configure/bootstrap.
>>>
>>> Thanks!
>>>
>>>
>>> --
>>> tangled strands of DNA explain the way that I behave.
>>> http://www.clock.org/~matt

Re:

[Matt]

On Thu, 14 Feb 2013, Xinliang David Li wrote:

> Ok for the google branch -- please provide the patch details in svn
> commit message (note that ChangeLog is not needed any more for the
> branch).

I don't have commit access (yet). Should I email overseers@gcc.gnu.org as 
mentioned at http://gcc.gnu.org/svnwrite.html to get the ball rolling?





> On Thu, Feb 14, 2013 at 11:53 AM, Matt Hargett <matt@use.net> wrote:
>> On Feb 14, 2013, at 10:40 AM, Xinliang David Li <davidxl@google.com> wrote:
>>
>>> On Thu, Feb 14, 2013 at 10:18 AM, Matt <matt@use.net> wrote:
>>>> The attached patches do two things:
>>>> 1. Backports a fix from trunk that eliminates bogus warning traces. On my
>>>> current codebase which links ~40MB of C++ with LTO, the bogus warning traces
>>>> are literally hundreds of lines.
>>>
>>> What is the trunk revision?
>>
>> Richard's original patch was committed to trunk in r195884.
>>
>>
>>>> I verified the backport fixed our issue by doing doing a profiledbootstrap
>>>> using the bootstrap-lto.mk config with -O3 added. I used the resulting
>>>> compiler on the proprietary codebase, C++Benchmark, scummvm, and a few other
>>>> open source projects to validate.
>>>>
>>>> 2. Our primary development platform is RHEL6.1-based, and the recent
>>>> autoconf requirement bump locked us out. I lowered the version, and saw no
>>>> difference in ability to configure/bootstrap.
>>>>
>>>> Thanks!
>>>>
>>>>
>>>> --
>>>> tangled strands of DNA explain the way that I behave.
>>>> http://www.clock.org/~matt
>


--
tangled strands of DNA explain the way that I behave.
http://www.clock.org/~matt

Re:

[Richard Biener]

On Thu, Oct 25, 2012 at 10:56 PM, Lawrence Crowl <crowl@googlers.com> wrote:
> Change hash_table to support a comparator type different from the
> value type stored in the hash table.  The 'find' functions now may
> take a different type from the value type.  This requires introducing
> a second typedef into the Descriptor conceptual type.  Change the
> Descriptor concept to use typedefs value_type and compare_type instead
> of T.  Change all users to match.
>
> Add usage documentation to hash-table.h.
>
> Tested on x86-64.
>
> Okay for trunk?

Can you elaborate on why this is needed?

Thanks,
Richard.

>
> Index: gcc/ChangeLog
>
> 2012-10-25  Lawrence Crowl  <crowl@google.com>
>
>         * hash-table.h: Add usage documentation.
>         (template struct typed_free_remove): Clarify documentation.
>         Rename template parameter.
>         (struct typed_noop_remove): Likewise.
>         (descriptor concept): Change typedef T to value_type.
>         Add typedef compare_type.  Use more precise template parameter name,
>         Descriptor instead of Descr.  Update users to match.
>         (struct hash_table): Change 'find' parameters to use compare_type
>         instead of the value type.
>
>
> Index: gcc/tree-ssa-tail-merge.c
> ===================================================================
> --- gcc/tree-ssa-tail-merge.c   (revision 192820)
> +++ gcc/tree-ssa-tail-merge.c   (working copy)
> @@ -226,10 +226,11 @@ struct same_succ_def
>    hashval_t hashval;
>
>    /* hash_table support.  */
> -  typedef same_succ_def T;
> -  static inline hashval_t hash (const same_succ_def *);
> -  static int equal (const same_succ_def *, const same_succ_def *);
> -  static void remove (same_succ_def *);
> +  typedef same_succ_def value_type;
> +  typedef same_succ_def compare_type;
> +  static inline hashval_t hash (const value_type *);
> +  static int equal (const value_type *, const compare_type *);
> +  static void remove (value_type *);
>  };
>  typedef struct same_succ_def *same_succ;
>  typedef const struct same_succ_def *const_same_succ;
> @@ -237,7 +238,7 @@ typedef const struct same_succ_def *cons
>  /* hash routine for hash_table support, returns hashval of E.  */
>
>  inline hashval_t
> -same_succ_def::hash (const same_succ_def *e)
> +same_succ_def::hash (const value_type *e)
>  {
>    return e->hashval;
>  }
> @@ -528,7 +529,7 @@ inverse_flags (const_same_succ e1, const
>  /* Compares SAME_SUCCs E1 and E2.  */
>
>  int
> -same_succ_def::equal (const_same_succ e1, const_same_succ e2)
> +same_succ_def::equal (const value_type *e1, const compare_type *e2)
>  {
>    unsigned int i, first1, first2;
>    gimple_stmt_iterator gsi1, gsi2;
> Index: gcc/tree-ssa-threadupdate.c
> ===================================================================
> --- gcc/tree-ssa-threadupdate.c (revision 192820)
> +++ gcc/tree-ssa-threadupdate.c (working copy)
> @@ -127,20 +127,21 @@ struct redirection_data : typed_free_rem
>    struct el *incoming_edges;
>
>    /* hash_table support.  */
> -  typedef redirection_data T;
> -  static inline hashval_t hash (const redirection_data *);
> -  static inline int equal (const redirection_data *, const redirection_data *);
> +  typedef redirection_data value_type;
> +  typedef redirection_data compare_type;
> +  static inline hashval_t hash (const value_type *);
> +  static inline int equal (const value_type *, const compare_type *);
>  };
>
>  inline hashval_t
> -redirection_data::hash (const redirection_data *p)
> +redirection_data::hash (const value_type *p)
>  {
>    edge e = p->outgoing_edge;
>    return e->dest->index;
>  }
>
>  inline int
> -redirection_data::equal (const redirection_data *p1, const
> redirection_data *p2)
> +redirection_data::equal (const value_type *p1, const compare_type *p2)
>  {
>    edge e1 = p1->outgoing_edge;
>    edge e2 = p2->outgoing_edge;
> Index: gcc/java/jcf-io.c
> ===================================================================
> --- gcc/java/jcf-io.c   (revision 192820)
> +++ gcc/java/jcf-io.c   (working copy)
> @@ -276,19 +276,21 @@ find_classfile (char *filename, JCF *jcf
>
>  struct charstar_hash : typed_noop_remove <char>
>  {
> -  typedef const char T;
> -  static inline hashval_t hash (const T *candidate);
> -  static inline bool equal (const T *existing, const T *candidate);
> +  typedef const char value_type;
> +  typedef const char compare_type;
> +  static inline hashval_t hash (const value_type *candidate);
> +  static inline bool equal (const value_type *existing,
> +                           const compare_type *candidate);
>  };
>
>  inline hashval_t
> -charstar_hash::hash (const T *candidate)
> +charstar_hash::hash (const value_type *candidate)
>  {
>    return htab_hash_string (candidate);
>  }
>
>  inline bool
> -charstar_hash::equal (const T *existing, const T *candidate)
> +charstar_hash::equal (const value_type *existing, const compare_type
> *candidate)
>  {
>    return strcmp (existing, candidate) == 0;
>  }
> Index: gcc/valtrack.h
> ===================================================================
> --- gcc/valtrack.h      (revision 192820)
> +++ gcc/valtrack.h      (working copy)
> @@ -46,32 +46,33 @@ struct dead_debug_global_entry
>  struct dead_debug_hash_descr
>  {
>    /* The hash table contains pointers to entries of this type.  */
> -  typedef struct dead_debug_global_entry T;
> +  typedef struct dead_debug_global_entry value_type;
> +  typedef struct dead_debug_global_entry compare_type;
>    /* Hash on the pseudo number.  */
> -  static inline hashval_t hash (T const *my);
> +  static inline hashval_t hash (const value_type *my);
>    /* Entries are identical if they refer to the same pseudo.  */
> -  static inline bool equal (T const *my, T const *other);
> +  static inline bool equal (const value_type *my, const compare_type *other);
>    /* Release entries when they're removed.  */
> -  static inline void remove (T *p);
> +  static inline void remove (value_type *p);
>  };
>
>  /* Hash on the pseudo number.  */
>  inline hashval_t
> -dead_debug_hash_descr::hash (T const *my)
> +dead_debug_hash_descr::hash (const value_type *my)
>  {
>    return REGNO (my->reg);
>  }
>
>  /* Entries are identical if they refer to the same pseudo.  */
>  inline bool
> -dead_debug_hash_descr::equal (T const *my, T const *other)
> +dead_debug_hash_descr::equal (const value_type *my, const compare_type *other)
>  {
>    return my->reg == other->reg;
>  }
>
>  /* Release entries when they're removed.  */
>  inline void
> -dead_debug_hash_descr::remove (T *p)
> +dead_debug_hash_descr::remove (value_type *p)
>  {
>    XDELETE (p);
>  }
> Index: gcc/objc/objc-act.c
> ===================================================================
> --- gcc/objc/objc-act.c (revision 192820)
> +++ gcc/objc/objc-act.c (working copy)
> @@ -3827,19 +3827,20 @@ objc_get_class_ivars (tree class_name)
>
>  struct decl_name_hash : typed_noop_remove <tree_node>
>  {
> -  typedef tree_node T;
> -  static inline hashval_t hash (const T *);
> -  static inline bool equal (const T *, const T *);
> +  typedef tree_node value_type;
> +  typedef tree_node compare_type;
> +  static inline hashval_t hash (const value_type *);
> +  static inline bool equal (const value_type *, const compare_type *);
>  };
>
>  inline hashval_t
> -decl_name_hash::hash (const T *q)
> +decl_name_hash::hash (const value_type *q)
>  {
>    return (hashval_t) ((intptr_t)(DECL_NAME (q)) >> 3);
>  }
>
>  inline bool
> -decl_name_hash::equal (const T *a, const T *b)
> +decl_name_hash::equal (const value_type *a, const compare_type *b)
>  {
>    return DECL_NAME (a) == DECL_NAME (b);
>  }
> Index: gcc/cfg.c
> ===================================================================
> --- gcc/cfg.c   (revision 192820)
> +++ gcc/cfg.c   (working copy)
> @@ -988,19 +988,21 @@ struct htab_bb_copy_original_entry
>
>  struct bb_copy_hasher : typed_noop_remove <htab_bb_copy_original_entry>
>  {
> -  typedef htab_bb_copy_original_entry T;
> -  static inline hashval_t hash (const T *);
> -  static inline bool equal (const T *existing, const T * candidate);
> +  typedef htab_bb_copy_original_entry value_type;
> +  typedef htab_bb_copy_original_entry compare_type;
> +  static inline hashval_t hash (const value_type *);
> +  static inline bool equal (const value_type *existing,
> +                           const compare_type * candidate);
>  };
>
>  inline hashval_t
> -bb_copy_hasher::hash (const T *data)
> +bb_copy_hasher::hash (const value_type *data)
>  {
>    return data->index1;
>  }
>
>  inline bool
> -bb_copy_hasher::equal (const T *data, const T *data2)
> +bb_copy_hasher::equal (const value_type *data, const compare_type *data2)
>  {
>    return data->index1 == data2->index1;
>  }
> Index: gcc/hash-table.h
> ===================================================================
> --- gcc/hash-table.h    (revision 192820)
> +++ gcc/hash-table.h    (working copy)
> @@ -21,7 +21,142 @@ along with GCC; see the file COPYING3.
>
>
>  /* This file implements a typed hash table.
> -   The implementation borrows from libiberty's hashtab.  */
> +   The implementation borrows from libiberty's htab_t in hashtab.h.
> +
> +
> +   INTRODUCTION TO TYPES
> +
> +   Users of the hash table generally need to be aware of three types.
> +
> +      1. The type being placed into the hash table.  This type is called
> +      the value type.
> +
> +      2. The type used to describe how to handle the value type within
> +      the hash table.  This descriptor type provides the hash table with
> +      several things.
> +
> +         - A typedef named 'value_type' to the value type (from above).
> +
> +         - A static member function named 'hash' that takes a value_type
> +         pointer and returns a hashval_t value.
> +
> +         - A typedef named 'compare_type' that is used to test when an value
> +         is found.  This type is the comparison type.  Usually, it will be the
> +         same as value_type.  If it is not the same type, you must generally
> +         explicitly compute hash values and pass them to the hash table.
> +
> +         - A static member function named 'equal' that takes a value_type
> +         pointer and a compare_type pointer, and returns a bool.
> +
> +         - A static function named 'remove' that takes an value_type pointer
> +         and frees the memory allocated by it.  This function is used when
> +         individual elements of the table need to be disposed of (e.g.,
> +         when deleting a hash table, removing elements from the table, etc).
> +
> +      3. The type of the hash table itself.  (More later.)
> +
> +   In very special circumstances, users may need to know about a fourth type.
> +
> +      4. The template type used to describe how hash table memory
> +      is allocated.  This type is called the allocator type.  It is
> +      parameterized on the value type.  It provides four functions.
> +
> +         - A static member function named 'control_alloc'.  This function
> +         allocates the control data blocks for the table.
> +
> +         - A static member function named 'control_free'.  This function
> +         frees the control data blocks for the table.
> +
> +         - A static member function named 'data_alloc'.  This function
> +         allocates the data elements in the table.
> +
> +         - A static member function named 'data_free'.  This function
> +         deallocates the data elements in the table.
> +
> +   Hash table are instantiated with two type arguments.
> +
> +      * The descriptor type, (2) above.
> +
> +      * The allocator type, (4) above.  In general, you will not need to
> +      provide your own allocator type.  By default, hash tables will use
> +      the class template xcallocator, which uses malloc/free for allocation.
> +
> +
> +   DEFINING A DESCRIPTOR TYPE
> +
> +   The first task in using the hash table is to describe the element type.
> +   We compose this into a few steps.
> +
> +      1. Decide on a removal policy for values stored in the table.
> +         This header provides class templates for the two most common
> +         policies.
> +
> +         * typed_free_remove implements the static 'remove' member function
> +         by calling free().
> +
> +         * typed_noop_remove implements the static 'remove' member function
> +         by doing nothing.
> +
> +         You can use these policies by simply deriving the descriptor type
> +         from one of those class template, with the appropriate argument.
> +
> +         Otherwise, you need to write the static 'remove' member function
> +         in the descriptor class.
> +
> +      2. Choose a hash function.  Write the static 'hash' member function.
> +
> +      3. Choose an equality testing function.  In most cases, its two
> +      arguments will be value_type pointers.  If not, the first argument must
> +      be a value_type pointer, and the second argument a compare_type pointer.
> +
> +
> +   AN EXAMPLE DESCRIPTOR TYPE
> +
> +   Suppose you want to put some_type into the hash table.  You could define
> +   the descriptor type as follows.
> +
> +      struct some_type_hasher : typed_noop_remove <some_type>
> +      // Deriving from typed_noop_remove means that we get a 'remove' that does
> +      // nothing.  This choice is good for raw values.
> +      {
> +        typedef some_type value_type;
> +        typedef some_type compare_type;
> +        static inline hashval_t hash (const value_type *);
> +        static inline bool equal (const value_type *, const compare_type *);
> +      };
> +
> +      inline hashval_t
> +      some_type_hasher::hash (const value_type *e)
> +      { ... compute and return a hash value for E ... }
> +
> +      inline bool
> +      some_type_hasher::equal (const value_type *p1, const compare_type *p2)
> +      { ... compare P1 vs P2.  Return true if they are the 'same' ... }
> +
> +
> +   AN EXAMPLE HASH_TABLE DECLARATION
> +
> +   To instantiate a hash table for some_type:
> +
> +      hash_table <some_type_hasher> some_type_hash_table;
> +
> +   There is no need to mention some_type directly, as the hash table will
> +   obtain it using some_type_hasher::value_type.
> +
> +   You can then used any of the functions in hash_table's public interface.
> +   See hash_table for details.  The interface is very similar to libiberty's
> +   htab_t.
> +
> +
> +   EASY DESCRIPTORS FOR POINTERS
> +
> +   The class template pointer_hash provides everything you need to hash
> +   pointers (as opposed to what they point to).  So, to instantiate a hash
> +   table over pointers to whatever_type,
> +
> +      hash_table <pointer_hash <whatever_type>> whatever_type_hash_table;
> +
> +*/
>
>
>  #ifndef TYPED_HASHTAB_H
> @@ -53,7 +188,7 @@ xcallocator <Type>::control_alloc (size_
>  }
>
>
> -/* Allocate memory for COUNT data blocks.  */
> +/* Allocate memory for COUNT data blocks.  */
>
>  template <typename Type>
>  inline Type *
> @@ -71,7 +206,7 @@ xcallocator <Type>::control_free (Type *
>  {
>    return ::free (memory);
>  }
> -
> +
>
>  /* Free memory for data blocks.  */
>
> @@ -83,50 +218,71 @@ xcallocator <Type>::data_free (Type *mem
>  }
>
>
> -/* Remove method dispatching to free.  */
> +/* Helpful type for removing with free.  */
>
> -template <typename Element>
> +template <typename Type>
>  struct typed_free_remove
>  {
> -  static inline void remove (Element *p) { free (p); }
> +  static inline void remove (Type *p);
>  };
>
> -/* No-op remove method.  */
>
> -template <typename Element>
> +/* Remove with free.  */
> +
> +template <typename Type>
> +inline void
> +typed_free_remove <Type>::remove (Type *p)
> +{
> +  free (p);
> +}
> +
> +
> +/* Helpful type for a no-op remove.  */
> +
> +template <typename Type>
>  struct typed_noop_remove
>  {
> -  static inline void remove (Element *) {}
> +  static inline void remove (Type *p);
>  };
>
>
> +/* Remove doing nothing.  */
> +
> +template <typename Type>
> +inline void
> +typed_noop_remove <Type>::remove (Type *p ATTRIBUTE_UNUSED)
> +{
> +}
> +
> +
>  /* Pointer hash with a no-op remove method.  */
>
> -template <typename Element>
> -struct pointer_hash : typed_noop_remove <Element>
> +template <typename Type>
> +struct pointer_hash : typed_noop_remove <Type>
>  {
> -  typedef Element T;
> +  typedef Type value_type;
> +  typedef Type compare_type;
>
>    static inline hashval_t
> -  hash (const T *);
> +  hash (const value_type *);
>
>    static inline int
> -  equal (const T *existing, const T * candidate);
> +  equal (const value_type *existing, const compare_type *candidate);
>  };
>
> -template <typename Element>
> +template <typename Type>
>  inline hashval_t
> -pointer_hash<Element>::hash (const T *candidate)
> +pointer_hash <Type>::hash (const value_type *candidate)
>  {
>    /* This is a really poor hash function, but it is what the current code uses,
>       so I am reusing it to avoid an additional axis in testing.  */
>    return (hashval_t) ((intptr_t)candidate >> 3);
>  }
>
> -template <typename Element>
> +template <typename Type>
>  inline int
> -pointer_hash<Element>::equal (const T *existing,
> -                             const T *candidate)
> +pointer_hash <Type>::equal (const value_type *existing,
> +                          const compare_type *candidate)
>  {
>    return existing == candidate;
>  }
> @@ -185,37 +341,38 @@ struct hash_table_control
>
>  /* User-facing hash table type.
>
> -   The table stores elements of type Element.
> +   The table stores elements of type Descriptor::value_type.
>
> -   It hashes elements with the hash function.
> +   It hashes values with the hash member function.
>       The table currently works with relatively weak hash functions.
> -     Use typed_pointer_hash <Element> when hashing pointers instead of objects.
> +     Use typed_pointer_hash <Value> when hashing pointers instead of objects.
>
> -   It compares elements with the equal function.
> +   It compares elements with the equal member function.
>       Two elements with the same hash may not be equal.
> -     Use typed_pointer_equal <Element> when hashing pointers instead
> of objects.
> +     Use typed_pointer_equal <Value> when hashing pointers instead of objects.
>
> -   It removes elements with the remove function.
> +   It removes elements with the remove member function.
>       This feature is useful for freeing memory.
> -     Use typed_null_remove <Element> when not freeing objects.
> -     Use typed_free_remove <Element> when doing a simple object free.
> +     Derive from typed_null_remove <Value> when not freeing objects.
> +     Derive from typed_free_remove <Value> when doing a simple object free.
>
> -   Use the Allocator template to allocate and free memory.
> +   Specify the template Allocator to allocate and free memory.
>       The default is xcallocator.
>
>  */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator = xcallocator>
>  class hash_table
>  {
>  public:
> -  typedef typename Descr::T T;
> +  typedef typename Descriptor::value_type value_type;
> +  typedef typename Descriptor::compare_type compare_type;
>
>  private:
> -  hash_table_control <T> *htab;
> +  hash_table_control <value_type> *htab;
>
> -  T **find_empty_slot_for_expand (hashval_t hash);
> +  value_type **find_empty_slot_for_expand (hashval_t hash);
>    void expand ();
>
>  public:
> @@ -223,35 +380,36 @@ public:
>    void create (size_t initial_slots);
>    bool is_created ();
>    void dispose ();
> -  T *find (const T *comparable);
> -  T *find_with_hash (const T *comparable, hashval_t hash);
> -  T **find_slot (const T *comparable, enum insert_option insert);
> -  T **find_slot_with_hash (const T *comparable, hashval_t hash,
> -                                  enum insert_option insert);
> +  value_type *find (const compare_type *comparable);
> +  value_type *find_with_hash (const compare_type *comparable, hashval_t hash);
> +  value_type **find_slot (const compare_type *comparable,
> +                         enum insert_option insert);
> +  value_type **find_slot_with_hash (const compare_type *comparable,
> +                                   hashval_t hash, enum insert_option insert);
>    void empty ();
> -  void clear_slot (T **slot);
> -  void remove_elt (const T *comparable);
> -  void remove_elt_with_hash (const T *comparable, hashval_t hash);
> +  void clear_slot (value_type **slot);
> +  void remove_elt (const compare_type *comparable);
> +  void remove_elt_with_hash (const compare_type *comparable, hashval_t hash);
>    size_t size();
>    size_t elements();
>    double collisions();
>
>    template <typename Argument,
> -           int (*Callback) (T **slot, Argument argument)>
> +           int (*Callback) (value_type **slot, Argument argument)>
>    void traverse_noresize (Argument argument);
>
>    template <typename Argument,
> -           int (*Callback) (T **slot, Argument argument)>
> +           int (*Callback) (value_type **slot, Argument argument)>
>    void traverse (Argument argument);
>  };
>
>
>  /* Construct the hash table.  The only useful operation next is create.  */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
>  inline
> -hash_table <Descr, Allocator>::hash_table ()
> +hash_table <Descriptor, Allocator>::hash_table ()
>  : htab (NULL)
>  {
>  }
> @@ -259,10 +417,10 @@ hash_table <Descr, Allocator>::hash_tabl
>
>  /* See if the table has been created, as opposed to constructed.  */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
>  inline bool
> -hash_table <Descr, Allocator>::is_created ()
> +hash_table <Descriptor, Allocator>::is_created ()
>  {
>    return htab != NULL;
>  }
> @@ -270,45 +428,44 @@ hash_table <Descr, Allocator>::is_create
>
>  /* Like find_with_hash, but compute the hash value from the element.  */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
> -inline typename Descr::T *
> -hash_table <Descr, Allocator>::find (const T *comparable)
> +inline typename Descriptor::value_type *
> +hash_table <Descriptor, Allocator>::find (const compare_type *comparable)
>  {
> -  return find_with_hash (comparable, Descr::hash (comparable));
> +  return find_with_hash (comparable, Descriptor::hash (comparable));
>  }
>
>
>  /* Like find_slot_with_hash, but compute the hash value from the element.  */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
> -inline typename Descr::T **
> -hash_table <Descr, Allocator>
> -::find_slot (const T *comparable, enum insert_option insert)
> +inline typename Descriptor::value_type **
> +hash_table <Descriptor, Allocator>
> +::find_slot (const compare_type *comparable, enum insert_option insert)
>  {
> -  return find_slot_with_hash (comparable, Descr::hash (comparable), insert);
> +  return find_slot_with_hash (comparable, Descriptor::hash
> (comparable), insert);
>  }
>
>
>  /* Like remove_elt_with_hash, but compute the hash value from the element.  */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
>  inline void
> -hash_table <Descr, Allocator>
> -::remove_elt (const T *comparable)
> +hash_table <Descriptor, Allocator>::remove_elt (const compare_type *comparable)
>  {
> -  remove_elt_with_hash (comparable, Descr::hash (comparable));
> +  remove_elt_with_hash (comparable, Descriptor::hash (comparable));
>  }
>
>
>  /* Return the current size of this hash table.  */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
>  inline size_t
> -hash_table <Descr, Allocator>::size()
> +hash_table <Descriptor, Allocator>::size()
>  {
>    return htab->size;
>  }
> @@ -316,10 +473,10 @@ hash_table <Descr, Allocator>::size()
>
>  /* Return the current number of elements in this hash table. */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
>  inline size_t
> -hash_table <Descr, Allocator>::elements()
> +hash_table <Descriptor, Allocator>::elements()
>  {
>    return htab->n_elements - htab->n_deleted;
>  }
> @@ -328,10 +485,10 @@ hash_table <Descr, Allocator>::elements(
>    /* Return the fraction of fixed collisions during all work with given
>       hash table. */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
>  inline double
> -hash_table <Descr, Allocator>::collisions()
> +hash_table <Descriptor, Allocator>::collisions()
>  {
>    if (htab->searches == 0)
>      return 0.0;
> @@ -342,19 +499,19 @@ hash_table <Descr, Allocator>::collision
>
>  /* Create a hash table with at least the given number of INITIAL_SLOTS.  */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
>  void
> -hash_table <Descr, Allocator>::create (size_t size)
> +hash_table <Descriptor, Allocator>::create (size_t size)
>  {
>    unsigned int size_prime_index;
>
>    size_prime_index = hash_table_higher_prime_index (size);
>    size = prime_tab[size_prime_index].prime;
>
> -  htab = Allocator <hash_table_control <T> > ::control_alloc (1);
> +  htab = Allocator <hash_table_control <value_type> > ::control_alloc (1);
>    gcc_assert (htab != NULL);
> -  htab->entries = Allocator <T*> ::data_alloc (size);
> +  htab->entries = Allocator <value_type*> ::data_alloc (size);
>    gcc_assert (htab->entries != NULL);
>    htab->size = size;
>    htab->size_prime_index = size_prime_index;
> @@ -364,20 +521,20 @@ hash_table <Descr, Allocator>::create (s
>  /* Dispose of a hash table.  Free all memory and return this hash table to
>     the non-created state.  Naturally the hash table must already exist.  */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
>  void
> -hash_table <Descr, Allocator>::dispose ()
> +hash_table <Descriptor, Allocator>::dispose ()
>  {
>    size_t size = htab->size;
> -  T **entries = htab->entries;
> +  value_type **entries = htab->entries;
>
>    for (int i = size - 1; i >= 0; i--)
>      if (entries[i] != HTAB_EMPTY_ENTRY && entries[i] != HTAB_DELETED_ENTRY)
> -      Descr::remove (entries[i]);
> +      Descriptor::remove (entries[i]);
>
> -  Allocator <T *> ::data_free (entries);
> -  Allocator <hash_table_control <T> > ::control_free (htab);
> +  Allocator <value_type *> ::data_free (entries);
> +  Allocator <hash_table_control <value_type> > ::control_free (htab);
>    htab = NULL;
>  }
>
> @@ -389,15 +546,14 @@ hash_table <Descr, Allocator>::dispose (
>     This function also assumes there are no deleted entries in the table.
>     HASH is the hash value for the element to be inserted.  */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
> -typename Descr::T **
> -hash_table <Descr, Allocator>
> -::find_empty_slot_for_expand (hashval_t hash)
> +typename Descriptor::value_type **
> +hash_table <Descriptor, Allocator>::find_empty_slot_for_expand (hashval_t hash)
>  {
>    hashval_t index = hash_table_mod1 (hash, htab->size_prime_index);
>    size_t size = htab->size;
> -  T **slot = htab->entries + index;
> +  value_type **slot = htab->entries + index;
>    hashval_t hash2;
>
>    if (*slot == HTAB_EMPTY_ENTRY)
> @@ -428,15 +584,15 @@ hash_table <Descr, Allocator>
>     table entries is changed.  If memory allocation fails, this function
>     will abort.  */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
>  void
> -hash_table <Descr, Allocator>::expand ()
> +hash_table <Descriptor, Allocator>::expand ()
>  {
> -  T **oentries;
> -  T **olimit;
> -  T **p;
> -  T **nentries;
> +  value_type **oentries;
> +  value_type **olimit;
> +  value_type **p;
> +  value_type **nentries;
>    size_t nsize, osize, elts;
>    unsigned int oindex, nindex;
>
> @@ -459,7 +615,7 @@ hash_table <Descr, Allocator>::expand ()
>        nsize = osize;
>      }
>
> -  nentries = Allocator <T *> ::data_alloc (nsize);
> +  nentries = Allocator <value_type *> ::data_alloc (nsize);
>    gcc_assert (nentries != NULL);
>    htab->entries = nentries;
>    htab->size = nsize;
> @@ -470,11 +626,11 @@ hash_table <Descr, Allocator>::expand ()
>    p = oentries;
>    do
>      {
> -      T *x = *p;
> +      value_type *x = *p;
>
>        if (x != HTAB_EMPTY_ENTRY && x != HTAB_DELETED_ENTRY)
>          {
> -          T **q = find_empty_slot_for_expand (Descr::hash (x));
> +          value_type **q = find_empty_slot_for_expand (Descriptor::hash (x));
>
>            *q = x;
>          }
> @@ -483,7 +639,7 @@ hash_table <Descr, Allocator>::expand ()
>      }
>    while (p < olimit);
>
> -  Allocator <T *> ::data_free (oentries);
> +  Allocator <value_type *> ::data_free (oentries);
>  }
>
>
> @@ -491,15 +647,15 @@ hash_table <Descr, Allocator>::expand ()
>     COMPARABLE element starting with the given HASH value.  It cannot
>     be used to insert or delete an element. */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
> -typename Descr::T *
> -hash_table <Descr, Allocator>
> -::find_with_hash (const T *comparable, hashval_t hash)
> +typename Descriptor::value_type *
> +hash_table <Descriptor, Allocator>
> +::find_with_hash (const compare_type *comparable, hashval_t hash)
>  {
>    hashval_t index, hash2;
>    size_t size;
> -  T *entry;
> +  value_type *entry;
>
>    htab->searches++;
>    size = htab->size;
> @@ -507,7 +663,7 @@ hash_table <Descr, Allocator>
>
>    entry = htab->entries[index];
>    if (entry == HTAB_EMPTY_ENTRY
> -      || (entry != HTAB_DELETED_ENTRY && Descr::equal (entry, comparable)))
> +      || (entry != HTAB_DELETED_ENTRY && Descriptor::equal (entry,
> comparable)))
>      return entry;
>
>    hash2 = hash_table_mod2 (hash, htab->size_prime_index);
> @@ -520,7 +676,8 @@ hash_table <Descr, Allocator>
>
>        entry = htab->entries[index];
>        if (entry == HTAB_EMPTY_ENTRY
> -          || (entry != HTAB_DELETED_ENTRY && Descr::equal (entry, comparable)))
> +          || (entry != HTAB_DELETED_ENTRY
> +             && Descriptor::equal (entry, comparable)))
>          return entry;
>      }
>  }
> @@ -534,17 +691,17 @@ hash_table <Descr, Allocator>
>     write the value you want into the returned slot.  When inserting an
>     entry, NULL may be returned if memory allocation fails. */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
> -typename Descr::T **
> -hash_table <Descr, Allocator>
> -::find_slot_with_hash (const T *comparable, hashval_t hash,
> +typename Descriptor::value_type **
> +hash_table <Descriptor, Allocator>
> +::find_slot_with_hash (const compare_type *comparable, hashval_t hash,
>                        enum insert_option insert)
>  {
> -  T **first_deleted_slot;
> +  value_type **first_deleted_slot;
>    hashval_t index, hash2;
>    size_t size;
> -  T *entry;
> +  value_type *entry;
>
>    size = htab->size;
>    if (insert == INSERT && size * 3 <= htab->n_elements * 4)
> @@ -563,9 +720,9 @@ hash_table <Descr, Allocator>
>      goto empty_entry;
>    else if (entry == HTAB_DELETED_ENTRY)
>      first_deleted_slot = &htab->entries[index];
> -  else if (Descr::equal (entry, comparable))
> +  else if (Descriptor::equal (entry, comparable))
>      return &htab->entries[index];
> -
> +
>    hash2 = hash_table_mod2 (hash, htab->size_prime_index);
>    for (;;)
>      {
> @@ -573,7 +730,7 @@ hash_table <Descr, Allocator>
>        index += hash2;
>        if (index >= size)
>         index -= size;
> -
> +
>        entry = htab->entries[index];
>        if (entry == HTAB_EMPTY_ENTRY)
>         goto empty_entry;
> @@ -582,7 +739,7 @@ hash_table <Descr, Allocator>
>           if (!first_deleted_slot)
>             first_deleted_slot = &htab->entries[index];
>         }
> -      else if (Descr::equal (entry, comparable))
> +      else if (Descriptor::equal (entry, comparable))
>         return &htab->entries[index];
>      }
>
> @@ -593,7 +750,7 @@ hash_table <Descr, Allocator>
>    if (first_deleted_slot)
>      {
>        htab->n_deleted--;
> -      *first_deleted_slot = static_cast <T *> (HTAB_EMPTY_ENTRY);
> +      *first_deleted_slot = static_cast <value_type *> (HTAB_EMPTY_ENTRY);
>        return first_deleted_slot;
>      }
>
> @@ -604,18 +761,18 @@ hash_table <Descr, Allocator>
>
>  /* This function clears all entries in the given hash table.  */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
>  void
> -hash_table <Descr, Allocator>::empty ()
> +hash_table <Descriptor, Allocator>::empty ()
>  {
>    size_t size = htab->size;
> -  T **entries = htab->entries;
> +  value_type **entries = htab->entries;
>    int i;
>
>    for (i = size - 1; i >= 0; i--)
>      if (entries[i] != HTAB_EMPTY_ENTRY && entries[i] != HTAB_DELETED_ENTRY)
> -      Descr::remove (entries[i]);
> +      Descriptor::remove (entries[i]);
>
>    /* Instead of clearing megabyte, downsize the table.  */
>    if (size > 1024*1024 / sizeof (PTR))
> @@ -623,13 +780,13 @@ hash_table <Descr, Allocator>::empty ()
>        int nindex = hash_table_higher_prime_index (1024 / sizeof (PTR));
>        int nsize = prime_tab[nindex].prime;
>
> -      Allocator <T *> ::data_free (htab->entries);
> -      htab->entries = Allocator <T *> ::data_alloc (nsize);
> +      Allocator <value_type *> ::data_free (htab->entries);
> +      htab->entries = Allocator <value_type *> ::data_alloc (nsize);
>        htab->size = nsize;
>        htab->size_prime_index = nindex;
>      }
>    else
> -    memset (entries, 0, size * sizeof (T *));
> +    memset (entries, 0, size * sizeof (value_type *));
>    htab->n_deleted = 0;
>    htab->n_elements = 0;
>  }
> @@ -639,19 +796,18 @@ hash_table <Descr, Allocator>::empty ()
>     useful when you've already done the lookup and don't want to do it
>     again. */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
>  void
> -hash_table <Descr, Allocator>
> -::clear_slot (T **slot)
> +hash_table <Descriptor, Allocator>::clear_slot (value_type **slot)
>  {
>    if (slot < htab->entries || slot >= htab->entries + htab->size
>        || *slot == HTAB_EMPTY_ENTRY || *slot == HTAB_DELETED_ENTRY)
>      abort ();
>
> -  Descr::remove (*slot);
> +  Descriptor::remove (*slot);
>
> -  *slot = static_cast <T *> (HTAB_DELETED_ENTRY);
> +  *slot = static_cast <value_type *> (HTAB_DELETED_ENTRY);
>    htab->n_deleted++;
>  }
>
> @@ -660,21 +816,21 @@ hash_table <Descr, Allocator>
>     from hash table starting with the given HASH.  If there is no
>     matching element in the hash table, this function does nothing. */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
>  void
> -hash_table <Descr, Allocator>
> -::remove_elt_with_hash (const T *comparable, hashval_t hash)
> +hash_table <Descriptor, Allocator>
> +::remove_elt_with_hash (const compare_type *comparable, hashval_t hash)
>  {
> -  T **slot;
> +  value_type **slot;
>
>    slot = find_slot_with_hash (comparable, hash, NO_INSERT);
>    if (*slot == HTAB_EMPTY_ENTRY)
>      return;
>
> -  Descr::remove (*slot);
> +  Descriptor::remove (*slot);
>
> -  *slot = static_cast <T *> (HTAB_DELETED_ENTRY);
> +  *slot = static_cast <value_type *> (HTAB_DELETED_ENTRY);
>    htab->n_deleted++;
>  }
>
> @@ -683,23 +839,22 @@ hash_table <Descr, Allocator>
>     each live entry.  If CALLBACK returns false, the iteration stops.
>     ARGUMENT is passed as CALLBACK's second argument. */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
>  template <typename Argument,
> -         int (*Callback) (typename Descr::T **slot, Argument argument)>
> +         int (*Callback) (typename Descriptor::value_type **slot, Argument argument)>
>  void
> -hash_table <Descr, Allocator>
> -::traverse_noresize (Argument argument)
> +hash_table <Descriptor, Allocator>::traverse_noresize (Argument argument)
>  {
> -  T **slot;
> -  T **limit;
> +  value_type **slot;
> +  value_type **limit;
>
>    slot = htab->entries;
>    limit = slot + htab->size;
>
>    do
>      {
> -      T *x = *slot;
> +      value_type *x = *slot;
>
>        if (x != HTAB_EMPTY_ENTRY && x != HTAB_DELETED_ENTRY)
>          if (! Callback (slot, argument))
> @@ -712,13 +867,13 @@ hash_table <Descr, Allocator>
>  /* Like traverse_noresize, but does resize the table when it is too empty
>     to improve effectivity of subsequent calls.  */
>
> -template <typename Descr,
> +template <typename Descriptor,
>           template <typename Type> class Allocator>
>  template <typename Argument,
> -         int (*Callback) (typename Descr::T **slot, Argument argument)>
> +         int (*Callback) (typename Descriptor::value_type **slot,
> +                          Argument argument)>
>  void
> -hash_table <Descr, Allocator>
> -::traverse (Argument argument)
> +hash_table <Descriptor, Allocator>::traverse (Argument argument)
>  {
>    size_t size = htab->size;
>    if (elements () * 8 < size && size > 32)
> Index: gcc/alloc-pool.c
> ===================================================================
> --- gcc/alloc-pool.c    (revision 192820)
> +++ gcc/alloc-pool.c    (working copy)
> @@ -22,7 +22,7 @@ along with GCC; see the file COPYING3.
>  #include "config.h"
>  #include "system.h"
>  #include "alloc-pool.h"
> -#include "hashtab.h"
> +#include "hash-table.h"
>
>  #define align_eight(x) (((x+7) >> 3) << 3)
>
> @@ -83,38 +83,42 @@ struct alloc_pool_descriptor
>    int elt_size;
>  };
>
> +struct alloc_pool_hasher : typed_noop_remove <alloc_pool_descriptor>
> +{
> +  typedef alloc_pool_descriptor value_type;
> +  typedef char compare_type;
> +  static inline hashval_t hash (const alloc_pool_descriptor *);
> +  static inline bool equal (const value_type *, const compare_type *);
> +};
> +
>  /* Hashtable mapping alloc_pool names to descriptors.  */
> -static htab_t alloc_pool_hash;
> +static hash_table <alloc_pool_hasher>  alloc_pool_hash;
>
>  /* Hashtable helpers.  */
> -static hashval_t
> -hash_descriptor (const void *p)
> +inline hashval_t
> +alloc_pool_hasher::hash (const value_type *d)
>  {
> -  const struct alloc_pool_descriptor *const d =
> -    (const struct alloc_pool_descriptor * )p;
>    return htab_hash_pointer (d->name);
>  }
> -static int
> -eq_descriptor (const void *p1, const void *p2)
> +
> +inline bool
> +alloc_pool_hasher::equal (const value_type *d,
> +                          const compare_type *p2)
>  {
> -  const struct alloc_pool_descriptor *const d =
> -    (const struct alloc_pool_descriptor *) p1;
>    return d->name == p2;
>  }
>
>  /* For given name, return descriptor, create new if needed.  */
>  static struct alloc_pool_descriptor *
> -alloc_pool_descriptor (const char *name)
> +allocate_pool_descriptor (const char *name)
>  {
>    struct alloc_pool_descriptor **slot;
>
> -  if (!alloc_pool_hash)
> -    alloc_pool_hash = htab_create (10, hash_descriptor, eq_descriptor, NULL);
> +  if (!alloc_pool_hash.is_created ())
> +    alloc_pool_hash.create (10);
>
> -  slot = (struct alloc_pool_descriptor **)
> -    htab_find_slot_with_hash (alloc_pool_hash, name,
> -                             htab_hash_pointer (name),
> -                             INSERT);
> +  slot = alloc_pool_hash.find_slot_with_hash (name,
> +                                             htab_hash_pointer (name), INSERT);
>    if (*slot)
>      return *slot;
>    *slot = XCNEW (struct alloc_pool_descriptor);
> @@ -158,7 +162,7 @@ create_alloc_pool (const char *name, siz
>
>    if (GATHER_STATISTICS)
>      {
> -      struct alloc_pool_descriptor *desc = alloc_pool_descriptor (name);
> +      struct alloc_pool_descriptor *desc = allocate_pool_descriptor (name);
>        desc->elt_size = size;
>        desc->created++;
>      }
> @@ -205,7 +209,7 @@ empty_alloc_pool (alloc_pool pool)
>
>    if (GATHER_STATISTICS)
>      {
> -      struct alloc_pool_descriptor *desc = alloc_pool_descriptor (pool->name);
> +      struct alloc_pool_descriptor *desc = allocate_pool_descriptor
> (pool->name);
>        desc->current -= (pool->elts_allocated - pool->elts_free) *
> pool->elt_size;
>      }
>
> @@ -253,7 +257,7 @@ pool_alloc (alloc_pool pool)
>
>    if (GATHER_STATISTICS)
>      {
> -      struct alloc_pool_descriptor *desc = alloc_pool_descriptor (pool->name);
> +      struct alloc_pool_descriptor *desc = allocate_pool_descriptor
> (pool->name);
>
>        desc->allocated += pool->elt_size;
>        desc->current += pool->elt_size;
> @@ -357,7 +361,7 @@ pool_free (alloc_pool pool, void *ptr)
>
>    if (GATHER_STATISTICS)
>      {
> -      struct alloc_pool_descriptor *desc = alloc_pool_descriptor (pool->name);
> +      struct alloc_pool_descriptor *desc = allocate_pool_descriptor
> (pool->name);
>        desc->current -= pool->elt_size;
>      }
>  }
> @@ -371,19 +375,20 @@ struct output_info
>    unsigned long total_allocated;
>  };
>
> -/* Called via htab_traverse.  Output alloc_pool descriptor pointed out by SLOT
> -   and update statistics.  */
> -static int
> -print_statistics (void **slot, void *b)
> +/* Called via hash_table.traverse.  Output alloc_pool descriptor pointed out by
> +   SLOT and update statistics.  */
> +int
> +print_alloc_pool_statistics (alloc_pool_descriptor **slot,
> +                            struct output_info *i)
>  {
> -  struct alloc_pool_descriptor *d = (struct alloc_pool_descriptor *) *slot;
> -  struct output_info *i = (struct output_info *) b;
> +  struct alloc_pool_descriptor *d = *slot;
>
>    if (d->allocated)
>      {
> -      fprintf (stderr, "%-22s %6d %10lu %10lu(%10lu) %10lu(%10lu)
> %10lu(%10lu)\n", d->name,
> -              d->elt_size, d->created, d->allocated, d->allocated / d->elt_size,
> -              d->peak, d->peak / d->elt_size,
> +      fprintf (stderr,
> +              "%-22s %6d %10lu %10lu(%10lu) %10lu(%10lu) %10lu(%10lu)\n",
> +              d->name, d->elt_size, d->created, d->allocated,
> +              d->allocated / d->elt_size, d->peak, d->peak / d->elt_size,
>                d->current, d->current / d->elt_size);
>        i->total_allocated += d->allocated;
>        i->total_created += d->created;
> @@ -400,14 +405,15 @@ dump_alloc_pool_statistics (void)
>    if (! GATHER_STATISTICS)
>      return;
>
> -  if (!alloc_pool_hash)
> +  if (!alloc_pool_hash.is_created ())
>      return;
>
>    fprintf (stderr, "\nAlloc-pool Kind         Elt size  Pools
> Allocated (elts)            Peak (elts)            Leak (elts)\n");
>    fprintf (stderr,
> "--------------------------------------------------------------------------------------------------------------\n");
>    info.total_created = 0;
>    info.total_allocated = 0;
> -  htab_traverse (alloc_pool_hash, print_statistics, &info);
> +  alloc_pool_hash.traverse <struct output_info *,
> +                           print_alloc_pool_statistics> (&info);
>    fprintf (stderr,
> "--------------------------------------------------------------------------------------------------------------\n");
>    fprintf (stderr, "%-22s           %7lu %10lu\n",
>            "Total", info.total_created, info.total_allocated);
> Index: gcc/dse.c
> ===================================================================
> --- gcc/dse.c   (revision 192820)
> +++ gcc/dse.c   (working copy)
> @@ -654,19 +654,21 @@ clear_alias_set_lookup (alias_set_type a
>
>  struct invariant_group_base_hasher : typed_noop_remove <group_info>
>  {
> -  typedef group_info T;
> -  static inline hashval_t hash (const T *);
> -  static inline bool equal (const T *, const T *);
> +  typedef group_info value_type;
> +  typedef group_info compare_type;
> +  static inline hashval_t hash (const value_type *);
> +  static inline bool equal (const value_type *, const compare_type *);
>  };
>
>  inline bool
> -invariant_group_base_hasher::equal (const T *gi1, const T *gi2)
> +invariant_group_base_hasher::equal (const value_type *gi1,
> +                                   const compare_type *gi2)
>  {
>    return rtx_equal_p (gi1->rtx_base, gi2->rtx_base);
>  }
>
>  inline hashval_t
> -invariant_group_base_hasher::hash (const T *gi)
> +invariant_group_base_hasher::hash (const value_type *gi)
>  {
>    int do_not_record;
>    return hash_rtx (gi->rtx_base, Pmode, &do_not_record, NULL, false);
> Index: gcc/tree-ssa-coalesce.c
> ===================================================================
> --- gcc/tree-ssa-coalesce.c     (revision 192820)
> +++ gcc/tree-ssa-coalesce.c     (working copy)
> @@ -1261,9 +1261,10 @@ coalesce_partitions (var_map map, ssa_co
>
>  struct ssa_name_var_hash : typed_noop_remove <union tree_node>
>  {
> -  typedef union tree_node T;
> -  static inline hashval_t hash (const_tree);
> -  static inline int equal (const_tree, const_tree);
> +  typedef union tree_node value_type;
> +  typedef union tree_node compare_type;
> +  static inline hashval_t hash (const value_type *);
> +  static inline int equal (const value_type *, const compare_type *);
>  };
>
>  inline hashval_t
> @@ -1273,7 +1274,7 @@ ssa_name_var_hash::hash (const_tree n)
>  }
>
>  inline int
> -ssa_name_var_hash::equal (const_tree n1, const_tree n2)
> +ssa_name_var_hash::equal (const value_type *n1, const compare_type *n2)
>  {
>    return SSA_NAME_VAR (n1) == SSA_NAME_VAR (n2);
>  }
> Index: gcc/coverage.c
> ===================================================================
> --- gcc/coverage.c      (revision 192820)
> +++ gcc/coverage.c      (working copy)
> @@ -79,10 +79,11 @@ typedef struct counts_entry
>    struct gcov_ctr_summary summary;
>
>    /* hash_table support.  */
> -  typedef counts_entry T;
> -  static inline hashval_t hash (const counts_entry *);
> -  static int equal (const counts_entry *, const counts_entry *);
> -  static void remove (counts_entry *);
> +  typedef counts_entry value_type;
> +  typedef counts_entry compare_type;
> +  static inline hashval_t hash (const value_type *);
> +  static int equal (const value_type *, const compare_type *);
> +  static void remove (value_type *);
>  } counts_entry_t;
>
>  static GTY(()) struct coverage_data *functions_head = 0;
> @@ -150,20 +151,20 @@ get_gcov_unsigned_t (void)
>  }
>
>  inline hashval_t
> -counts_entry::hash (const counts_entry_t *entry)
> +counts_entry::hash (const value_type *entry)
>  {
>    return entry->ident * GCOV_COUNTERS + entry->ctr;
>  }
>
>  inline int
> -counts_entry::equal (const counts_entry_t *entry1,
> -                    const counts_entry_t *entry2)
> +counts_entry::equal (const value_type *entry1,
> +                    const compare_type *entry2)
>  {
>    return entry1->ident == entry2->ident && entry1->ctr == entry2->ctr;
>  }
>
>  inline void
> -counts_entry::remove (counts_entry_t *entry)
> +counts_entry::remove (value_type *entry)
>  {
>    free (entry->counts);
>    free (entry);
> Index: gcc/tree-ssa-pre.c
> ===================================================================
> --- gcc/tree-ssa-pre.c  (revision 192820)
> +++ gcc/tree-ssa-pre.c  (working copy)
> @@ -173,7 +173,8 @@ typedef struct pre_expr_d : typed_noop_r
>    pre_expr_union u;
>
>    /* hash_table support.  */
> -  typedef pre_expr_d T;
> +  typedef pre_expr_d value_type;
> +  typedef pre_expr_d compare_type;
>    static inline hashval_t hash (const pre_expr_d *);
>    static inline int equal (const pre_expr_d *, const pre_expr_d *);
>  } *pre_expr;
> @@ -186,7 +187,7 @@ typedef struct pre_expr_d : typed_noop_r
>  /* Compare E1 and E1 for equality.  */
>
>  inline int
> -pre_expr_d::equal (const struct pre_expr_d *e1, const struct pre_expr_d *e2)
> +pre_expr_d::equal (const value_type *e1, const compare_type *e2)
>  {
>    if (e1->kind != e2->kind)
>      return false;
> @@ -211,7 +212,7 @@ pre_expr_d::equal (const struct pre_expr
>  /* Hash E.  */
>
>  inline hashval_t
> -pre_expr_d::hash (const struct pre_expr_d *e)
> +pre_expr_d::hash (const value_type *e)
>  {
>    switch (e->kind)
>      {
> @@ -499,9 +500,10 @@ typedef struct expr_pred_trans_d : typed
>    hashval_t hashcode;
>
>    /* hash_table support.  */
> -  typedef expr_pred_trans_d T;
> -  static inline hashval_t hash (const expr_pred_trans_d *);
> -  static inline int equal (const expr_pred_trans_d *, const
> expr_pred_trans_d *);
> +  typedef expr_pred_trans_d value_type;
> +  typedef expr_pred_trans_d compare_type;
> +  static inline hashval_t hash (const value_type *);
> +  static inline int equal (const value_type *, const compare_type *);
>  } *expr_pred_trans_t;
>  typedef const struct expr_pred_trans_d *const_expr_pred_trans_t;
>
> @@ -512,8 +514,8 @@ expr_pred_trans_d::hash (const expr_pred
>  }
>
>  inline int
> -expr_pred_trans_d::equal (const expr_pred_trans_d *ve1,
> -                         const expr_pred_trans_d *ve2)
> +expr_pred_trans_d::equal (const value_type *ve1,
> +                         const compare_type *ve2)
>  {
>    basic_block b1 = ve1->pred;
>    basic_block b2 = ve2->pred;
>
> --
> Lawrence Crowl

Re:

[Diego Novillo]

On 08/03/12 14:55 , Han Shen(沈涵) wrote:

> +2012-03-02 Han Shen <shenhan@google.com <mailto:shenhan@google.com>>
> +
> + Backport r184357 from trunk
> +
> + 2012-02-17 Doug Kwan <dougkwan@google.com <mailto:dougkwan@google.com>>
> +
> + * contrib/testsuite-management/validate_failures.py
> + (GetMakefileValue): Check for cross compilers.
> +

Looks fine.  Thanks.


Diego.

Re:

[Uros Bizjak]

Hello!

> Here is a patch which adds few more splits for AGU stalls avoidance on
> Atom. It also fixes cost model and detects AGU stalls more
> efficiently.
>
> Bootstrapped and checked on x86_64-linux.
>
> 2011-09-02  Enkovich Ilya  <ilya.enkovich@intel.com>
>
>	* config/i386/i386-protos.h (ix86_lea_outperforms): New.
>	(ix86_avoid_lea_for_add): Likewise.
>	(ix86_avoid_lea_for_addr): Likewise.
>	(ix86_split_lea_for_addr): Likewise.
>
>	* config/i386/i386.c (LEA_MAX_STALL): New.
>	(increase_distance): Likewise.
>	(insn_defines_reg): Likewise.
>	(insn_uses_reg_mem): Likewise.
>	(distance_non_agu_define_in_bb): Likewise.
>	(distance_agu_use_in_bb): Likewise.
>	(ix86_lea_outperforms): Likewise.
>	(ix86_ok_to_clobber_flags): Likewise.
>	(ix86_avoid_lea_for_add): Likewise.
>	(ix86_avoid_lea_for_addr): Likewise.
>	(ix86_split_lea_for_addr): Likewise.
>	(distance_non_agu_define): Search in pred BBs added.
>	(distance_agu_use): Search in succ BBs added.
>	(IX86_LEA_PRIORITY): Value changed from 2 to 0.
>	(LEA_SEARCH_THRESHOLD): Now depends on LEA_MAX_STALL.
>	(ix86_lea_for_add_ok): Use ix86_lea_outperforms to make decision.
>
>	* config/i386/i386.md: Splits added to transform lea into a
>	sequence of instructions.

Did you also test on x32 ? H.J.'s x32 page [1] currently says that
Atom LEA optimization is disabled on x32 for some reason.

The patch looks OK to me, with a few nits below.

[1] https://sites.google.com/site/x32abi/

--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h

+bool
+ix86_avoid_lea_for_addr (rtx insn, rtx operands[])
+{
+  unsigned int regno0 = true_regnum (operands[0]) ;
+  unsigned int regno1 = -1;
+  unsigned int regno2 = -1;

Use INVALID_REGNUM here.

+extern void
+ix86_split_lea_for_addr (rtx operands[], enum machine_mode mode)
+{

Missing comment.

--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -5777,6 +5777,41 @@
         (const_string "none")))
    (set_attr "mode" "QI")])

+;; Split non destructive adds if we cannot use lea.
+(define_split
+  [(set (match_operand:SWI48 0 "register_operand" "")
+	(plus:SWI48 (match_operand:SWI48 1 "register_operand" "")
+              (match_operand:SWI48 2 "nonmemory_operand" "")))
+   (clobber (reg:CC FLAGS_REG))]
+  "reload_completed && ix86_avoid_lea_for_add (insn, operands)"
+  [(set (match_dup 0) (match_dup 1))
+   (parallel [(set (match_dup 0) (plus:<MODE> (match_dup 0) (match_dup 2)))
+	      (clobber (reg:CC FLAGS_REG))])
+  ]
+)

Put all closing braces on one line:

	      (clobber (reg:CC FLAGS_REG))])])

+;; Split lea into one or more ALU instructions if profitable.
+(define_split
+  [(set (match_operand:SI 0 "register_operand" "")
+	(subreg:SI (match_operand:DI 1 "lea_address_operand" "") 0))]
+  "reload_completed && ix86_avoid_lea_for_addr (insn, operands)"
+  [(const_int 0)]
+{
+  ix86_split_lea_for_addr (operands, SImode);
+  DONE;
+})

This is valid only for TARGET_64BIT.

Please note that x32 adds quite some different LEA patterns (see
i386.md, line 5466+). I suggest you merge your splitters with these
define_insn patterns into define_insn_and_split, adding "&&
reload_completed && ix86_avoid_lea_for_addr (insn, operands)" as a
split condition.

Uros.

Re:

[Uros Bizjak]

Hello!

> H.J. Lu writes:
> >I am not sure how useful that is for 32bit since it will generate a
> >nop for most machines which do need mfence.
>
> I don't understand what you're saying.  Using "lock orb" should result
> in a memory fence on any IA-32 SMP system, old or new.  It's just a more
> heavyweight way of ordering loads and stores.
>
> The Linux kernel apparently takes the same approach, using either "lock
> addl" or "mfence" depending on whether SSE2 instructions are available
> at compile time.
>   

No, linux dynamically patches its code:

#define mb() alternative("lock; addl $0,0(%%esp)", "mfence", 
X86_FEATURE_XMM2)
#define rmb() alternative("lock; addl $0,0(%%esp)", "lfence", 
X86_FEATURE_XMM2)
#define wmb() alternative("lock; addl $0,0(%%esp)", "sfence", 
X86_FEATURE_XMM)

But anyway, gcc can't do that by itself.

Uros.

Re:

[H.J. Lu]

On Sun, Nov 23, 2008 at 12:36 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> Hello!
>
>> H.J. Lu writes:
>> >I am not sure how useful that is for 32bit since it will generate a
>> >nop for most machines which do need mfence.
>>
>> I don't understand what you're saying.  Using "lock orb" should result
>> in a memory fence on any IA-32 SMP system, old or new.  It's just a more
>> heavyweight way of ordering loads and stores.
>>
>> The Linux kernel apparently takes the same approach, using either "lock
>> addl" or "mfence" depending on whether SSE2 instructions are available
>> at compile time.
>>
>
> No, linux dynamically patches its code:
>
> #define mb() alternative("lock; addl $0,0(%%esp)", "mfence",
> X86_FEATURE_XMM2)
> #define rmb() alternative("lock; addl $0,0(%%esp)", "lfence",
> X86_FEATURE_XMM2)
> #define wmb() alternative("lock; addl $0,0(%%esp)", "sfence",
> X86_FEATURE_XMM)
>
> But anyway, gcc can't do that by itself.
>

Can we call cpuid to initialize  __cpuid_edx if it is referenced?


-- 
H.J.

Re:

[Rafael Espindola]

2008/7/30 Bernhard Fischer <rep.dot.nop@gmail.com>:
> <38a0d8450807240644u34e8a393w4b07d6dc03d3ddee@mail.gmail.com>
> <b798aad50807251405r54ed5a7ej30efe2d12e8a33fe@mail.gmail.com>
> <38a0d8450807290056y64311224q1844c211494e17b8@mail.gmail.com>
> Bcc: dentry@inode.at
> Subject: Re: [patch] record extern weak decls in assemble_external
> Reply-To:
>
> Hi,
>
> Perhaps you could also have a look at PR31537?

I can test it once I commit my patch to call assemble_external for
function decls too.

> TIA,
>

Cheers,
-- 
Rafael Avila de Espindola

Google Ireland Ltd.
Gordon House
Barrow Street
Dublin 4
Ireland

Registered in Dublin, Ireland
Registration Number: 368047

Re:

[Tobias Burnus]

Hi Lee,

I only glanced at your patch.

Lee Millward wrote:
> * gfortran.dg/cmplx_intrinsic_1.f90
>	* gfortran.dg/pr32222.f90: New test.
>	* gfortran.dg/pr32238.f90: New test.
>	* gfortran.dg/pr32242.f90: New test

You miss something like
! { dg-do run }
or
! { dg-do compile }

For those test cases with modules, you should add

! { dg-final { cleanup-modules "MODULE_NAME" } }

to delete the .mod files after compilation.

Tobias

Re:

[Lee Millward]

Hi Tobias,

On 7/6/07, Tobias Burnus <burnus@ph2.uni-koeln.de> wrote:
> Hi Lee,
>
> I only glanced at your patch.
>
> Lee Millward wrote:
> > * gfortran.dg/cmplx_intrinsic_1.f90
> >       * gfortran.dg/pr32222.f90: New test.
> >       * gfortran.dg/pr32238.f90: New test.
> >       * gfortran.dg/pr32242.f90: New test
>
> You miss something like
> ! { dg-do run }
> or
> ! { dg-do compile }

Isn't this on by default?

> For those test cases with modules, you should add
>
> ! { dg-final { cleanup-modules "MODULE_NAME" } }
>
> to delete the .mod files after compilation.
>

Thanks for that, I didn't know about that directive so I'll add it in
and retest.

Cheers,
Lee.

[libstdc++]

[Richard Henderson]

Something like the following is required for Win64, which is a P64 target.

I'm not sure what's required generically to assert that intptr_t is
present; certainly it's available on Linux and Win64...


r~


	* include/bits/locale_facets.tcc (do_put): Convert pointer to
	intptr_t for printing.

--- include/bits/locale_facets.tcc	(revision 123278)
+++ include/bits/locale_facets.tcc	(local)
@@ -1314,7 +1314,7 @@ _GLIBCXX_BEGIN_LDBL_NAMESPACE
       __io.flags((__flags & __fmt) | (ios_base::hex | ios_base::showbase));
 
       __s = _M_insert_int(__s, __io, __fill,
-			  reinterpret_cast<unsigned long>(__v));
+			  reinterpret_cast<intptr_t>(__v));
       __io.flags(__flags);
       return __s;
     }

Re: [libstdc++]

[Paolo Carlini]

Hi,

>Something like the following is required for Win64, which is a P64 target.
>  
>
I'm sure, I already briefly considered this issue, don't remember 
exactly the context.

>I'm not sure what's required generically to assert that intptr_t is
>present; certainly it's available on Linux and Win64...
>  
>
Yes, that's a problem, it's available only in C99, and we can't assume 
that kind of libc, in general. I don't know, maybe we can use long long 
(or maybe better, unsigned long long), and choose the right type via 
something like:

typedef __gnu_cxx::__conditional_type<(sizeof(const void*) <= 
sizeof(unsigned long)), unsigned long, unsigned long long>::__type 
_UIntPtrType;

Is it sufficient to cover all the targets where libstdc++ actually 
works? One, more general issue in the context of C++03, is that if 
_GLIBCXX_USE_LONG_LONG is undefined neither unsigned long long (neither 
uintptr_t for that matter) can be output because operator<<(unsigned 
long long) is missing. Probably, however, that problem isn't real 
anymore, because long long is already in the working draft of C++0x and 
we can assume it to be available unconditionally.

Paolo.

Re:

[Paolo Bonzini]

> Yes, that's a problem, it's available only in C99, and we can't assume
> that kind of libc, in general. I don't know, maybe we can use long long
> (or maybe better, unsigned long long), and choose the right type via
> something like:
> 
> typedef __gnu_cxx::__conditional_type<(sizeof(const void*) <=
> sizeof(unsigned long)), unsigned long, unsigned long long>::__type
> _UIntPtrType;

You can also use

typedef unsigned int __attribute__((__mode__(__pointer__))) _UIntPtrType;

Paolo

re:

[Kenneth Zadeck]

Kaz, 

This patch is fine.  This is after all, your port.

Kenny

> The last one is for SH specific changes.
> 
> SH backend uses the old flow analysis stuff in sh_output_mi_thunk
> and it causes compile time errors in dataflow blanch.  The appended
> patch simply disables them.  The 2nd hunk of the patch below is to
> avoid moving pop insn for the link register to the outside of epilogue
> when -fomit-frame-pointer option is specified .  It looks that more
> optimizations are done in dataflow branch for epilogue.  Changing
> UNSPEC_EH_RETURN to UNSPECV_EH_RETURN and unspec to unspec_volatile
> in eh_return patterns in the last hunk of the patch are needed to
> prevent optimizing eh_return insns away in epilogue.
> The remained part of the patch is lengthy but mechanical.  It adds
> new sibcall_value* patterns to generate (set return_reg (call ...))
> insn instead (call ...) insn for sibcall_value.  These new patterns
> are slightly modified versions of existing sibcall* patterns.
> Without it, df can't compute the correct use of return registers
> when sibcall occurs.
> The first hunk is to remove unneeded brackets.
> 
> With this patch and the other generic changes I've sent, dataflow-branch
> bootstrapped on sh4-unknown-linux-gnu and there are no real regressions
> compared with trunk.
> 
> Regards,
> 	kaz
> --
> 	* config/sh/sh.c (sh_expand_prologue): Remove unneeded brackets.
> 	(sh_expand_epilogue): Add blockage insn when not
> 	frame_pointer_needed.
> 	(sh_output_mi_thunk): Don't use flow analysis here.
> 	* config/sh/sh.md (UNSPEC_EH_RETURN): Remove.
> 	(UNSPECV_EH_RETURN): New macro.
> 	(sibcall_valuei): New.
> 	(sibcall_valuei_pcrel, sibcall_value_pcrel): Likewise.
> 	(sibcall_value_compact, sibcall_value_media): Likewise.
> 	(sibcall_value): Use new sibcall_value* patterns.
> 	(eh_set_ra_si): Use unspec_volatile and UNSPECV_EH_RETURN.
> 	(eh_set_ra_di, eh_set_ra_di+1): Likewise.
> 
> diff -uprN ORIG/dataflow/gcc/config/sh/sh.c LOCAL/dataflow/gcc/config/sh/sh.c
> --- ORIG/dataflow/gcc/config/sh/sh.c	2007-01-24 09:06:27.000000000 +0900
> +++ LOCAL/dataflow/gcc/config/sh/sh.c	2007-01-25 21:41:12.000000000 +0900
> @@ -6181,7 +6181,7 @@ sh_expand_prologue (void)
>         incoming-argument decoder and/or of the return trampoline from
>         the GOT, so make sure the PIC register is preserved and
>         initialized.  */
> -    df_set_regs_ever_live ([PIC_OFFSET_TABLE_REGNUM], true);
> +    df_set_regs_ever_live (PIC_OFFSET_TABLE_REGNUM, true);
>  
>    if (TARGET_SHCOMPACT
>        && (current_function_args_info.call_cookie & ~ CALL_COOKIE_RET_TRAMP(1)))
> @@ -6749,7 +6749,11 @@ sh_expand_epilogue (bool sibcall_p)
>      {
>        save_size = 0;
>        if (TEST_HARD_REG_BIT (live_regs_mask, PR_REG))
> -	pop (PR_REG);
> +	{
> +	  if (!frame_pointer_needed)
> +	    emit_insn (gen_blockage ());
> +	  pop (PR_REG);
> +	}
>        for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
>  	{
>  	  int j = (FIRST_PSEUDO_REGISTER - 1) - i;
> @@ -10308,6 +10312,7 @@ sh_output_mi_thunk (FILE *file, tree thu
>    insn_locators_initialize ();
>    insns = get_insns ();
>  
> +#if 0
>    if (optimize > 0)
>      {
>        /* Initialize the bitmap obstacks.  */
> @@ -10334,6 +10339,14 @@ sh_output_mi_thunk (FILE *file, tree thu
>        else if (flag_pic)
>  	split_all_insns_noflow ();
>      }
> +#else
> +  if (optimize > 0)
> +    {
> +      if (! cfun->cfg)
> +	init_flow ();
> +      split_all_insns_noflow ();
> +    }
> +#endif
>  
>    sh_reorg ();
>  
> @@ -10345,6 +10358,7 @@ sh_output_mi_thunk (FILE *file, tree thu
>    final (insns, file, 1);
>    final_end_function ();
>  
> +#if 0
>    if (optimize > 0)
>      {
>        /* Release all memory allocated by df.  */
> @@ -10358,6 +10372,7 @@ sh_output_mi_thunk (FILE *file, tree thu
>        bitmap_obstack_release (&reg_obstack);
>        bitmap_obstack_release (NULL);
>      }
> +#endif
>  
>    reload_completed = 0;
>    epilogue_completed = 0;
> diff -uprN ORIG/dataflow/gcc/config/sh/sh.md LOCAL/dataflow/gcc/config/sh/sh.md
> --- ORIG/dataflow/gcc/config/sh/sh.md	2007-01-24 09:06:27.000000000 +0900
> +++ LOCAL/dataflow/gcc/config/sh/sh.md	2007-01-27 21:16:27.000000000 +0900
> @@ -135,7 +135,6 @@
>    (UNSPEC_FSINA		16)
>    (UNSPEC_NSB		17)
>    (UNSPEC_ALLOCO	18)
> -  (UNSPEC_EH_RETURN	19)
>    (UNSPEC_TLSGD		20)
>    (UNSPEC_TLSLDM	21)
>    (UNSPEC_TLSIE		22)
> @@ -163,6 +162,7 @@
>    (UNSPECV_CONST8	6)
>    (UNSPECV_WINDOW_END	10)
>    (UNSPECV_CONST_END	11)
> +  (UNSPECV_EH_RETURN	12)
>  ])
>  
>  ;; -------------------------------------------------------------------------
> @@ -8013,15 +8013,197 @@ label:
>    DONE;
>  }")
>  
> -(define_expand "sibcall_value"
> -  [(set (match_operand 0 "" "")
> -	(call (match_operand 1 "" "")
> +(define_insn "sibcall_valuei"
> +  [(set (match_operand 0 "" "=rf")
> +	(call (mem:SI (match_operand:SI 1 "register_operand" "k"))
> +	      (match_operand 2 "" "")))
> +   (use (reg:PSI FPSCR_REG))
> +   (return)]
> +  "TARGET_SH1"
> +  "jmp	@%1%#"
> +  [(set_attr "needs_delay_slot" "yes")
> +   (set (attr "fp_mode")
> +	(if_then_else (eq_attr "fpu_single" "yes")
> +		      (const_string "single") (const_string "double")))
> +   (set_attr "type" "jump_ind")])
> +
> +(define_insn "sibcall_valuei_pcrel"
> +  [(set (match_operand 0 "" "=rf")
> +	(call (mem:SI (match_operand:SI 1 "arith_reg_operand" "k"))
> +	      (match_operand 2 "" "")))
> +   (use (match_operand 3 "" ""))
> +   (use (reg:PSI FPSCR_REG))
> +   (return)]
> +  "TARGET_SH2"
> +  "braf	%1\\n%O3:%#"
> +  [(set_attr "needs_delay_slot" "yes")
> +   (set (attr "fp_mode")
> +	(if_then_else (eq_attr "fpu_single" "yes")
> +		      (const_string "single") (const_string "double")))
> +   (set_attr "type" "jump_ind")])
> +
> +(define_insn_and_split "sibcall_value_pcrel"
> +  [(set (match_operand 0 "" "=rf")
> +	(call (mem:SI (match_operand:SI 1 "symbol_ref_operand" ""))
>  	      (match_operand 2 "" "")))
> -   (match_operand 3 "" "")]
> +   (use (reg:PSI FPSCR_REG))
> +   (clobber (match_scratch:SI 3 "=k"))
> +   (return)]
> +  "TARGET_SH2"
> +  "#"
> +  "reload_completed"
> +  [(const_int 0)]
> +  "
> +{
> +  rtx lab = PATTERN (gen_call_site ());
> +  rtx call_insn;
> +
> +  emit_insn (gen_sym_label2reg (operands[3], operands[1], lab));
> +  call_insn = emit_call_insn (gen_sibcall_valuei_pcrel (operands[0],
> +							operands[3],
> +							operands[2],
> +							copy_rtx (lab)));
> +  SIBLING_CALL_P (call_insn) = 1;
> +  DONE;
> +}"
> +  [(set_attr "needs_delay_slot" "yes")
> +   (set (attr "fp_mode")
> +	(if_then_else (eq_attr "fpu_single" "yes")
> +		      (const_string "single") (const_string "double")))
> +   (set_attr "type" "jump_ind")])
> +
> +(define_insn "sibcall_value_compact"
> +  [(set (match_operand 0 "" "=rf,rf")
> +	(call (mem:SI (match_operand:SI 1 "register_operand" "k,k"))
> +	      (match_operand 2 "" "")))
> +   (return)
> +   (use (match_operand:SI 3 "register_operand" "z,x"))
> +   (use (reg:SI R1_REG))
> +   (use (reg:PSI FPSCR_REG))
> +   ;; We want to make sure the `x' above will only match MACH_REG
> +   ;; because sibcall_epilogue may clobber MACL_REG.
> +   (clobber (reg:SI MACL_REG))]
> +  "TARGET_SHCOMPACT"
> +  "@
> +	jmp	@%1%#
> +	jmp	@%1\\n	sts	%3, r0"
> +  [(set_attr "needs_delay_slot" "yes,no")
> +   (set_attr "length" "2,4")
> +   (set (attr "fp_mode") (const_string "single"))
> +   (set_attr "type" "jump_ind")])
> +
> +(define_insn "sibcall_value_media"
> +  [(set (match_operand 0 "" "=rf")
> +	(call (mem:DI (match_operand 1 "target_reg_operand" "k"))
> +	      (match_operand 2 "" "")))
> +   (use (reg:SI PR_MEDIA_REG))
> +   (return)]
> +  "TARGET_SHMEDIA"
> +  "blink	%1, r63"
> +  [(set_attr "type" "jump_media")])
> +
> +(define_expand "sibcall_value"
> +  [(parallel
> +    [(set (match_operand 0 "arith_reg_operand" "")
> +	  (call (mem:SI (match_operand 1 "arith_reg_operand" ""))
> +	  	(match_operand 2 "" "")))
> +     (match_operand 3 "" "")
> +     (use (reg:PSI FPSCR_REG))
> +     (return)])]
>    ""
>    "
>  {
> -  emit_call_insn (gen_sibcall (operands[1], operands[2], operands[3]));
> +  if (TARGET_SHMEDIA)
> +    {
> +      operands[1] = shmedia_prepare_call_address (operands[1], 1);
> +      emit_call_insn (gen_sibcall_value_media (operands[0], operands[1],
> +					       operands[2]));
> +      DONE;
> +    }
> +  else if (TARGET_SHCOMPACT && operands[3]
> +	   && (INTVAL (operands[3]) & ~ CALL_COOKIE_RET_TRAMP (1)))
> +    {
> +      rtx cookie_rtx = operands[3];
> +      long cookie = INTVAL (cookie_rtx);
> +      rtx func = XEXP (operands[1], 0);
> +      rtx mach, r1;
> +
> +      if (flag_pic)
> +	{
> +	  if (GET_CODE (func) == SYMBOL_REF && ! SYMBOL_REF_LOCAL_P (func))
> +	    {
> +	      rtx reg = gen_reg_rtx (Pmode);
> +
> +	      emit_insn (gen_symGOT2reg (reg, func));
> +	      func = reg;
> +	    }
> +	  else
> +	    func = legitimize_pic_address (func, Pmode, 0);
> +	}
> +
> +      /* FIXME: if we could tell whether all argument registers are
> +	 already taken, we could decide whether to force the use of
> +	 MACH_REG or to stick to R0_REG.  Unfortunately, there's no
> +	 simple way to tell.  We could use the CALL_COOKIE, but we
> +	 can't currently tell a register used for regular argument
> +	 passing from one that is unused.  If we leave it up to reload
> +	 to decide which register to use, it seems to always choose
> +	 R0_REG, which leaves no available registers in SIBCALL_REGS
> +	 to hold the address of the trampoline.  */
> +      mach = gen_rtx_REG (SImode, MACH_REG);
> +      r1 = gen_rtx_REG (SImode, R1_REG);
> +
> +      /* Since such a call function may use all call-clobbered
> +	 registers, we force a mode switch earlier, so that we don't
> +	 run out of registers when adjusting fpscr for the call.  */
> +      emit_insn (gen_force_mode_for_call ());
> +
> +      operands[1]
> +	= function_symbol (NULL, \"__GCC_shcompact_call_trampoline\",
> +			   SFUNC_GOT);
> +      operands[1] = force_reg (SImode, operands[1]);
> +
> +      /* We don't need a return trampoline, since the callee will
> +	 return directly to the upper caller.  */
> +      if (cookie & CALL_COOKIE_RET_TRAMP (1))
> +	{
> +	  cookie &= ~ CALL_COOKIE_RET_TRAMP (1);
> +	  cookie_rtx = GEN_INT (cookie);
> +	}
> +
> +      emit_move_insn (mach, func);
> +      emit_move_insn (r1, cookie_rtx);
> +
> +      emit_call_insn (gen_sibcall_value_compact (operands[0], operands[1],
> +						 operands[2], mach));
> +      DONE;
> +    }
> +  else if (TARGET_SHCOMPACT && flag_pic
> +	   && GET_CODE (XEXP (operands[1], 0)) == SYMBOL_REF
> +	   && ! SYMBOL_REF_LOCAL_P (XEXP (operands[1], 0)))
> +    {
> +      rtx reg = gen_reg_rtx (Pmode);
> +
> +      emit_insn (gen_symGOT2reg (reg, XEXP (operands[1], 0)));
> +      XEXP (operands[1], 0) = reg;
> +    }
> +  if (flag_pic && TARGET_SH2
> +      && GET_CODE (operands[1]) == MEM
> +      && GET_CODE (XEXP (operands[1], 0)) == SYMBOL_REF
> +      /* The PLT needs the PIC register, but the epilogue would have
> +	 to restore it, so we can only use PC-relative PIC calls for
> +	 static functions.  */
> +      && SYMBOL_REF_LOCAL_P (XEXP (operands[1], 0)))
> +    {
> +      emit_call_insn (gen_sibcall_value_pcrel (operands[0],
> +					       XEXP (operands[1], 0),
> +					       operands[2]));
> +      DONE;
> +    }
> +  else
> +    operands[1] = force_reg (SImode, XEXP (operands[1], 0));
> +
> +  emit_call_insn (gen_sibcall_valuei (operands[0], operands[1], operands[2]));
>    DONE;
>  }")
>  
> @@ -9095,19 +9277,22 @@ mov.l\\t1f,r0\\n\\
>  ;; until we know where it will be put in the stack frame.
>  
>  (define_insn "eh_set_ra_si"
> -  [(unspec [(match_operand:SI 0 "register_operand" "r")] UNSPEC_EH_RETURN)
> +  [(unspec_volatile [(match_operand:SI 0 "register_operand" "r")]
> +      UNSPECV_EH_RETURN)
>     (clobber (match_scratch:SI 1 "=&r"))]
>    "! TARGET_SHMEDIA64"
>    "#")
>  
>  (define_insn "eh_set_ra_di"
> -  [(unspec [(match_operand:DI 0 "register_operand" "r")] UNSPEC_EH_RETURN)
> +  [(unspec_volatile [(match_operand:DI 0 "register_operand" "r")]
> +      UNSPECV_EH_RETURN)
>     (clobber (match_scratch:DI 1 "=&r"))]
>    "TARGET_SHMEDIA64"
>    "#")
>  
>  (define_split
> -  [(unspec [(match_operand 0 "register_operand" "")] UNSPEC_EH_RETURN)
> +  [(unspec_volatile [(match_operand 0 "register_operand" "")]
> +      UNSPECV_EH_RETURN)
>     (clobber (match_scratch 1 ""))]
>    "reload_completed"
>    [(const_int 0)]

Re:

[ИнфоПространство]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; format=flowed; charset="windows-1251"; reply-type=original, Size: 2486 bytes --]

  =- ÊÎÐÏÎÐÀÒÈÂÍÛÅ ÌÅÐÎÏÐÈßÒÈß -=
  =- ÍÀ ÎÑÒÎÆÅÍÊÅ ÍÀ 2000 êâ.ì -=

 • êîíôåðåíöèè, ñåìèíàðû, ñîáðàíèÿ
 • âûñòàâêè, ïðåçåíòàöèè, ïðàçäíèêè
 • áàíêåòû, ôóðøåòû

 1. Ôóíêöèîíàëüíàÿ ïðèíàäëåæíîñòü: Öåíòð ïðåäíàçíà÷åí äëÿ ïðîâåäåíèÿ âûñòàâîê, êîíôåðåíöèé, ñåìèíàðîâ, ïðåçåíòàöèé, ïîêàçîâ è ïðàçäíè÷íûõ ìåðîïðèÿòèé.

 2. Ìåñòîíàõîæäåíèå: Èñòîðè÷åñêèé öåíòð, 300 ìåòðîâ îò Õðàìà Õðèñòà Ñïàñèòåëÿ, ì.Êðîïîòêèíñêàÿ, ìèêð-í Îñòîæåíêà, 50 ìåòðîâ îò Ïðå÷èñòåíñêîé íàá.

 3. Òåõíè÷åñêèå õàðàêòåðèñòèêè: Îáùàÿ ïëîùàäü öåíòðà 2000 êâ.ì, óíèâåðñàëüíûé çàë-òðàíñôîðìåð ñ äèàïàçîíîì ïëîùàäåé îò 20 äî 1500 êâ.ì, 2 VIP-çàëà, Êàôå-ïèööåðèÿ-êîíäèòåðñêàÿ.

 4. Îñîáåííîñòè:
 - Ñïåöèàëüíûå âûñòàâî÷íûå ñòåíäû äî ïîòîëêà ñ ðàçëè÷íîé öâåòîâîé è ôóíêöèîíàëüíîé ãàììîé.
 - Âîçìîæíîñòü ýêñïîíèðîâàíèÿ àâòîìîáèëåé.
 - Âñòðîåííîå â ïîòîëîê âûñòàâî÷íîå îñâåùåíèå.
 - Øèðîêèé âûáîð ìåáåëè.
 - 2 ñöåíû
 - 2 âõîäà: öåíòðàëüíûé è òåõíè÷åñêèé.
 - Âûñîêîêà÷åñòâåííîå êîâðîâîå ïîêðûòèå
 - Ïðîôåññèîíàëüíîå çâóêîóñèëèòåëüíîå îáîðóäîâàíèå.

 Êîíôåðåíö-ïàêåò îò 36 ó.å. Âíóòð. êóðñ êîìïàíèè: 1 ó.å.=30 ðóá.

 Äèðåêòîð ïî ðàçâèòèþ áèçíåñà è îðãàíèçàöèè êîðïîðàòèâíûõ ìåðîïðèÿòèé:
 Ñàâðàñîâà Íàòàëüÿ  òåë.: 290-06-21, 290~7~241, 290-0066; ô.: 290-0-649
-------------------------------------------
                 Ìåæäóíàðîäíûé 
                 èíôîðìàöèîííî-
                 âûñòàâî÷íûé öåíòð 

                 =- ÈíôîÏðîñòðàíñòâî -=

                 1-é Çà÷àòüåâñêèé ïåð.,4
                 òåë. (095) 2907241
                 ôàêñ (095) 202-9-245
\0

Re:

[John David Anglin]

> 2005-04-23  Jan-Benedict Glaw  <jbglaw@lug-owl.de>
> 
> 	gcc/config/vax/
> 	* elf.h: Update whitespace.

Ok.  Installed to trunk.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re:

[Ульяна Викентьевна]

Âíèìaíèe àêöèÿ!
Çàêàæè äî 20 äåêàáðÿ Å-Mail paccûëêó è ïîëó÷è áoíyc(paccûëêó ïî icq)....
 
bad@3432.cjb.net 





\0

Re:

[Лора Маратовна]

Âíèìàíèå!
Òîëüêî äî 1 äåêàáðÿ 2OO4 ãîäà âû ñìîæåòå çàêàçàòü Å-Mail paccûëêó(ïëàòíî) è ïîëó÷èòü paccûëêó ïî icq(áåñïëàòíî)....
 
aftermath@e881.cjb.net 





\0

Re:

[Richard Earnshaw]

Mark,

This patch

2001-04-18  Mark Mitchell  <mark@codesourcery.com>

        * loop.c (load_mems): Examine all the instructions in the loop
        before concluding that all jumps branch to the first instruction
        after the loop.

is (perhaps indirectly) breaking the ARM bootstrap.  The code here

8861              /* If this is a jump outside of the loop but not right
8862                 after the end of the loop, we would have to emit new 
fixup
8863                 sequences for each such label.  */
8864              if (JUMP_LABEL (p) != end_label
8865                  && (INSN_UID (JUMP_LABEL (p)) >= max_uid_for_loop
8866                      || INSN_LUID (JUMP_LABEL (p)) < INSN_LUID (loop->
start)
8867                      || INSN_LUID (JUMP_LABEL (p)) > INSN_LUID (loop->
end)))

 tries to dereference the JUMP_LABEL of an addr_vec; but JUMP_LABEL is 
null in this case, so we segfault.

R.

Re:

[grahams]

Jan

Your last checkin to loop.c (revision 1.243) contains a typo.

The typo is indicated in this extract from your patch note your
change reversed the test from "not insn" to "is insn".


@@ -4124,13 +4123,13 @@
 
 		  for (next = NEXT_INSN (dominator); ; next = NEXT_INSN (next))
 		    {
-		      if ((GET_RTX_CLASS (GET_CODE (next)) == 'i'
+		      if ((INSN_P (next)
 			   && (reg_mentioned_p (giv, PATTERN (next))
 			       || reg_set_p (bl2->biv->src_reg, next)))
 			  || GET_CODE (next) == JUMP_INSN)
 			break;
 #ifdef HAVE_cc0
-		      if (GET_RTX_CLASS (GET_CODE (next)) != 'i'
+		      if (INSN_P (next)
		          ^^^^ typo this should be ! INSN_P (next)
 			  || ! sets_cc0_p (PATTERN (next)))
 #endif
 			dominator = next;

Graham

Re:

[Jan Hubicka]

> Jan
> 
> Your last checkin to loop.c (revision 1.243) contains a typo.

Oops, thanks for noticing this.  I am going to fix this under obvious bugfix
rule.

Honza

No Subject

[Nick Clifton]

Hi Guys,

I would like to submit the enclosed patch.  It fixes the --help output
so that target specific options are prefixed with -m, and language
specific options which take arguments can be documented as such by
seperating the option name from its argument by a space.  eg

	  "-I <include directory>"

The code will still search for a match against "-I" (only), but the
--help output will include the " <include directory>" as well.

Cheers
	Nick


Fri Oct 16 10:23:17 1998  Nick Clifton  <nickc@cygnus.com>

	* toplev.c (display_help): Prepend '-m' to target specific
	options. 
	(check_lang_option): Ignore text after end of first word of a
	language specific option.


Index: toplev.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/toplev.c,v
retrieving revision 1.111
diff -p -w -r1.111 toplev.c
*** toplev.c	1998/10/16 00:08:44	1.111
--- toplev.c	1998/10/16 17:26:53
*************** display_help ()
*** 4120,4126 ****
  	  else if (description == NULL)
  	    undoc = 1;
  	  else if (* description != 0)
! 	    doc += printf ("  %-23.23s %s\n", option, description);
  	}
        
  #ifdef TARGET_OPTIONS      
--- 4120,4126 ----
  	  else if (description == NULL)
  	    undoc = 1;
  	  else if (* description != 0)
! 	    doc += printf ("  -m%-21.21s %s\n", option, description);
  	}
        
  #ifdef TARGET_OPTIONS      
*************** display_help ()
*** 4134,4140 ****
  	  else if (description == NULL)
  	    undoc = 1;
  	  else if (* description != 0)
! 	    doc += printf ("  %-23.23s %s\n", option, description);
  	}
  #endif
        if (undoc)
--- 4134,4140 ----
  	  else if (description == NULL)
  	    undoc = 1;
  	  else if (* description != 0)
! 	    doc += printf ("  -m%-21.21s %s\n", option, description);
  	}
  #endif
        if (undoc)
*************** check_lang_option (option, lang_option)
*** 4159,4169 ****
--- 4159,4173 ----
    lang_independent_options * indep_options;
    int    len;
    long    k;
+   char * space;
    
    /* Ignore NULL entries.  */
    if (option == NULL || lang_option == NULL)
      return 0;
  
+   if ((space = strchr (lang_option, ' ')) != NULL)
+     len = space - lang_option;
+   else
      len = strlen (lang_option);
    
    /* If they do not match to the first n characters then fail.  */

Re:

[Jeffrey A Law]

  In message < 199810161730.KAA18379@elmo.cygnus.com >you write:
  > Hi Guys,
  > 
  > I would like to submit the enclosed patch.  It fixes the --help output
  > so that target specific options are prefixed with -m, and language
  > specific options which take arguments can be documented as such by
  > seperating the option name from its argument by a space.  eg
  > 
  > 	  "-I <include directory>"
  > 
  > The code will still search for a match against "-I" (only), but the
  > --help output will include the " <include directory>" as well.
  > 
  > Cheers
  > 	Nick
  > 
  > 
  > Fri Oct 16 10:23:17 1998  Nick Clifton  <nickc@cygnus.com>
  > 
  > 	* toplev.c (display_help): Prepend '-m' to target specific
  > 	options. 
  > 	(check_lang_option): Ignore text after end of first word of a
  > 	language specific option.
  > 
Seems reasonable to me.

Thanks,
jeff

No Subject

[Nick Clifton]

Hi,

  I would like to submit the following patch to the pragma handling
code in c-pragma.c.  It fixes a bug in the handling of '#pragma pack(<n>)' 
which was introduced by the HANDLE_PRAGMA_PACK_PUSH_POP patch.  The
bug was that insert_pack_attributes() was being called when #pragma
pack(<n>) was used, when the funciton is only intended to work with
#pragma pack(push,<n>).

Cheers
	Nick



Tue Oct  6 09:44:53 1998  Nick Clifton  <nickc@cygnus.com>

	* c-pragma.c (insert_pack_attributes): Do not insert
	attributes unless #pragma pack(push,<n>) is in effect.


Index: c-pragma.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/c-pragma.c,v
retrieving revision 1.9
diff -p -r1.9 c-pragma.c
*** c-pragma.c	1998/10/01 10:50:06	1.9
--- c-pragma.c	1998/10/06 16:44:17
*************** insert_pack_attributes (node, attributes
*** 146,152 ****
    tree a;
  
    /* If we are not packing, then there is nothing to do.  */
!   if (maximum_field_alignment == 0)
      return;
  
    /* We are only interested in fields.  */
--- 146,153 ----
    tree a;
  
    /* If we are not packing, then there is nothing to do.  */
!   if (maximum_field_alignment == 0
!       || alignment_stack == NULL)
      return;
  
    /* We are only interested in fields.  */

Re:

[Jeffrey A Law]

  In message < 199810061650.JAA08909@elmo.cygnus.com >you write:
  > Hi,
  > 
  >   I would like to submit the following patch to the pragma handling
  > code in c-pragma.c.  It fixes a bug in the handling of '#pragma pack(<n>)' 
  > which was introduced by the HANDLE_PRAGMA_PACK_PUSH_POP patch.  The
  > bug was that insert_pack_attributes() was being called when #pragma
  > pack(<n>) was used, when the funciton is only intended to work with
  > #pragma pack(push,<n>).
  > 
  > Cheers
  > 	Nick
  > 
  > 
  > 
  > Tue Oct  6 09:44:53 1998  Nick Clifton  <nickc@cygnus.com>
  > 
  > 	* c-pragma.c (insert_pack_attributes): Do not insert
  > 	attributes unless #pragma pack(push,<n>) is in effect.
Fine by me.
jeff