Re: patch for gcse bug

[Jeffrey A Law <law at cygnus dot com>]

  In message <19981008000336.C21728@dot.cygnus.com>you write:
  > Ok, so people have probably seen that the patch checked in this
  > morning exposed some latent problems.
  > 
  > Below is my proposed solution. 
  > 
  > First, some modifications to flow so as to make it easy to detect
  > abnormal flow control out of a call insn.  Iff the call ends a basic
  > block due to abnormal flow, it will be the insn at BLOCK_END.  To
  > preserve this, I add nop rtl when there is no such abnormal flow,
  > but a call just happens to end a basic block.
  > 
  > Second, and much more important, is the modification to the PRE
  > global property PPout.  Using a new local property I dub TRANSPout,
  > I remove expressions from PPout that could be killed by such a 
  > call, that is, most mems since we do no interprocedual alias analysis.
  > 
  > 
  > r~
  > 
  > 
  > 	* flow.c (find_basic_blocks): Calc upper bound for extra nops in
  > 	max_uids_for_flow.
  > 	(find_basic_blocks_1): Add a nop to the end of a basic block when
  > 	a trailing call insn does not have abnormal control flow.
  > 	* gcse.c (pre_transpout): New variable.
  > 	(alloc_pre_mem, free_pre_mem, dump_pre_data): Bookkeeping for it.
  > 	(compute_pre_transpout): Calculate it.
  > 	(compute_pre_ppinout): Use it to eliminate impossible placements
  > 	due to abnormal control flow through calls.
  > 	(compute_pre_data): Call compute_pre_transpout.
Just a couple nits.

  > + 		    /* Else this call does not end a basic block.  In
  > + 		       order to disambiguate this call from one that
  > + 		       does end a block, we might need to add a nop
  > + 		       insn following.  We'll find this out in 
  > + 		       find_basic_blocks_1, but we need to account
  > + 		       for the possibility in max_uid_for_flow.  */
  > + 		    extra_uids_for_flow++;
I found the comment just a little confusing.

You might instead say something like

Else this call does not cause a new block to be created.  However, it may
still be the end of a basic block (if it is followed by a JUMP or CODE_LABEL).

To disambiguate calls which cause new blocks to be created and those which
just happen to be at the end of a block we insert NOTEs during
find_basic_blocks_1 after calls which are the last insn in a block by chance.
We must account for such insns in max_uid_for_flow.


Or something along those lines.  Or maybe you differentiate them as calls
which create abnormal edges vs those which do not.

  > ! 	  /* If the previous insn was a call that was not itself an edge,
  > ! 	     we want to add a nop so that the call insn itself is not at
  > ! 	     basic_block_end.  This allows us to simply distinguish the
  > ! 	     two cases elsewhere.  */
I think I'd use "a call that did not create an abnormal edge, we want to add
a nop so that the CALL_INSN itself is not at basic_block_end.  This allows us
to easily distinguish between normal calls and those which create abnormal
edges in the flow graph."  Or something similar.


  > + /* Compute transparent outgoing information for each block.
  > + 
  > +    An expression is transparent to an edge unless it is killed by
  > +    the edge itself.  This can only happen when the edge is traversed
  > +    through a call, as may happen with non-local labels and exceptions.  *  > /
You might make a note that this should be unnecessary if we split the abnormal
critical edge.  These problems should only arise for abnormal critical edges.


I don't see any changes that need to be made to the code itself.

I think we should do the same thing for EARLout in the LCM implementation.

jeff