[avr-gcc-list] Re: bugfix for cselib_invalidate_regno

[Denis Chertykov]

Richard Earnshaw <address@hidden> writes:

> > Richard Earnshaw <address@hidden> writes:
> > 
> > > > Denis Chertykov <address@hidden> writes:
> > > > 
> > > > > I have founded a bug in cselib.c:cselib_invalidate_regno.
> > > > > The avr port triggers this bug.
> > > > > Current version of cselib_invalidate_regno didn't invalidate lower
> > > > > register numbers if they contain values that overlap REGNO in case
> > > > > that (regno < FIRST_PSEUDO_REGISTER && mode == VOIDmode). This happens
> > > > > if cselib_invalidate_regno called from cselib_process_insn which
> > > > > handle CALL_INSN.
> > > > > IE
> > > > > 
> > > > >   /* If this is a call instruction, forget anything stored in a
> > > > >      call clobbered register, or, if this is not a const call, in
> > > > >      memory.  */
> > > > >   if (GET_CODE (insn) == CALL_INSN)
> > > > >     {
> > > > >       for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
> > > > >       if (call_used_regs[i])
> > > > >         cselib_invalidate_regno (i, VOIDmode);
> > > > > 
> 
> > > 
> > > Why not change the caller to pass reg_raw_mode[regno].  That seems much 
> > > cleaner to me than pretending a hard register can have no mode.
> > 
> > Because my patch make cselib_invalidate_regno more universal.
> > Your example will work with my patch too.
> > 
> 
> No it doesn't.  At least, not unless we agree that we want this meaning to 
> exist everywhere.  There's nothing in the documentation that says that a 
> hard register in mode VOIDmode has its natural size.  So all you are doing 
> here is adding an undocumented local convention.  I dislike that strongly.
> 
> > cselib_invalidate_regno have only two callers, are you sure that the
> > second always pass the mode != VOIDmode ?
> 
> IMO cselib_invalidate_regno should abort if passed VOIDmode for a hard 
> register.  As things stand at present it's meaningless.

The patch:

2003-05-27  Denis Chertykov  <address@hidden>

        * cselib.c (cselib_invalidate_regno): Abort if hardreg have a
        VOIDmode.
        * cselib.c (cselib_process_insn): Pass reg_raw_mode for hardreg in
        call of cselib_invalidate_regno.


Index: cselib.c
===================================================================
RCS file: /cvsroot/gcc/egcs/gcc/cselib.c,v
retrieving revision 1.27
diff -c -3 -p -r1.27 cselib.c
*** cselib.c    17 Apr 2003 01:07:12 -0000      1.27
--- cselib.c    27 May 2003 11:30:12 -0000
*************** cselib_invalidate_regno (regno, mode)
*** 1019,1026 ****
       pseudos, only REGNO is affected.  For hard regs, we must take MODE
       into account, and we must also invalidate lower register numbers
       if they contain values that overlap REGNO.  */
!   if (regno < FIRST_PSEUDO_REGISTER && mode != VOIDmode) 
      {
        if (regno < max_value_regs)
        i = 0;
        else
--- 1019,1029 ----
       pseudos, only REGNO is affected.  For hard regs, we must take MODE
       into account, and we must also invalidate lower register numbers
       if they contain values that overlap REGNO.  */
!   if (regno < FIRST_PSEUDO_REGISTER)
      {
+       if (mode == VOIDmode)
+       abort ();
+       
        if (regno < max_value_regs)
        i = 0;
        else
*************** cselib_process_insn (insn)
*** 1427,1433 ****
      {
        for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
        if (call_used_regs[i])
!         cselib_invalidate_regno (i, VOIDmode);
  
        if (! CONST_OR_PURE_CALL_P (insn))
        cselib_invalidate_mem (callmem);
--- 1430,1436 ----
      {
        for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
        if (call_used_regs[i])
!         cselib_invalidate_regno (i, reg_raw_mode[i]);
  
        if (! CONST_OR_PURE_CALL_P (insn))
        cselib_invalidate_mem (callmem);