[rfc] exception handling in gimple

[rfc] exception handling in gimple

[Richard Henderson]

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

I've yet to write the ChangeLog entry, and I'm just
re-running the build test cycle after resolving some
merge errors with Matz's last gimple expanson patch,
but I think this patch is pretty close to done.

The previous test run, with vta merge included, had
no C++ regressions, 2 Ada failures that I don't believe
were regressions, and 2 Java failures that I didn't
get around to examining.

I've tested sjlj support as well; there are 10 Ada
regressions wrt dwarf2 that need to be examined, but
there were no C++ regressions, so it's in fairly good
shape.

So this is well beyond the WIP stage, and I'd like
to get some real feedback on it.


r~


[-- Attachment #2: d-eh-6.gz --]
[-- Type: application/x-gzip, Size: 98954 bytes --]

Re: [rfc] exception handling in gimple

[Michael Matz]

Hi,

On Wed, 2 Sep 2009, Richard Henderson wrote:

> So this is well beyond the WIP stage, and I'd like to get some real 
> feedback on it.

I haven't read nearly all of it yet, but one thing I noticed.  You 
introduce some new predicates (which is wonderful :) ), and use them in 
some places where it's not immediately clear they're equivalent:

--- a/gcc/dce.c
+++ b/gcc/dce.c
@@ -81,10 +81,8 @@ deletable_insn_p_1 (rtx body)
     default:
       if (volatile_refs_p (body))
        return false;
-
-      if (flag_non_call_exceptions && may_trap_p (body))
+      if (insn_could_throw_p (body))
        return false;

Or e.g.:

--- a/gcc/dse.c
+++ b/gcc/dse.c
@@ -2531,7 +2531,7 @@ scan_insn (bb_info_t bb_info, rtx insn)
      them.  */
   if ((GET_CODE (PATTERN (insn)) == CLOBBER)
       || volatile_refs_p (PATTERN (insn))
-      || (flag_non_call_exceptions && may_trap_p (PATTERN (insn)))
+      || insn_could_throw_p (insn)

(at least cse.c has a similar hunk)

Your new insn_could_throw_p() always returns true for calls, no matter if 
they possibly are marked as nothrow, or are pure/const.  I think this 
pessimizes the above hunks (though I haven't checked the surroundings).
Further, insn_could_throw_p() takes a full instruction, not a pattern, but 
at least in the first hunk you have the pattern already.

A general remark: The exception machinery and how it's implemented always 
sorely lacked high level internal documentation.  E.g. what exactly is in 
struct eh_landing_pad_d.post_landing_pad vs .landing_pad, and similar, how 
are the high level language constructs implemented with the different 
eh-tree nodes (what's e.g. EH_FILTER and how does it relate to 
try/catch/throw, and so on), .  I somewhat hoped an exception rewrite 
would add something enlightening from the author of also the older 
exception stuff so I can wrap my little mind around it more easily.  In 
the past I always figured out what is supposed to be doing what, but I 
forget after a few months and always have to relearn the ugly details by 
browsing sources instead of comments :)


Ciao,
Michael.

Re: [rfc] exception handling in gimple

[Michael Matz]

Hi,

On Wed, 2 Sep 2009, Richard Henderson wrote:

> So this is well beyond the WIP stage, and I'd like to get some real 
> feedback on it.

[still reading patch linearly, skipping the two large files :) ]

I think this (tree-cfg.c):

-  /* OpenMP directives alter control flow.  */
-  if (is_gimple_omp (t))
-    return true;
+    case GIMPLE_EH_DISPATCH:
+      /* EH_DISPATCH branches to the individual catch handlers at
+        this level of a try or allowed-exceptions region.  It can
+        fallthru to the next statement as well.  */
+      return true;
+
+    case GIMPLE_OMP_PARALLEL:
... and all other GIMPLE_OMP_ stuff explicitely listed ...

is better done like:

switch (code) {
  case call: ...
  case dispatch: ...
  default:
    if (is_gimple_omp (t))
      ...
    else
      break;
}

If we have a nice predicate already we should use it to reduce maintenance 
costs.


Ciao,
Michael.

Re: [rfc] exception handling in gimple

[Richard Henderson]

On 09/08/2009 07:33 AM, Michael Matz wrote:
> I haven't read nearly all of it yet, but one thing I noticed.  You
> introduce some new predicates (which is wonderful :) ), and use them in
> some places where it's not immediately clear they're equivalent:
>
> --- a/gcc/dce.c
> +++ b/gcc/dce.c
> @@ -81,10 +81,8 @@ deletable_insn_p_1 (rtx body)
>       default:
>         if (volatile_refs_p (body))
>          return false;
> -
> -      if (flag_non_call_exceptions&&  may_trap_p (body))
> +      if (insn_could_throw_p (body))
>          return false;

You're right that this one isn't equivalent; I had already found
that bug and moved the insn_could_throw_p check to deleteable_insn_p.

> -      || (flag_non_call_exceptions&&  may_trap_p (PATTERN (insn)))
> +      || insn_could_throw_p (insn)
>
> (at least cse.c has a similar hunk)
>
> Your new insn_could_throw_p() always returns true for calls, no matter if
> they possibly are marked as nothrow, or are pure/const.  I think this
> pessimizes the above hunks (though I haven't checked the surroundings).
> Further, insn_could_throw_p() takes a full instruction, not a pattern, but
> at least in the first hunk you have the pattern already.

Yes, insn_could_throw_p, like stmt_could_throw_p, is supposed to
disregard the on-the-side EH information (in this case REG_EH_REGION)
so that we can tell if the on-the-side EH information is necessary.

You're probably right that DSE could use the insn_nothrow_p predicate
instead.  However, I wanted to make the transformations as close to
direct as possible.  In some instances, like CSE, the pass isn't
prepared to deal with REG_EH_REGION notes at all.  So the fact that
a call is const, and therefore nothrow, doesn't really help since
there's still a REG_EH_REGION 0 note to deal with.

> A general remark: The exception machinery and how it's implemented always
> sorely lacked high level internal documentation.  E.g. what exactly is in
> struct eh_landing_pad_d.post_landing_pad vs .landing_pad, and similar, how
> are the high level language constructs implemented with the different
> eh-tree nodes (what's e.g. EH_FILTER and how does it relate to
> try/catch/throw, and so on), .  I somewhat hoped an exception rewrite
> would add something enlightening from the author of also the older
> exception stuff so I can wrap my little mind around it more easily.  In
> the past I always figured out what is supposed to be doing what, but I
> forget after a few months and always have to relearn the ugly details by
> browsing sources instead of comments :)

I did add a page or two of documentation about the EH lowering process
and the 99 passes that seem to be involved.  But I can certainly add
more that describe the data structures.

I'll finish up some varray to vec conversion I'm in the middle of,
add those docs, and get a revised patch out tonight.


r~

Re: [rfc] exception handling in gimple

[Richard Henderson]

On 09/08/2009 08:16 AM, Michael Matz wrote:
> +    case GIMPLE_OMP_PARALLEL:
> ... and all other GIMPLE_OMP_ stuff explicitely listed ...
>
> is better done like:
>
> switch (code) {
>    case call: ...
>    case dispatch: ...
>    default:
>      if (is_gimple_omp (t))
>        ...
>      else
>        break;
> }
>
> If we have a nice predicate already we should use it to reduce maintenance
> costs.

How about I add a "CASE_GIMPLE_OMP"?  No maintainence overlap
if we implement is_gimple_omp in terms of that.


r~

Re: [rfc] exception handling in gimple

[Richard Henderson]

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

On 09/08/2009 09:26 AM, Richard Henderson wrote:
> How about I add a "CASE_GIMPLE_OMP"? No maintainence overlap
> if we implement is_gimple_omp in terms of that.

I.e. this.


r~

[-- Attachment #2: z --]
[-- Type: text/plain, Size: 3665 bytes --]

diff --git a/gcc/gimple.h b/gcc/gimple.h
index 74fd656..c2b87ef 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -4207,23 +4207,32 @@ gimple_return_set_retval (gimple gs, tree retval)
 
 /* Returns true when the gimple statment STMT is any of the OpenMP types.  */
 
+#define CASE_GIMPLE_OMP				\
+    case GIMPLE_OMP_PARALLEL:			\
+    case GIMPLE_OMP_TASK:			\
+    case GIMPLE_OMP_FOR:			\
+    case GIMPLE_OMP_SECTIONS:			\
+    case GIMPLE_OMP_SECTIONS_SWITCH:		\
+    case GIMPLE_OMP_SINGLE:			\
+    case GIMPLE_OMP_SECTION:			\
+    case GIMPLE_OMP_MASTER:			\
+    case GIMPLE_OMP_ORDERED:			\
+    case GIMPLE_OMP_CRITICAL:			\
+    case GIMPLE_OMP_RETURN:			\
+    case GIMPLE_OMP_ATOMIC_LOAD:		\
+    case GIMPLE_OMP_ATOMIC_STORE:		\
+    case GIMPLE_OMP_CONTINUE
+
 static inline bool
 is_gimple_omp (const_gimple stmt)
 {
-  return (gimple_code (stmt) == GIMPLE_OMP_PARALLEL
-	  || gimple_code (stmt) == GIMPLE_OMP_TASK
-	  || gimple_code (stmt) == GIMPLE_OMP_FOR
-	  || gimple_code (stmt) == GIMPLE_OMP_SECTIONS
-	  || gimple_code (stmt) == GIMPLE_OMP_SECTIONS_SWITCH
-	  || gimple_code (stmt) == GIMPLE_OMP_SINGLE
-	  || gimple_code (stmt) == GIMPLE_OMP_SECTION
-	  || gimple_code (stmt) == GIMPLE_OMP_MASTER
-	  || gimple_code (stmt) == GIMPLE_OMP_ORDERED
-	  || gimple_code (stmt) == GIMPLE_OMP_CRITICAL
-	  || gimple_code (stmt) == GIMPLE_OMP_RETURN
-	  || gimple_code (stmt) == GIMPLE_OMP_ATOMIC_LOAD
-	  || gimple_code (stmt) == GIMPLE_OMP_ATOMIC_STORE
-	  || gimple_code (stmt) == GIMPLE_OMP_CONTINUE);
+  switch (gimple_code (stmt))
+    {
+    CASE_GIMPLE_OMP:
+      return true;
+    default:
+      return false;
+    }
 }
 
 
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index f7b6ee4..c428cea 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -2821,20 +2821,7 @@ is_ctrl_altering_stmt (gimple t)
 	 fallthru to the next statement as well.  */
       return true;
 
-    case GIMPLE_OMP_PARALLEL:
-    case GIMPLE_OMP_TASK:
-    case GIMPLE_OMP_FOR:
-    case GIMPLE_OMP_SECTIONS:
-    case GIMPLE_OMP_SECTIONS_SWITCH:
-    case GIMPLE_OMP_SINGLE:
-    case GIMPLE_OMP_SECTION:
-    case GIMPLE_OMP_MASTER:
-    case GIMPLE_OMP_ORDERED:
-    case GIMPLE_OMP_CRITICAL:
-    case GIMPLE_OMP_RETURN:
-    case GIMPLE_OMP_ATOMIC_LOAD:
-    case GIMPLE_OMP_ATOMIC_STORE:
-    case GIMPLE_OMP_CONTINUE:
+    CASE_GIMPLE_OMP:
       /* OpenMP directives alter control flow.  */
       return true;
 
@@ -4273,17 +4260,6 @@ verify_gimple_debug (gimple stmt ATTRIBUTE_UNUSED)
 static bool
 verify_types_in_gimple_stmt (gimple stmt)
 {
-  if (is_gimple_omp (stmt))
-    {
-      /* OpenMP directives are validated by the FE and never operated
-	 on by the optimizers.  Furthermore, GIMPLE_OMP_FOR may contain
-	 non-gimple expressions when the main index variable has had
-	 its address taken.  This does not affect the loop itself
-	 because the header of an GIMPLE_OMP_FOR is merely used to determine
-	 how to setup the parallel iteration.  */
-      return false;
-    }
-
   switch (gimple_code (stmt))
     {
     case GIMPLE_ASSIGN:
@@ -4322,6 +4298,15 @@ verify_types_in_gimple_stmt (gimple stmt)
     case GIMPLE_EH_DISPATCH:
       return false;
 
+    CASE_GIMPLE_OMP:
+      /* OpenMP directives are validated by the FE and never operated
+	 on by the optimizers.  Furthermore, GIMPLE_OMP_FOR may contain
+	 non-gimple expressions when the main index variable has had
+	 its address taken.  This does not affect the loop itself
+	 because the header of an GIMPLE_OMP_FOR is merely used to determine
+	 how to setup the parallel iteration.  */
+      return false;
+
     case GIMPLE_DEBUG:
       return verify_gimple_debug (stmt);
 

Re: [rfc] exception handling in gimple

[Richard Henderson]

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

Still writing the changelog, but here's the version that's
current in my tree as of this afternoon.  Notable changes
from the last version are: documentation added, varrays
removed, and some bugs fixed.


r~

[-- Attachment #2: d-eh-8.gz --]
[-- Type: application/x-gzip, Size: 106992 bytes --]

Re: [rfc] exception handling in gimple

[Olivier Hainque]

Richard Henderson wrote:
> Still writing the changelog, but here's the version that's
> current in my tree as of this afternoon.

 Very nice to see this area all refreshed :)

 Documentation-wise, FWIW, we have had a blurb about the runtime tables
 organization for while in ada/raise-gcc.c, quoted below. We found it
 very useful on several occasions to help understanding the personality
 routines implementations.

 The intent for this piece was to convey the general structure, which
 in principle belongs to a more common place. There are only tiny bits
 of Ada specific info there, easy to adjust if need be.

 This was written a long time ago, from our understanding of the
 organization at that time, so there might be a few inaccuracies.
 This would easier to maintain and by the most knowledgable
 people if it were in a more common place.

 Anyway, just to let you know that we'd be happy to see this info
 moved/updated to a more common place if you think it would be useful.
 Your current EH circuitry rewrite sounds like a nice possible
 opportunity for this.

 Olivier

--

/* There are three major runtime tables involved, generated by the
   GCC back-end. Contents slightly vary depending on the underlying
   implementation scheme (dwarf zero cost / sjlj).

   =======================================
   * Tables for the dwarf zero cost case *
   =======================================

   call_site []
   -------------------------------------------------------------------
   * region-start | region-length | landing-pad | first-action-index *
   -------------------------------------------------------------------

   Identify possible actions to be taken and where to resume control
   for that when an exception propagates through a pc inside the region
   delimited by start and length.

   A null landing-pad indicates that nothing is to be done.

   Otherwise, first-action-index provides an entry into the action[]
   table which heads a list of possible actions to be taken (see below).

   If it is determined that indeed an action should be taken, that
   is, if one action filter matches the exception being propagated,
   then control should be transfered to landing-pad.

   A null first-action-index indicates that there are only cleanups
   to run there.

   action []
   -------------------------------
   * action-filter | next-action *
   -------------------------------

   This table contains lists (called action chains) of possible actions
   associated with call-site entries described in the call-site [] table.
   There is at most one action list per call-site entry.

   A null action-filter indicates a cleanup.

   Non null action-filters provide an index into the ttypes [] table
   (see below), from which information may be retrieved to check if it
   matches the exception being propagated.

   action-filter > 0  means there is a regular handler to be run,

   action-filter < 0  means there is a some "exception_specification"
                      data to retrieve, which is only relevant for C++
                      and should never show up for Ada.

   next-action indexes the next entry in the list. 0 indicates there is
   no other entry.

   ttypes []
   ---------------
   * ttype-value *
   ---------------

   A null value indicates a catch-all handler in C++, and an "others"
   handler in Ada.

   Non null values are used to match the exception being propagated:
   In C++ this is a pointer to some rtti data, while in Ada this is an
   exception id.

   The special id value 1 indicates an "all_others" handler.

   For C++, this table is actually also used to store "exception
   specification" data. The differentiation between the two kinds
   of entries is made by the sign of the associated action filter,
   which translates into positive or negative offsets from the
   so called base of the table:

   Exception Specification data is stored at positive offsets from
   the ttypes table base, which Exception Type data is stored at
   negative offsets:

   ---------------------------------------------------------------------------

   Here is a quick summary of the tables organization:

          +-- Unwind_Context (pc, ...)
          |
          |(pc)
          |
          |   CALL-SITE[]
          |
          |   +=============================================================+
          |   | region-start + length |  landing-pad   | first-action-index |
          |   +=============================================================+
          +-> |       pc range          0 => no-action   0 => cleanups only |
              |                         !0 => jump @              N --+     |
              +====================================================== | ====+
                                                                      |
                                                                      |
       ACTION []                                                      |
                                                                      |
       +==========================================================+   |
       |              action-filter           |   next-action     |   |
       +==========================================================+   |
       |  0 => cleanup                                            |   |
       | >0 => ttype index for handler ------+  0 => end of chain | <-+
       | <0 => ttype index for spec data     |                    |
       +==================================== | ===================+
                                             |
                                             |
       TTYPES []                             |
                                             |  Offset negated from
                 +=====================+     |  the actual base.
                 |     ttype-value     |     |
    +============+=====================+     |
    |            |  0 => "others"      |     |
    |    ...     |  1 => "all others"  | <---+
    |            |  X => exception id  |
    |  handlers  +---------------------+
    |            |        ...          |
    |    ...     |        ...          |
    |            |        ...          |
    +============+=====================+ <<------ Table base
    |    ...     |        ...          |
    |   specs    |        ...          | (should not see negative filter
    |    ...     |        ...          |  values for Ada).
    +============+=====================+


   ============================
   * Tables for the sjlj case *
   ============================

   So called "function contexts" are pushed on a context stack by calls to
   _Unwind_SjLj_Register on function entry, and popped off at exit points by
   calls to _Unwind_SjLj_Unregister. The current call_site for a function is
   updated in the function context as the function's code runs along.

   The generic unwinding engine in _Unwind_RaiseException walks the function
   context stack and not the actual call chain.

   The ACTION and TTYPES tables remain unchanged, which allows to search them
   during the propagation phase to determine whether or not the propagated
   exception is handled somewhere. When it is, we only "jump" up once directly
   to the context where the handler will be found. Besides, this allows "break
   exception unhandled" to work also

   The CALL-SITE table is setup differently, though: the pc attached to the
   unwind context is a direct index into the table, so the entries in this
   table do not hold region bounds any more.

   A special index (-1) is used to indicate that no action is possibly
   connected with the context at hand, so null landing pads cannot appear
   in the table.

   Additionally, landing pad values in the table do not represent code address
   to jump at, but so called "dispatch" indices used by a common landing pad
   for the function to switch to the appropriate post-landing-pad.

   +-- Unwind_Context (pc, ...)
   |
   | pc = call-site index
   |  0 => terminate (should not see this for Ada)
   | -1 => no-action
   |
   |   CALL-SITE[]
   |
   |   +=====================================+
   |   |  landing-pad   | first-action-index |
   |   +=====================================+
   +-> |                  0 => cleanups only |
       | dispatch index             N        |
       +=====================================+

Re: [rfc] exception handling in gimple

[Eric Botcazou]

> Still writing the changelog, but here's the version that's
> current in my tree as of this afternoon.  Notable changes
> from the last version are: documentation added, varrays
> removed, and some bugs fixed.

Nice documentation.  Some proofreading:

An exception is an event that can be "thrown" from within a
+   function. This event can then be "caught" by the callers of
+   the function.

Missing space before "This".

+   The representation of exceptions changes several times during
+   the compliation process:

compilation

+   the TRY nodes to straght-line code.  Statements that had been within

straight-line

+   which allows us to assign map the set of types manipulated by
+   all of the CATCH and EH_FILTER regions to a set of integers.

assign or map?

+   GIMPLE_EH_DISPATCH statements to switch or conditional branches

to a switch?

-- 
Eric Botcazou

Re: [rfc] exception handling in gimple

[Michael Matz]

Hi,

On Wed, 9 Sep 2009, Olivier Hainque wrote:

>  Documentation-wise, FWIW, we have had a blurb about the runtime tables 
>  organization for while in ada/raise-gcc.c, quoted below. We found it 
>  very useful on several occasions to help understanding the personality 
>  routines implementations.

Yes, I found it also very useful in the past to understand what the LSDA 
data actually contains :)


Ciao,
Michael.

Re: [rfc] exception handling in gimple

[Michael Matz]

Hi,

On Tue, 8 Sep 2009, Richard Henderson wrote:

> Still writing the changelog, but here's the version that's current in my 
> tree as of this afternoon.  Notable changes from the last version are: 
> documentation added,

Marvelous.

While reading the except.c hunks I noticed this:

+      /* Handle three cases here:
+        (1) no reachable handler and no edge,
+        (2) reachable handler and an existing edge to post-landing-pad,
+        (3) reachable handler and a missing edge.
+        ??? Not sure why case 3 slips through to this point, it sure
+        seems like we shouldn't be introducing new statements that can
+        throw during the gimple->rtl lowering process.  The alternative
+        is that we're losing edges somewhere, which is equally bad.  */

Have you meanwhile found out why case 3 can happen there?  Because, as you 
say, expansion shouldn't introduce newly throwing things.  Perhaps 
something to do with builtin expansion that happen to expand (for whatever 
reason) to normal calls that suddenly might throw?

Also, in the sjlj RTL expanders it's still building a cmp-jump sequence 
in the (one) landing pad.  Shouldn't this eventually also be done on the 
tree-side already (to be able to use a switch)?

+/* YOUAREHERE: Replace with expand_resx.  */
+
+rtx
+expand_builtin_eh_copy_values (tree exp)

I AM WHERE? :)


Ciao,
Michael.

Re: [rfc] exception handling in gimple

[Michael Matz]

Hi,

On Tue, 8 Sep 2009, Richard Henderson wrote:

> On 09/08/2009 09:26 AM, Richard Henderson wrote:
> > How about I add a "CASE_GIMPLE_OMP"? No maintainence overlap
> > if we implement is_gimple_omp in terms of that.
> 
> I.e. this.

Works for me.


Ciao,
Michael.

Re: [rfc] exception handling in gimple

[Richard Henderson]

On 09/09/2009 07:34 AM, Michael Matz wrote:
> While reading the except.c hunks I noticed this:
>
> +      /* Handle three cases here:
> +        (1) no reachable handler and no edge,
> +        (2) reachable handler and an existing edge to post-landing-pad,
> +        (3) reachable handler and a missing edge.
> +        ??? Not sure why case 3 slips through to this point, it sure
> +        seems like we shouldn't be introducing new statements that can
> +        throw during the gimple->rtl lowering process.  The alternative
> +        is that we're losing edges somewhere, which is equally bad.  */
>
> Have you meanwhile found out why case 3 can happen there?

No, though I have a theory.  I'll experiment with that today.

> Also, in the sjlj RTL expanders it's still building a cmp-jump sequence
> in the (one) landing pad.  Shouldn't this eventually also be done on the
> tree-side already (to be able to use a switch)?

I'm not sure I can generate the same rtl from trees, because of where I
go off and place all the bits from the parts of 
expand_builtin_setjmp_receiver.

I do think I can rearrange things such that I can call back into 
expand_case though.  We'll see.


> +/* YOUAREHERE: Replace with expand_resx.  */

Oops.  =)


r~

Re: [rfc] exception handling in gimple

[Richard Henderson]

On 09/09/2009 01:39 AM, Eric Botcazou wrote:
> Nice documentation.  Some proofreading:

All fixed, thanks.


r~

Re: [rfc] exception handling in gimple

[Richard Henderson]

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

On 09/09/2009 07:34 AM, Michael Matz wrote:
> Have you meanwhile found out why case 3 can happen there?  Because, as you
> say, expansion shouldn't introduce newly throwing things.  Perhaps
> something to do with builtin expansion that happen to expand (for whatever
> reason) to normal calls that suddenly might throw?

I've regtested with the first patch applied, and the assert doesn't 
trigger anymore.  I'm not sure what was going on originally.

> Also, in the sjlj RTL expanders it's still building a cmp-jump sequence
> in the (one) landing pad.  Shouldn't this eventually also be done on the
> tree-side already (to be able to use a switch)?

The second patch generates a switch statement for sjlj dispatch from 
within the finish_eh_generation pass.  It works on a spot-checked test 
case; I'll let the full build cycle run overnight.


r~

[-- Attachment #2: commit-18d098b --]
[-- Type: text/plain, Size: 1822 bytes --]

commit 18d098b8ac53d32e1d62e6abc93bff9d7d42979f
Author: Richard Henderson <rth@twiddle.net>
Date:   Wed Sep 9 12:00:19 2009 -0700

    Remove fixup for missing EH edge during finish_eh_generation.

diff --git a/gcc/except.c b/gcc/except.c
index 7a9ea8f..18916f2 100644
--- a/gcc/except.c
+++ b/gcc/except.c
@@ -1388,29 +1388,21 @@ finish_eh_generation (void)
 	if (e->flags & EDGE_EH)
 	  break;
 
-      /* Handle three cases here:
-	 (1) no reachable handler and no edge,
-	 (2) reachable handler and an existing edge to post-landing-pad,
-	 (3) reachable handler and a missing edge.
-	 ??? Not sure why case 3 slips through to this point, it sure
-	 seems like we shouldn't be introducing new statements that can
-	 throw during the gimple->rtl lowering process.  The alternative
-	 is that we're losing edges somewhere, which is equally bad.  */
-      if (lp == NULL)
-	{
-	  gcc_assert (e == NULL);
-	  continue;
-	}
-      if (e == NULL)
-	e = make_edge (bb, BLOCK_FOR_INSN (lp->landing_pad), EDGE_EH);
-      else
+      /* We should not have generated any new throwing insns during this
+	 pass, and we should not have lost any EH edges, so we only need
+	 to handle two cases here:
+	 (1) reachable handler and an existing edge to post-landing-pad,
+	 (2) no reachable handler and no edge.  */
+      gcc_assert ((lp != NULL) == (e != NULL));
+      if (lp != NULL)
 	{
 	  gcc_assert (BB_HEAD (e->dest) == label_rtx (lp->post_landing_pad));
-          redirect_edge_succ (e, BLOCK_FOR_INSN (lp->landing_pad));
+
+	  redirect_edge_succ (e, BLOCK_FOR_INSN (lp->landing_pad));
+	  e->flags |= (CALL_P (BB_END (bb))
+		       ? EDGE_ABNORMAL | EDGE_ABNORMAL_CALL
+		       : EDGE_ABNORMAL);
 	}
-      e->flags |= (CALL_P (BB_END (bb))
-		   ? EDGE_ABNORMAL | EDGE_ABNORMAL_CALL
-		   : EDGE_ABNORMAL);
     }
 }
 

[-- Attachment #3: d-eh-sjlj-case --]
[-- Type: text/plain, Size: 9457 bytes --]

	* cfgbuild.c (find_bb_boundaries): Don't start a new BB for
	tablejump labels.
	* except.c (sjlj_emit_dispatch_table): Build a switch statement.
	* expr.c (expand_expr_real_2): Handle NON_LVALUE_EXPR.
	* gimple.c (gimple_build_switch_nlabels): Rename from
	gimple_build_switch_1; export; handle null default_label.
	(gimple_build_switch): Handle null default_label.
	(gimple_build_switch_vec): Likewise.
	* gimple.h (gimple_build_switch_nlabels): Declare.

diff --git a/gcc/cfgbuild.c b/gcc/cfgbuild.c
index a7827ab..7d87a7a 100644
--- a/gcc/cfgbuild.c
+++ b/gcc/cfgbuild.c
@@ -444,8 +444,10 @@ find_bb_boundaries (basic_block bb)
     {
       enum rtx_code code = GET_CODE (insn);
 
-      /* On code label, split current basic block.  */
-      if (code == CODE_LABEL)
+      /* In case we've previously seen an insn that effects a control
+	 flow transfer, split the block.  */
+      if ((flow_transfer_insn || code == CODE_LABEL)
+	  && inside_basic_block_p (insn))
 	{
 	  fallthru = split_block (bb, PREV_INSN (insn));
 	  if (flow_transfer_insn)
@@ -463,36 +465,10 @@ find_bb_boundaries (basic_block bb)
 	  bb = fallthru->dest;
 	  remove_edge (fallthru);
 	  flow_transfer_insn = NULL_RTX;
-	  if (LABEL_ALT_ENTRY_P (insn))
+	  if (code == CODE_LABEL && LABEL_ALT_ENTRY_P (insn))
 	    make_edge (ENTRY_BLOCK_PTR, bb, 0);
 	}
 
-      /* __builtin_unreachable () may cause a barrier to be emitted in
-	 the middle of a BB.  We need to split it in the same manner
-	 as if the barrier were preceded by a control_flow_insn_p
-	 insn.  */
-      if (code == BARRIER && !flow_transfer_insn)
-	flow_transfer_insn = prev_nonnote_insn_bb (insn);
-
-      /* In case we've previously seen an insn that effects a control
-	 flow transfer, split the block.  */
-      if (flow_transfer_insn && inside_basic_block_p (insn))
-	{
-	  fallthru = split_block (bb, PREV_INSN (insn));
-	  BB_END (bb) = flow_transfer_insn;
-
-	  /* Clean up the bb field for the insns between the blocks.  */
-	  for (x = NEXT_INSN (flow_transfer_insn);
-	       x != BB_HEAD (fallthru->dest);
-	       x = NEXT_INSN (x))
-	    if (!BARRIER_P (x))
-	      set_block_for_insn (x, NULL);
-
-	  bb = fallthru->dest;
-	  remove_edge (fallthru);
-	  flow_transfer_insn = NULL_RTX;
-	}
-
       if (control_flow_insn_p (insn))
 	flow_transfer_insn = insn;
       if (insn == end)
diff --git a/gcc/except.c b/gcc/except.c
index 18916f2..eecb7a1 100644
--- a/gcc/except.c
+++ b/gcc/except.c
@@ -1226,12 +1226,13 @@ sjlj_emit_dispatch_table (rtx dispatch_label, int num_dispatch)
   enum machine_mode unwind_word_mode = targetm.unwind_word_mode ();
   enum machine_mode filter_mode = targetm.eh_return_filter_mode ();
   eh_landing_pad lp;
-  rtx mem, dispatch, seq, fc, before, exc_ptr_reg, filter_reg;
+  rtx mem, seq, fc, before, exc_ptr_reg, filter_reg;
   rtx first_reachable_label;
   basic_block bb;
   eh_region r;
   edge e;
   int i, disp_index;
+  gimple switch_stmt;
 
   fc = crtl->eh.sjlj_fc;
 
@@ -1252,12 +1253,7 @@ sjlj_emit_dispatch_table (rtx dispatch_label, int num_dispatch)
     = gen_rtx_EXPR_LIST (VOIDmode, dispatch_label, forced_labels);
 #endif
 
-  /* Load up dispatch index, exc_ptr and filter values from the
-     function context.  */
-  mem = adjust_address (fc, TYPE_MODE (integer_type_node),
-			sjlj_fc_call_site_ofs);
-  dispatch = copy_to_reg (mem);
-
+  /* Load up exc_ptr and filter values from the function context.  */
   mem = adjust_address (fc, unwind_word_mode, sjlj_fc_data_ofs);
   if (unwind_word_mode != ptr_mode)
     {
@@ -1276,11 +1272,24 @@ sjlj_emit_dispatch_table (rtx dispatch_label, int num_dispatch)
   filter_reg = force_reg (filter_mode, mem);
 
   /* Jump to one of the directly reachable regions.  */
-  /* ??? This really ought to be using a switch statement.  */
 
   disp_index = 0;
   first_reachable_label = NULL;
 
+  /* If there's exactly one call site in the function, don't bother
+     generating a switch statement.  */
+  switch_stmt = NULL;
+  if (num_dispatch > 1)
+    {
+      tree disp;
+
+      mem = adjust_address (fc, TYPE_MODE (integer_type_node),
+			    sjlj_fc_call_site_ofs);
+      disp = make_tree (integer_type_node, mem);
+
+      switch_stmt = gimple_build_switch_nlabels (num_dispatch, disp, NULL);
+    }
+
   for (i = 1; VEC_iterate (eh_landing_pad, cfun->eh->lp_array, i, lp); ++i)
     if (lp && lp->post_landing_pad)
       {
@@ -1289,10 +1298,24 @@ sjlj_emit_dispatch_table (rtx dispatch_label, int num_dispatch)
 	start_sequence ();
 
 	lp->landing_pad = dispatch_label;
-	label = gen_label_rtx ();
+
+	if (num_dispatch > 1)
+	  {
+	    tree t_label, case_elt;
+
+	    t_label = create_artificial_label (UNKNOWN_LOCATION);
+	    case_elt = build3 (CASE_LABEL_EXPR, void_type_node,
+			       build_int_cst (NULL, disp_index),
+			       NULL, t_label);
+	    gimple_switch_set_label (switch_stmt, disp_index, case_elt);
+
+	    label = label_rtx (t_label);
+	  }
+	else
+	  label = gen_label_rtx ();
+
 	if (disp_index == 0)
 	  first_reachable_label = label;
-
 	emit_label (label);
 
 	r = lp->region;
@@ -1310,18 +1333,26 @@ sjlj_emit_dispatch_table (rtx dispatch_label, int num_dispatch)
 	e->count = bb->count;
 	e->probability = REG_BR_PROB_BASE;
 
-	emit_cmp_and_jump_insns (dispatch, GEN_INT (disp_index), EQ, NULL_RTX,
-				 TYPE_MODE (integer_type_node), 0, label);
-
 	disp_index++;
       }
   gcc_assert (disp_index == num_dispatch);
-  expand_builtin_trap ();
+
+  if (num_dispatch > 1)
+    {
+      expand_case (switch_stmt);
+      expand_builtin_trap ();
+    }
 
   seq = get_insns ();
   end_sequence ();
 
   bb = emit_to_new_bb_before (seq, first_reachable_label);
+  if (num_dispatch == 1)
+    {
+      e = make_edge (bb, bb->next_bb, EDGE_FALLTHRU);
+      e->count = bb->count;
+      e->probability = REG_BR_PROB_BASE;
+    }
 }
 
 static void
@@ -1335,7 +1366,7 @@ sjlj_build_landing_pads (void)
   VEC_safe_grow (int, heap, sjlj_lp_call_site_index, num_dispatch);
 
   num_dispatch = sjlj_assign_call_site_values ();
-  if (num_dispatch)
+  if (num_dispatch > 0)
     {
       rtx dispatch_label = gen_label_rtx ();
       int align = STACK_SLOT_ALIGNMENT (sjlj_fc_type_node,
diff --git a/gcc/expr.c b/gcc/expr.c
index ab0ce23..2c235d0 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -7237,6 +7237,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode,
 
   switch (code)
     {
+    case NON_LVALUE_EXPR:
     case PAREN_EXPR:
     CASE_CONVERT:
       if (treeop0 == error_mark_node)
diff --git a/gcc/gimple.c b/gcc/gimple.c
index c32805f..2cda548 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -790,14 +790,15 @@ gimple_build_resx (int region)
    NLABELS is the number of labels in the switch excluding the default.
    DEFAULT_LABEL is the default label for the switch statement.  */
 
-static inline gimple 
-gimple_build_switch_1 (unsigned nlabels, tree index, tree default_label)
+gimple 
+gimple_build_switch_nlabels (unsigned nlabels, tree index, tree default_label)
 {
   /* nlabels + 1 default label + 1 index.  */
   gimple p = gimple_build_with_ops (GIMPLE_SWITCH, ERROR_MARK,
-				    nlabels + 1 + 1);
+				    1 + (default_label != NULL) + nlabels);
   gimple_switch_set_index (p, index);
-  gimple_switch_set_default_label (p, default_label);
+  if (default_label)
+    gimple_switch_set_default_label (p, default_label);
   return p;
 }
 
@@ -812,15 +813,14 @@ gimple
 gimple_build_switch (unsigned nlabels, tree index, tree default_label, ...)
 {
   va_list al;
-  unsigned i;
-  gimple p;
-  
-  p = gimple_build_switch_1 (nlabels, index, default_label);
+  unsigned i, offset;
+  gimple p = gimple_build_switch_nlabels (nlabels, index, default_label);
 
   /* Store the rest of the labels.  */
   va_start (al, default_label);
-  for (i = 1; i <= nlabels; i++)
-    gimple_switch_set_label (p, i, va_arg (al, tree));
+  offset = (default_label != NULL);
+  for (i = 0; i < nlabels; i++)
+    gimple_switch_set_label (p, i + offset, va_arg (al, tree));
   va_end (al);
 
   return p;
@@ -836,14 +836,13 @@ gimple_build_switch (unsigned nlabels, tree index, tree default_label, ...)
 gimple
 gimple_build_switch_vec (tree index, tree default_label, VEC(tree, heap) *args)
 {
-  unsigned i;
-  unsigned nlabels = VEC_length (tree, args);
-  gimple p = gimple_build_switch_1 (nlabels, index, default_label);
+  unsigned i, offset, nlabels = VEC_length (tree, args);
+  gimple p = gimple_build_switch_nlabels (nlabels, index, default_label);
 
-  /*  Put labels in labels[1 - (nlabels + 1)].
-     Default label is in labels[0].  */
-  for (i = 1; i <= nlabels; i++)
-    gimple_switch_set_label (p, i, VEC_index (tree, args, i - 1));
+  /* Copy the labels from the vector to the switch statement.  */
+  offset = (default_label != NULL);
+  for (i = 0; i < nlabels; i++)
+    gimple_switch_set_label (p, i + offset, VEC_index (tree, args, i));
 
   return p;
 }
diff --git a/gcc/gimple.h b/gcc/gimple.h
index c2b87ef..92aae60 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -794,6 +794,7 @@ gimple gimple_build_try (gimple_seq, gimple_seq, enum gimple_try_flags);
 gimple gimple_build_wce (gimple_seq);
 gimple gimple_build_resx (int);
 gimple gimple_build_eh_dispatch (int);
+gimple gimple_build_switch_nlabels (unsigned, tree, tree);
 gimple gimple_build_switch (unsigned, tree, tree, ...);
 gimple gimple_build_switch_vec (tree, tree, VEC(tree,heap) *);
 gimple gimple_build_omp_parallel (gimple_seq, tree, tree, tree);

Re: [rfc] exception handling in gimple

[Richard Henderson]

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

On 09/08/2009 05:07 PM, Richard Henderson wrote:
> Still writing the changelog, but here's the version that's
> current in my tree as of this afternoon. Notable changes
> from the last version are: documentation added, varrays
> removed, and some bugs fixed.

Here's the changelog for the patch I posted.


r~

[-- Attachment #2: d-eh-9.cl --]
[-- Type: application/simple-filter+xml, Size: 18528 bytes --]

Re: [rfc] exception handling in gimple

[Richard Henderson]

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

One additional fix from the d-eh-8 patch:

Two new openmp failures after merging from mainline
and pulling in the free_lang_data changes.  We weren't
getting std::terminate properly mangled.


r~

[-- Attachment #2: commit-5a6851a --]
[-- Type: text/plain, Size: 788 bytes --]

commit 5a6851a6135c91044836e53f4963cc9dfa772825
Author: Richard Henderson <rth@twiddle.net>
Date:   Wed Sep 9 13:14:32 2009 -0700

    Mark must_not_throw failure_decl as used.

diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index 68d25a4..ff6dc8f 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -1741,6 +1741,11 @@ lower_eh_must_not_throw (struct leh_state *state, gimple tp)
     = gimple_eh_must_not_throw_fndecl (inner);
   this_region->u.must_not_throw.failure_loc = gimple_location (tp);
 
+  /* In order to get mangling applied to this decl, we must mark it
+     used now.  Otherwise, pass_ipa_free_lang_data won't think it
+     needs to happen.  */
+  TREE_USED (this_region->u.must_not_throw.failure_decl) = 1;
+
   this_state = *state;
   this_state.cur_region = this_region;
 

Re: [rfc] exception handling in gimple

[Michael Matz]

Hi,

On Wed, 9 Sep 2009, Richard Henderson wrote:

> > Also, in the sjlj RTL expanders it's still building a cmp-jump 
> > sequence in the (one) landing pad.  Shouldn't this eventually also be 
> > done on the tree-side already (to be able to use a switch)?
> 
> The second patch generates a switch statement for sjlj dispatch from 
> within the finish_eh_generation pass.  It works on a spot-checked test 
> case; I'll let the full build cycle run overnight.

Cool.

@@ -7237,6 +7237,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode,

   switch (code)
     {
+    case NON_LVALUE_EXPR:
     case PAREN_EXPR:
     CASE_CONVERT:
       if (treeop0 == error_mark_node)

Blaehh... reminds me that I once wanted to try to get rid of that tree 
code, or at least that fold produces it when outside gimple form (the 
latter probably is the reason that you need that hunk, as you build a 
new gimple switch while expanding).


Ciao,
Michael.

Re: [rfc] exception handling in gimple

[Michael Matz]

Hi,

On Wed, 9 Sep 2009, Richard Henderson wrote:

> Two new openmp failures after merging from mainline and pulling in the 
> free_lang_data changes.  We weren't getting std::terminate properly 
> mangled.

I'd try it on trunk now, still some time left until end of September :)


Ciao,
Michael.

Re: [rfc] exception handling in gimple

[Joseph S. Myers]

On Thu, 10 Sep 2009, Michael Matz wrote:

> +    case NON_LVALUE_EXPR:
>      case PAREN_EXPR:
>      CASE_CONVERT:
>        if (treeop0 == error_mark_node)
> 
> Blaehh... reminds me that I once wanted to try to get rid of that tree 
> code, or at least that fold produces it when outside gimple form (the 
> latter probably is the reason that you need that hunk, as you build a 
> new gimple switch while expanding).

Finishing the existing incomplete transition from the separate NOP_EXPR 
and CONVERT_EXPR tree codes to just having CONVERT_EXPR would be nice as 
well.  Most places do now treat them the same via CASE_CONVERT etc.; I 
don't know what we have left that might receive both codes and treats them 
differently, that needs resolving before one of the codes can go away.

The C front end should no longer need fold to generate NON_LVALUE_EXPR to 
ensure invalid code is diagnosed; lvalue checks are now done before 
folding and c_fully_fold_internal removes any NON_LVALUE_EXPRs created by 
fold.  I don't know what requirements the C++ front end may still place on 
fold in this regard.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [rfc] exception handling in gimple

[Richard Henderson]

On 09/10/2009 03:49 AM, Michael Matz wrote:
> +    case NON_LVALUE_EXPR:
>       case PAREN_EXPR:
>       CASE_CONVERT:
>         if (treeop0 == error_mark_node)
>
> Blaehh... reminds me that I once wanted to try to get rid of that tree
> code, or at least that fold produces it when outside gimple form (the
> latter probably is the reason that you need that hunk, as you build a
> new gimple switch while expanding).

Actually, it's generated within expand_case itself.
It folds (INDEX - 0) to NON_LVALUE_EXPR(INDEX).

I've no idea how this gets handled differently for
switch statements coming from gimple originally.


r~

Re: [rfc] exception handling in gimple

[Richard Henderson]

On 09/10/2009 03:55 AM, Michael Matz wrote:
> I'd try it on trunk now, still some time left until end of September :)

Huh?  I am trying it on trunk.  I just meant that a recent
pull from mainline brought in new failures.


r~

Re: [rfc] exception handling in gimple

[Richard Henderson]

On 09/10/2009 03:49 AM, Michael Matz wrote:
> +    case NON_LVALUE_EXPR:
>       case PAREN_EXPR:
>       CASE_CONVERT:
>         if (treeop0 == error_mark_node)
>
> Blaehh... reminds me that I once wanted to try to get rid of that tree
> code, or at least that fold produces it when outside gimple form (the
> latter probably is the reason that you need that hunk, as you build a
> new gimple switch while expanding).

Actually, it's generated within expand_case itself.
It folds (INDEX - 0) to NON_LVALUE_EXPR(INDEX).

I've no idea how this gets handled differently for
switch statements coming from gimple originally.


r~

Re: [rfc] exception handling in gimple

[Michael Matz]

Hi,

On Thu, 10 Sep 2009, Richard Henderson wrote:

> On 09/10/2009 03:55 AM, Michael Matz wrote:
> > I'd try it on trunk now, still some time left until end of September :)
> 
> Huh?  I am trying it on trunk.

Oh, I meant trying in the sense of forcing others to try.  By checking in 
:)


Ciao,
Michael.

Re: [rfc] exception handling in gimple

[Richard Henderson]

On 09/11/2009 03:42 AM, Michael Matz wrote:
> Oh, I meant trying in the sense of forcing others to try.  By checking in
> :)

An idea to which I am not opposed.  I think I'll wait til Monday,
when I'll actually be around to deal with the fallout.


r~

Re: [rfc] exception handling in gimple

[H.J. Lu]

On Tue, Sep 8, 2009 at 5:07 PM, Richard Henderson <rth@redhat.com> wrote:
> Still writing the changelog, but here's the version that's
> current in my tree as of this afternoon.  Notable changes
> from the last version are: documentation added, varrays
> removed, and some bugs fixed.
>

It caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41360

-- 
H.J.

CONVERT_EXPR cleanup

[Michael Matz]

Hi,

On Thu, 10 Sep 2009, Joseph S. Myers wrote:

> Finishing the existing incomplete transition from the separate NOP_EXPR 
> and CONVERT_EXPR tree codes to just having CONVERT_EXPR would be nice as 
> well.  Most places do now treat them the same via CASE_CONVERT etc.; I 
> don't know what we have left that might receive both codes and treats 
> them differently, that needs resolving before one of the codes can go 
> away.

I half-heartedly checked, and it's surprisingly not much.  What I did was 
this in tree.c:

@@ -3547,6 +3547,9 @@ build1_stat (enum tree_code code, tree t
 #endif
   tree t;

+  if (code == CONVERT_EXPR)
+    code = NOP_EXPR;

and looking for fallout.  So effectively never producing CONVERT_EXPR 
anymore.  It's one missing warning from the C++ frontend, for C only 
DFP related things that I can fix easily in tree.c:get_narrower, nothing 
for fortran.  I didn't check Ada, java or objc.

So, would you be open to changing all CONVERT_EXPR in the C frontend to 
NOP_EXPR (that is in convert.c, c-parser.c and c-typeck.c)?  Most build1's 
of the middle-end that create CONVERT_EXPRs look already bogus.

FWIW, the patch I lightly tested was the one below.


Ciao,
Michael.
-- 
Index: tree.c
===================================================================
--- tree.c	(revision 152233)
+++ tree.c	(working copy)
@@ -3547,6 +3547,9 @@ build1_stat (enum tree_code code, tree t
 #endif
   tree t;
 
+  if (code == CONVERT_EXPR)
+    code = NOP_EXPR;
+
 #ifdef GATHER_STATISTICS
   switch (TREE_CODE_CLASS (code))
     {
@@ -7561,6 +7564,23 @@ get_unwidened (tree op, tree for_type)
   return win;
 }
 \f
+/* Given a TYPE returns a number that represents sort of a kind of that
+   type, namely if it's integral, real, decimal real or fixed point.  */
+static int
+kind_of_type (tree type)
+{
+  if (INTEGRAL_TYPE_P (type))
+    return 1;
+  else if (DECIMAL_FLOAT_TYPE_P (type))
+    return 2;
+  else if (SCALAR_FLOAT_TYPE_P (type))
+    return 3;
+  else if (FIXED_POINT_TYPE_P (type))
+    return 4;
+  else
+    gcc_unreachable ();
+}
+
 /* Return OP or a simpler expression for a narrower value
    which can be sign-extended or zero-extended to give back OP.
    Store in *UNSIGNEDP_PTR either 1 if the value should be zero-extended
@@ -7572,7 +7592,7 @@ get_narrower (tree op, int *unsignedp_pt
   int uns = 0;
   int first = 1;
   tree win = op;
-  bool integral_p = INTEGRAL_TYPE_P (TREE_TYPE (op));
+  int typekind = kind_of_type (TREE_TYPE (op));
 
   while (TREE_CODE (op) == NOP_EXPR)
     {
@@ -7609,13 +7629,12 @@ get_narrower (tree op, int *unsignedp_pt
 	    uns = TYPE_UNSIGNED (TREE_TYPE (op));
 	  first = 0;
 	  op = TREE_OPERAND (op, 0);
-	  /* Keep trying to narrow, but don't assign op to win if it
-	     would turn an integral type into something else.  */
-	  if (INTEGRAL_TYPE_P (TREE_TYPE (op)) != integral_p)
-	    continue;
 	}
 
-      win = op;
+      /* If we see a change in type kind, don't set win, but
+	 continue trying to narrow.  */
+      if (kind_of_type (TREE_TYPE (op)) == typekind)
+	win = op;
     }
 
   if (TREE_CODE (op) == COMPONENT_REF

Re: CONVERT_EXPR cleanup

[Joseph S. Myers]

On Mon, 28 Sep 2009, Michael Matz wrote:

> So, would you be open to changing all CONVERT_EXPR in the C frontend to 
> NOP_EXPR (that is in convert.c, c-parser.c and c-typeck.c)?  Most build1's 
> of the middle-end that create CONVERT_EXPRs look already bogus.

I think the correct thing to be changing is places that can receive one or 
both of those codes, to treat them the same (with any consequent fixes 
needed), rather than arbitrarily changing places generating one code to 
generate the other.

(I'd prefer CONVERT_EXPR rather than NOP_EXPR to be the name that finally 
survives the unification.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: CONVERT_EXPR cleanup

[Michael Matz]

Hi,

On Mon, 28 Sep 2009, Joseph S. Myers wrote:

> > So, would you be open to changing all CONVERT_EXPR in the C frontend to 
> > NOP_EXPR (that is in convert.c, c-parser.c and c-typeck.c)?  Most build1's 
> > of the middle-end that create CONVERT_EXPRs look already bogus.
> 
> I think the correct thing to be changing is places that can receive one or 
> both of those codes, to treat them the same (with any consequent fixes 
> needed), rather than arbitrarily changing places generating one code to 
> generate the other.

Not generating the one in the first place (and consequentially fixing 
that) would lead to exactly the same end-result.  But yes, I can also 
change all of the left explicit tests to CONVERT_EXPR_P/CASE_CONVERT if 
that feels more assuring.

> (I'd prefer CONVERT_EXPR rather than NOP_EXPR to be the name that 
> finally survives the unification.)

Yes, I just did it this way, because there are much more occurences of 
NOP_EXPR than CONVERT_EXPR.  It just was a ball park check, remember :)


Ciao,
Michael.

Re: [rfc] exception handling in gimple

[H.J. Lu]

On Tue, Sep 15, 2009 at 7:43 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Sep 8, 2009 at 5:07 PM, Richard Henderson <rth@redhat.com> wrote:
>> Still writing the changelog, but here's the version that's
>> current in my tree as of this afternoon.  Notable changes
>> from the last version are: documentation added, varrays
>> removed, and some bugs fixed.
>>
>
> It caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41360
>

This also caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42450


-- 
H.J.

Re: [rfc] exception handling in gimple

[H.J. Lu]

On Mon, Dec 21, 2009 at 2:07 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Sep 15, 2009 at 7:43 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Sep 8, 2009 at 5:07 PM, Richard Henderson <rth@redhat.com> wrote:
>>> Still writing the changelog, but here's the version that's
>>> current in my tree as of this afternoon.  Notable changes
>>> from the last version are: documentation added, varrays
>>> removed, and some bugs fixed.
>>>
>>
>> It caused:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41360
>>
>
> This also caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42450
>
>

It also caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43365

-- 
H.J.