Re: [PATCH V1] Common infrastructure for load-store fusion for aarch64 and rs6000 target

[Richard Sandiford <richard.sandiford@arm.com>]

Ajit Agarwal <aagarwa1@linux.ibm.com> writes:
>>> diff --git a/gcc/df-problems.cc b/gcc/df-problems.cc
>>> index 88ee0dd67fc..a8d0ee7c4db 100644
>>> --- a/gcc/df-problems.cc
>>> +++ b/gcc/df-problems.cc
>>> @@ -3360,7 +3360,7 @@ df_set_unused_notes_for_mw (rtx_insn *insn, struct df_mw_hardreg *mws,
>>>    if (df_whole_mw_reg_unused_p (mws, live, artificial_uses))
>>>      {
>>>        unsigned int regno = mws->start_regno;
>>> -      df_set_note (REG_UNUSED, insn, mws->mw_reg);
>>> +      //df_set_note (REG_UNUSED, insn, mws->mw_reg);
>>>        dead_debug_insert_temp (debug, regno, insn, DEBUG_TEMP_AFTER_WITH_REG);
>>>  
>>>        if (REG_DEAD_DEBUGGING)
>>> @@ -3375,7 +3375,7 @@ df_set_unused_notes_for_mw (rtx_insn *insn, struct df_mw_hardreg *mws,
>>>  	if (!bitmap_bit_p (live, r)
>>>  	    && !bitmap_bit_p (artificial_uses, r))
>>>  	  {
>>> -	    df_set_note (REG_UNUSED, insn, regno_reg_rtx[r]);
>>> +	   // df_set_note (REG_UNUSED, insn, regno_reg_rtx[r]);
>>>  	    dead_debug_insert_temp (debug, r, insn, DEBUG_TEMP_AFTER_WITH_REG);
>>>  	    if (REG_DEAD_DEBUGGING)
>>>  	      df_print_note ("adding 2: ", insn, REG_NOTES (insn));
>>> @@ -3493,9 +3493,9 @@ df_create_unused_note (rtx_insn *insn, df_ref def,
>>>  	|| bitmap_bit_p (artificial_uses, dregno)
>>>  	|| df_ignore_stack_reg (dregno)))
>>>      {
>>> -      rtx reg = (DF_REF_LOC (def))
>>> -                ? *DF_REF_REAL_LOC (def): DF_REF_REG (def);
>>> -      df_set_note (REG_UNUSED, insn, reg);
>>> +      //rtx reg = (DF_REF_LOC (def))
>>> +      //          ? *DF_REF_REAL_LOC (def): DF_REF_REG (def);
>>> +      //df_set_note (REG_UNUSED, insn, reg);
>>>        dead_debug_insert_temp (debug, dregno, insn, DEBUG_TEMP_AFTER_WITH_REG);
>>>        if (REG_DEAD_DEBUGGING)
>>>  	df_print_note ("adding 3: ", insn, REG_NOTES (insn));
>> 
>> I don't think this can be right.  The last hunk of the var-tracking.cc
>> patch also seems to be reverting a correct change.
>> 
>
> We generate sequential registers using (subreg V16QI (reg 00mode) 16)
> and (reg OOmode 0)
> where OOmode is 256 bit and V16QI is 128 bits in order to generate
> sequential register pair.

OK.  As I mentioned in the other message I just sent, it seems pretty
standard to use a 256-bit mode to represent a pair of 128-bit values.
In that case:

- (reg:OO R) always refers to both registers in the pair, and any assignment
  to it modifies both registers in the pair

- (subreg:V16QI (reg:OO R) 0) refers to the first register only, and can
  be modified independently of the second register

- (subreg:V16QI (reg:OO R) 16) refers to the second register only, and can
  be modified independently of the first register

Is that how you're using it?

One thing to be wary of is that it isn't possible to assign to two
subregs of the same reg in a single instruction (at least AFAIK).
So any operation that wants to store to both registers in the pair
must store to (reg:OO R) itself, not to the two subregs.

> If I keep the above REG_UNUSED notes ira generates
> REG_UNUSED and in cprop_harreg pass and dce pass deletes store pairs and
> we get incorrect code.
>
> By commenting REG_UNUSED notes it is not generated and we get the correct store
> pair fusion and cprop_hardreg and dce doesn't deletes them.
>
> Please let me know is there are better ways to address this instead of commenting
> above generation of REG_UNUSED notes.

Could you quote an example rtl sequence that includes incorrect notes?
It might help to understand the problem a bit more.

Thanks,
Richard