* [rfc] exception handling in gimple
@ 2009-09-03 1:23 Richard Henderson
2009-09-08 14:34 ` Michael Matz
2009-09-08 15:16 ` Michael Matz
0 siblings, 2 replies; 31+ messages in thread
From: Richard Henderson @ 2009-09-03 1:23 UTC (permalink / raw)
To: GCC Patches
[-- 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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-03 1:23 [rfc] exception handling in gimple Richard Henderson
@ 2009-09-08 14:34 ` Michael Matz
2009-09-08 16:25 ` Richard Henderson
2009-09-08 15:16 ` Michael Matz
1 sibling, 1 reply; 31+ messages in thread
From: Michael Matz @ 2009-09-08 14:34 UTC (permalink / raw)
To: Richard Henderson; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-03 1:23 [rfc] exception handling in gimple Richard Henderson
2009-09-08 14:34 ` Michael Matz
@ 2009-09-08 15:16 ` Michael Matz
2009-09-08 16:26 ` Richard Henderson
1 sibling, 1 reply; 31+ messages in thread
From: Michael Matz @ 2009-09-08 15:16 UTC (permalink / raw)
To: Richard Henderson; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-08 14:34 ` Michael Matz
@ 2009-09-08 16:25 ` Richard Henderson
2009-09-09 0:07 ` Richard Henderson
0 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2009-09-08 16:25 UTC (permalink / raw)
To: Michael Matz; +Cc: GCC Patches
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~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-08 15:16 ` Michael Matz
@ 2009-09-08 16:26 ` Richard Henderson
2009-09-08 16:42 ` Richard Henderson
0 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2009-09-08 16:26 UTC (permalink / raw)
To: Michael Matz; +Cc: GCC Patches
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~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-08 16:26 ` Richard Henderson
@ 2009-09-08 16:42 ` Richard Henderson
2009-09-09 14:38 ` Michael Matz
0 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2009-09-08 16:42 UTC (permalink / raw)
To: Michael Matz; +Cc: GCC Patches
[-- 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);
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-08 16:25 ` Richard Henderson
@ 2009-09-09 0:07 ` Richard Henderson
2009-09-09 8:07 ` Olivier Hainque
` (5 more replies)
0 siblings, 6 replies; 31+ messages in thread
From: Richard Henderson @ 2009-09-09 0:07 UTC (permalink / raw)
To: Michael Matz; +Cc: GCC Patches
[-- 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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-09 0:07 ` Richard Henderson
@ 2009-09-09 8:07 ` Olivier Hainque
2009-09-09 10:36 ` Michael Matz
2009-09-09 8:43 ` Eric Botcazou
` (4 subsequent siblings)
5 siblings, 1 reply; 31+ messages in thread
From: Olivier Hainque @ 2009-09-09 8:07 UTC (permalink / raw)
To: Richard Henderson; +Cc: GCC Patches, 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 |
+=====================================+
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-09 0:07 ` Richard Henderson
2009-09-09 8:07 ` Olivier Hainque
@ 2009-09-09 8:43 ` Eric Botcazou
2009-09-09 15:34 ` Richard Henderson
2009-09-09 14:34 ` Michael Matz
` (3 subsequent siblings)
5 siblings, 1 reply; 31+ messages in thread
From: Eric Botcazou @ 2009-09-09 8:43 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc-patches, Michael Matz
> 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-09 8:07 ` Olivier Hainque
@ 2009-09-09 10:36 ` Michael Matz
0 siblings, 0 replies; 31+ messages in thread
From: Michael Matz @ 2009-09-09 10:36 UTC (permalink / raw)
To: Olivier Hainque; +Cc: Richard Henderson, GCC Patches
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-09 0:07 ` Richard Henderson
2009-09-09 8:07 ` Olivier Hainque
2009-09-09 8:43 ` Eric Botcazou
@ 2009-09-09 14:34 ` Michael Matz
2009-09-09 15:22 ` Richard Henderson
2009-09-10 0:15 ` Richard Henderson
2009-09-10 2:28 ` Richard Henderson
` (2 subsequent siblings)
5 siblings, 2 replies; 31+ messages in thread
From: Michael Matz @ 2009-09-09 14:34 UTC (permalink / raw)
To: Richard Henderson; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-08 16:42 ` Richard Henderson
@ 2009-09-09 14:38 ` Michael Matz
0 siblings, 0 replies; 31+ messages in thread
From: Michael Matz @ 2009-09-09 14:38 UTC (permalink / raw)
To: Richard Henderson; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-09 14:34 ` Michael Matz
@ 2009-09-09 15:22 ` Richard Henderson
2009-09-10 0:15 ` Richard Henderson
1 sibling, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2009-09-09 15:22 UTC (permalink / raw)
To: Michael Matz; +Cc: GCC Patches
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~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-09 8:43 ` Eric Botcazou
@ 2009-09-09 15:34 ` Richard Henderson
0 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2009-09-09 15:34 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Michael Matz
On 09/09/2009 01:39 AM, Eric Botcazou wrote:
> Nice documentation. Some proofreading:
All fixed, thanks.
r~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-09 14:34 ` Michael Matz
2009-09-09 15:22 ` Richard Henderson
@ 2009-09-10 0:15 ` Richard Henderson
2009-09-10 10:49 ` Michael Matz
1 sibling, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2009-09-10 0:15 UTC (permalink / raw)
To: Michael Matz; +Cc: GCC Patches
[-- 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);
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-09 0:07 ` Richard Henderson
` (2 preceding siblings ...)
2009-09-09 14:34 ` Michael Matz
@ 2009-09-10 2:28 ` Richard Henderson
2009-09-10 2:32 ` Richard Henderson
2009-09-15 15:43 ` H.J. Lu
5 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2009-09-10 2:28 UTC (permalink / raw)
To: Michael Matz; +Cc: GCC Patches
[-- 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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-09 0:07 ` Richard Henderson
` (3 preceding siblings ...)
2009-09-10 2:28 ` Richard Henderson
@ 2009-09-10 2:32 ` Richard Henderson
2009-09-10 10:55 ` Michael Matz
2009-09-15 15:43 ` H.J. Lu
5 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2009-09-10 2:32 UTC (permalink / raw)
To: Michael Matz; +Cc: GCC Patches
[-- 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;
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-10 0:15 ` Richard Henderson
@ 2009-09-10 10:49 ` Michael Matz
2009-09-10 11:41 ` Joseph S. Myers
` (2 more replies)
0 siblings, 3 replies; 31+ messages in thread
From: Michael Matz @ 2009-09-10 10:49 UTC (permalink / raw)
To: Richard Henderson; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-10 2:32 ` Richard Henderson
@ 2009-09-10 10:55 ` Michael Matz
2009-09-10 16:28 ` Richard Henderson
0 siblings, 1 reply; 31+ messages in thread
From: Michael Matz @ 2009-09-10 10:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-10 10:49 ` Michael Matz
@ 2009-09-10 11:41 ` Joseph S. Myers
2009-09-28 15:00 ` CONVERT_EXPR cleanup Michael Matz
2009-09-10 16:25 ` [rfc] exception handling in gimple Richard Henderson
2009-09-10 16:46 ` Richard Henderson
2 siblings, 1 reply; 31+ messages in thread
From: Joseph S. Myers @ 2009-09-10 11:41 UTC (permalink / raw)
To: Michael Matz; +Cc: Richard Henderson, GCC Patches
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-10 10:49 ` Michael Matz
2009-09-10 11:41 ` Joseph S. Myers
@ 2009-09-10 16:25 ` Richard Henderson
2009-09-10 16:46 ` Richard Henderson
2 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2009-09-10 16:25 UTC (permalink / raw)
To: Michael Matz; +Cc: GCC Patches
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~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-10 10:55 ` Michael Matz
@ 2009-09-10 16:28 ` Richard Henderson
2009-09-11 10:42 ` Michael Matz
0 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2009-09-10 16:28 UTC (permalink / raw)
To: Michael Matz; +Cc: GCC Patches
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~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-10 10:49 ` Michael Matz
2009-09-10 11:41 ` Joseph S. Myers
2009-09-10 16:25 ` [rfc] exception handling in gimple Richard Henderson
@ 2009-09-10 16:46 ` Richard Henderson
2 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2009-09-10 16:46 UTC (permalink / raw)
To: Michael Matz; +Cc: GCC Patches
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~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-10 16:28 ` Richard Henderson
@ 2009-09-11 10:42 ` Michael Matz
2009-09-11 15:23 ` Richard Henderson
0 siblings, 1 reply; 31+ messages in thread
From: Michael Matz @ 2009-09-11 10:42 UTC (permalink / raw)
To: Richard Henderson; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-11 10:42 ` Michael Matz
@ 2009-09-11 15:23 ` Richard Henderson
0 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2009-09-11 15:23 UTC (permalink / raw)
To: Michael Matz; +Cc: GCC Patches
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~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-09 0:07 ` Richard Henderson
` (4 preceding siblings ...)
2009-09-10 2:32 ` Richard Henderson
@ 2009-09-15 15:43 ` H.J. Lu
2009-12-22 1:54 ` H.J. Lu
5 siblings, 1 reply; 31+ messages in thread
From: H.J. Lu @ 2009-09-15 15:43 UTC (permalink / raw)
To: Richard Henderson; +Cc: Michael Matz, GCC Patches
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* CONVERT_EXPR cleanup
2009-09-10 11:41 ` Joseph S. Myers
@ 2009-09-28 15:00 ` Michael Matz
2009-09-28 15:25 ` Joseph S. Myers
0 siblings, 1 reply; 31+ messages in thread
From: Michael Matz @ 2009-09-28 15:00 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: CONVERT_EXPR cleanup
2009-09-28 15:00 ` CONVERT_EXPR cleanup Michael Matz
@ 2009-09-28 15:25 ` Joseph S. Myers
2009-09-28 15:44 ` Michael Matz
0 siblings, 1 reply; 31+ messages in thread
From: Joseph S. Myers @ 2009-09-28 15:25 UTC (permalink / raw)
To: Michael Matz; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: CONVERT_EXPR cleanup
2009-09-28 15:25 ` Joseph S. Myers
@ 2009-09-28 15:44 ` Michael Matz
0 siblings, 0 replies; 31+ messages in thread
From: Michael Matz @ 2009-09-28 15:44 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-09-15 15:43 ` H.J. Lu
@ 2009-12-22 1:54 ` H.J. Lu
2010-03-14 17:58 ` H.J. Lu
0 siblings, 1 reply; 31+ messages in thread
From: H.J. Lu @ 2009-12-22 1:54 UTC (permalink / raw)
To: Richard Henderson; +Cc: Michael Matz, GCC Patches
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [rfc] exception handling in gimple
2009-12-22 1:54 ` H.J. Lu
@ 2010-03-14 17:58 ` H.J. Lu
0 siblings, 0 replies; 31+ messages in thread
From: H.J. Lu @ 2010-03-14 17:58 UTC (permalink / raw)
To: Richard Henderson; +Cc: Michael Matz, GCC Patches
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2010-03-14 17:30 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-03 1:23 [rfc] exception handling in gimple Richard Henderson
2009-09-08 14:34 ` Michael Matz
2009-09-08 16:25 ` Richard Henderson
2009-09-09 0:07 ` Richard Henderson
2009-09-09 8:07 ` Olivier Hainque
2009-09-09 10:36 ` Michael Matz
2009-09-09 8:43 ` Eric Botcazou
2009-09-09 15:34 ` Richard Henderson
2009-09-09 14:34 ` Michael Matz
2009-09-09 15:22 ` Richard Henderson
2009-09-10 0:15 ` Richard Henderson
2009-09-10 10:49 ` Michael Matz
2009-09-10 11:41 ` Joseph S. Myers
2009-09-28 15:00 ` CONVERT_EXPR cleanup Michael Matz
2009-09-28 15:25 ` Joseph S. Myers
2009-09-28 15:44 ` Michael Matz
2009-09-10 16:25 ` [rfc] exception handling in gimple Richard Henderson
2009-09-10 16:46 ` Richard Henderson
2009-09-10 2:28 ` Richard Henderson
2009-09-10 2:32 ` Richard Henderson
2009-09-10 10:55 ` Michael Matz
2009-09-10 16:28 ` Richard Henderson
2009-09-11 10:42 ` Michael Matz
2009-09-11 15:23 ` Richard Henderson
2009-09-15 15:43 ` H.J. Lu
2009-12-22 1:54 ` H.J. Lu
2010-03-14 17:58 ` H.J. Lu
2009-09-08 15:16 ` Michael Matz
2009-09-08 16:26 ` Richard Henderson
2009-09-08 16:42 ` Richard Henderson
2009-09-09 14:38 ` Michael Matz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).