[PATCH] Don't emit CLOBBERs for eh registers

Ian Lance Taylor iant@google.com
Tue Sep 12 16:41:00 GMT 2006


Andreas Krebbel <Andreas.Krebbel@de.ibm.com> writes:

> +   /* The exception handling registers die at eh edges.  */
> +   for (i = 0; ; ++i)
> +     {
> +       unsigned regno = EH_RETURN_DATA_REGNO (i);
> +       if (regno == INVALID_REGNUM)
> + 	break;
> +       SET_REGNO_REG_SET (invalidated_by_call, regno);
> +     }
> + 

This needs to be wrapped in #ifdef EH_RETURN_DATA_REGNO.

Also, please rename invalidated_by_call to invalidated_by_eh_edge or
something like that.  Your change looks correct, but it looks odd
since the regset no longer holds just registers invalidated by a call.

> + #ifdef EH_RETURN_DATA_REGNO
> + 	{
> + 	  if (bb_has_pred_edge_with_flags_p (b, EDGE_EH))
> + 	    {
> + 	      unsigned int i;
> + 	      HARD_REG_SET invalidated_by_eh_edge;
> + 
> + 	      CLEAR_HARD_REG_SET (invalidated_by_eh_edge);
> + 
> + 	      /* The exception handling registers die at eh edges.  */
> + 	      for (i = 0; ; ++i)
> + 		{
> + 		  unsigned regno = EH_RETURN_DATA_REGNO (i);
> + 		  if (regno == INVALID_REGNUM)
> + 		    break;
> + 		  SET_HARD_REG_BIT (invalidated_by_eh_edge, regno);
> + 		}
> + 	      
> + 	      EXECUTE_IF_SET_IN_REG_SET (old, FIRST_PSEUDO_REGISTER, i, rsi)
> + 		{
> + 		  int a = reg_allocno[i];
> + 		  if (a >= 0)
> + 		    IOR_HARD_REG_SET (allocno[a].hard_reg_conflicts,
> + 				      invalidated_by_eh_edge);
> + 		}
> + 	    }
> + 	}
> + #endif

You don't need the outer layer of braces here.

More seriously, I think the correct code here would be simply:

#ifdef EH_RETURN_DATA_REGNO
  if (bb_has_pred_edge_with_flags (b, EDGE_EH))
    {
      unsigned int i;

      for (i = 0; ; ++i)
        {
          unsigned int regno = EH_RETURN_DATA_REGNO (i);
          if (regno == INVALID_REGNUM)
            break;
          record_one_conflict (regno);
        }
    }
#endif

I believe that will have the same effect as your code, while being
slightly less efficient.  But, more to the point, it is how the rest
of the code works and is more likely to stay correct over time.



> + /* Return true when one of the predecessor edges of BB is marked with FLAGS.  */
> + static inline bool bb_has_pred_edge_with_flags_p (struct basic_block_def *bb,
> + 						  int flags)
> + {
> +   edge e;
> +   edge_iterator ei;
> + 
> +   FOR_EACH_EDGE (e, ei, bb->preds)
> +     {
> +       if ((e->flags & flags) == flags)
> + 	return true;
> +     }
> +   return false;
> + }
> + 

I think I would prefer that this simply be
    bb_has_eh_pred (basic_block bb)
rather than passing in a flag.  I think it is unlikely that we will
need the full generality.  As a followup patch that function can
replace df_has_eh_preds.  (Use basic_block, not struct
basic_block_def *).

This patch is OK for mainline with those changes assuming it still
passes the bootstrap and testsuite on at least i686 and s390.

Thanks very much for pursuing this.

Ian



More information about the Gcc-patches mailing list