Kenneth Zadeck wrote on 08/28/06 09:57:

> I have not done this because I do not rule the earth.  That was not
> what I was assigned to do, and I agreed that DWARF3 sounded like a
> reasonable way to go.  Now that I understand the details of DWARF3, I
> have changed my mind about the correct direction.  Now is the time to
> make that change before there is a lot of infrastructure built that
> assumes the DWARF3 encoding.
> 
FWIW.  I agree with your conclusion.  I do not know DWARF3 very well,
but it always felt a bit heavy handed to me.

At first, I was under the impression that we were going to use DWARF3 to
describe the types and symbol table, and embed our bytecode alongside.
It sounds to me like we want to either invent our own bytecode or adapt
an existing one to our needs.  Inventing our own is probably the best
long-term alternative.

A few comments on the code:

> Index: lto-tree-flags.def
> [ ... ]
> +/* This file is designed to be inlined into both the writing and the
> +   reading of the lto information.  What it does depends on the glue
> +   that put in front and at the end of this and how ADD_BASE_FLAG is
> +   defined. 
> +
> +   For those interested in extra credit, this could also be used as
> +   the checking code to see if each flag is used correctly.  10 extra
> +   credit points will be given for the intrepid programmer who does
> +   this and is able to removes the comment that this was generated
> +   from in tree.h.  */
>
Nothing in this comment helps in understanding what the file is supposed
to do.  What is this used for?

> +  switch (code)
> +    {
> +    case ABS_EXPR:
> +      break;
> +      
> +    case ADDR_EXPR:
> +      break;
> +      
> +      ADD_BASE_FLAG (static_flag)
>
This code is unreachable.  There are many instances of this here.


> +/* Return the master clone node of N if it is available and if
> +   CHECK_OVERWRITE is true, not overwritable.  */ 
> +
What does it mean to be overwritable?  You never seem to call this
function with check_overwrite == false.


> +struct char_ptr_base
> +{
> +  char *ptr;
> +};
> +
Hmm?  Why?  Planning to have something different in the future?


> +/* An incore byte stream to buffer the various parts of the
> +function. The entire structure should be zeroed when created.  The
> +record consists of a set of blocks.  The first sizeof (ptr) bytes are
> +used as a chain, and the rest store the bytes to be written.  */
> +
> +struct output_stream
> +{
> +  /* The pointer to the first block in the stream.  */
> +  struct char_ptr_base * first_block;
> +  /* The pointer to the last and current block in the stream.  */
> +  struct char_ptr_base * current_block;
> +  /* The pointer to where the next char should be written.  */
> +  char * current_pointer;
> +  /* The number of characters left in the current block.  */
> +  unsigned int left_in_block;
> +  /* The block size of the last block allocated.  */
> +  unsigned int block_size;
> +  /* The total number of characters written.  */
> +  unsigned int total_size;
> +};
> +
Maybe there's code out there for paging/caching data streams?  Though we
would probably want to tweak it quite a bit, it may save some effort for
the base functionality.

Even if we end up not using DWARF3 as output, the streaming aspect of
this code will remain, I guess?

> +#if STUPID_TYPE_SYSTEM
>
STUPID_TYPE_SYSTEM?  No need to be insulting.  It's unpleasant.

> +/* Find, or generate, if there is not one already, the abbrev table
> +   entries for the various stmt_table forms.  The forms in the case
> +   statement here must EXACTLY MATCH the forms in the case statement
> +   in output_stmt_operands.  */
> +
> +static int
> +get_stmt_form (unsigned int tag)
> +{
> +  int index = tag - DW_TAG_gimple_stmt_table_first;
> +  int entry = stmt_form_abbrev_table[index];
> +
> +  if (entry)
> +    return entry;
> +
> +  switch (tag)
> +    {
> +    case DW_TAG_gimple_asm_expr:
>
Hmm.  Interesting.  We can use this as a verification tool, as well.
>From here we could synthesize an up-to-date GIMPLE grammar and keep it
self-consistent.  Nice.

As we find problems with the writer/reader, we should update either the
grammar or GIMPLE (or both).

> @@ -168,12 +167,6 @@ struct eh_region GTY(())
>        int filter;
>      } GTY ((tag ("ERT_ALLOWED_EXCEPTIONS"))) allowed;
>  
> -    /* The type given by a call to "throw foo();", or discovered
> -       for a throw.  */
> -    struct eh_region_u_throw {
> -      tree type;
> -    } GTY ((tag ("ERT_THROW"))) throw;
> -
>      /* Retain the cleanup expression even after expansion so that
>         we can match up fixup regions.  */
>      struct eh_region_u_cleanup {
> @@ -706,13 +699,6 @@ remove_unreachable_regions (rtx insns)
>         bool kill_it = true;
>         switch (r->type)
>           {
> -         case ERT_THROW:
> -           /* Don't remove ERT_THROW regions if their outer region
> -              is reachable.  */
> -           if (r->outer && reachable[r->outer->region_number])
> -             kill_it = false;
> -           break;
> -
>           case ERT_MUST_NOT_THROW:
>             /* MUST_NOT_THROW regions are implementable solely in the
>                runtime, but their existence continues to affect calls
> @@ -849,8 +835,7 @@ current_function_has_exception_handlers 
>  
>        region = VEC_index (eh_region, cfun->eh->region_array, i);
>        if (region
> -       && region->region_number == i
> -       && region->type != ERT_THROW)
> +       && region->region_number == i)
>       return true;
>      }
>  
> @@ -1506,7 +1491,6 @@ build_post_landing_pads (void)
>         break;
>  
>       case ERT_CATCH:
> -     case ERT_THROW:
>         /* Nothing to do.  */
>         break;
>  
What are these changes for?

Reply via email to