Re: Problem with auto-inc code (ARM)

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

  In message <199911081445.OAA06583@cam-mail1.cambridge.arm.com>you write:
 > Before flow we have the insns:
  > 
  > (insn/i 5853 5851 5855 (set (mem:QI (reg:SI 1185) 0)
  >         (subreg:QI (reg/v:SI 1178) 0)) 214 {*movqi_insn} (nil)
  >     (nil))
  > 
  > (insn/i 5855 5853 5856 (set (reg:SI 1185)
  >         (plus:SI (reg:SI 1185)
  >             (const_int 1 [0x1]))) 8 {*addsi3_insn} (nil)
  >     (nil))
  > 
  > (insn/i 5856 5855 5857 (set (mem/s:SI (reg/v:SI 52) 10)
  >         (reg:SI 1185)) 189 {*movsi_insn} (nil)
  >     (expr_list:REG_DEAD (reg:SI 1185)
  >         (nil)))
  > 
  > find_auto_inc detects that insn 5855 is a valid increment to be combined 
  > with insn 5853 to create
  > 
  > (insn/i 5853 5851 5855 (set (mem:QI (post_inc:SI (reg:SI 1185)) 0)
  >         (subreg:QI (reg/v:SI 1178) 0)) 214 {*movqi_insn} (nil)
  >     (nil))
  > 
  > (note 5855 5853 5856 "" NOTE_INSN_DELETED)
  > 
  > (insn/i 5856 5855 5857 (set (mem/s:SI (reg/v:SI 52) 10)
  >         (reg:SI 1185)) 189 {*movsi_insn} (insn_list 5855 (nil))
  >     (expr_list:REG_DEAD (reg:SI 1185)
  >         (nil)))
  > 
  > Unfortunately, this leaves the log_links of insn 5856 pointing at the 
  > deleted insn, which trips the checking code in combine when we try to get 
  > the links from the deleted insn as part of a three-way combine.
  > 
  > I guess the correct thing to do is to update the links correctly, but I'm 
  > not particularly familiar with the data structures used in flow.  Is there 
  > a simple way to find insn 5856 when we delete the increment?
Not that I'm aware of.  In combine the dataflow is always backwards (ie the
user points back to the setter).

You probably could scan forward from the increment up to the first insn with
a dependency on the increment or until you hit the end of the current block.


  > If this isn't feasible I can see two other approaches:
  > 
  > 1) we could modify combine_instructions to check that an insn hasn't been 
  > deleted before we try to dereference its links.  I don't like this, since 
  > it's really papering over a crack.
True.  But this is more bullet proof since it isolates combine from code
which has left turds in the dependency chain.  At one time I believe the
scheduler used to do something similar -- but I believe that code became
dead once sched was changed to completely rebuild the dependency chain.


  > 2) We could turn the insn that we have deleted into a no-op move so that 
  > it is never deleted (this is essentially what would happen if the 
  > increment insn had written its result to a different register with the 
  > source register dying in the insn); but there is a comment in there that 
  > says that it should be deleted so that it doesn't appear as a "use and a 
  > set" of the reg.
This would be feasible from a correctness standpoint.   But I'd prefer fixing
the auto-inc code or #1 if we can make those work.

jeff