Hello Richard:
On 19/06/24 3:26 pm, Richard Sandiford wrote:
> Ajit Agarwal <aagar...@linux.ibm.com> writes:
>> On 19/06/24 2:52 pm, Richard Sandiford wrote:
>>> Ajit Agarwal <aagar...@linux.ibm.com> writes:
>>>> On 19/06/24 2:40 pm, Richard Sandiford wrote:
>>>>> Ajit Agarwal <aagar...@linux.ibm.com> writes:
>>>>>> Hello Richard:
>>>>>>
>>>>>> On 19/06/24 1:54 pm, Richard Sandiford wrote:
>>>>>>> Ajit Agarwal <aagar...@linux.ibm.com> writes:
>>>>>>>>> What happens if you leave the assert alone? When does it fire? Is it
>>>>>>>>> still for uses in debug insns? If so, it's the fusion pass's
>>>>>>>>> responsibility
>>>>>>>>> to update those, as mentioned above. And it must update them before,
>>>>>>>>> or at the same time as, it deletes the definition.
>>>>>>>>>
>>>>>>>>
>>>>>>>> For debug insn I call reset_debug_use and now I dont see issues
>>>>>>>> with debug insn and issues I see with non debug insn where
>>>>>>>> def is there in old_defs and use has to be removed for the insn
>>>>>>>> that we modify load with OO UNSPEC to generate lxvp.
>>>>>>>
>>>>>>> Can you walk me through it step-by-step? If you leave the assert
>>>>>>> alone, when does it fire? What set of insn_changes are being made
>>>>>>> when the assert fires? (Calling debug on the changes will show this.)
>>>>>>> And what use does the assert fire on? (Again, calling debug on the use
>>>>>>> will show this.)
>>>>>>>
>>>>>>
>>>>>> (insn 660 735 739 50 (set (reg:OO 405 [ MEM[(_Float128 *)src_196] ])
>>>>>> (unspec:OO [
>>>>>> (mem:OO (reg/v/f:DI 197 [ base ]) [9 MEM[(_Float128
>>>>>> *)src_196]+0 S16 A128])
>>>>>> ] UNSPEC_LXVP)) 2188 {*movoo1}
>>>>>> (nil))
>>>>>>
>>>>>> This is definition.
>>>>>>
>>>>>> (insn 661 659 662 50 (set (reg:TF 179 [ result$imag ])
>>>>>> (plus:TF (reg:TF 179 [ result$imag ])
>>>>>> (subreg:TF (reg:OO 405 [ MEM[(_Float128 *)src_196] ]) 0)))
>>>>>> {addtf3}
>>>>>>
>>>>>> This is use.
>>>>>>
>>>>>> change has the above definition and the assert fires at the
>>>>>> above use.
>>>>>
>>>>> But can you call debug on the insn_change that contains the deleted def,
>>>>> and call debug on the access_info that triggers the assert?
>>>>>
>>>>
>>>> I am afraid I am not getting what exactly you meant here.
>>>
>>> One way would be to apply a patch like the below and quote the
>>> output from the last "Changes:" onward.
>>>
>>> Richard
>>>
>>
>> Thanks.
>>
>> This is the dump of use at assert point.
>>
>> use of superceded set r178:i131 (V2DI pseudo) by insn i133 in bb2 [ebb2] at
>> point 180
>> defined in bb2 [ebb2] at point 108
>>
>> This is the dump of change.
>>
>> deletion of insn i130 in bb2 [ebb2] at point 106:
>> deleted
>> uses:
>> use of set r219:i291 (DI pseudo)
>> appears inside an address
>> superceded use of set mem:i114
>> defines:
>> set r177:i131 (OO pseudo)
>> used by insn i132 in bb2 [ebb2] at point 178
>> change to insn i131 in bb2 [ebb2] at point 108:
>> +--------------------------------------
>> | 131: r177:OO=unspec[[r219:DI]] 101
>> +--------------------------------------
>> uses:
>> superceded use of set r219:i291 (DI pseudo)
>> appears inside an address
>> superceded use of set mem:i114
>> defines:
>> superceded set r178:i131 (V2DI pseudo)
>> used by insn i133 in bb2 [ebb2] at point 180
>> ~~~~~~~
>> new cost: 2147483647
>> new uses:
>> use of set r219:i291 (DI pseudo) by insn i131 in bb2 [ebb2] at point 108
>> defined in bb2 [ebb2] at point 62
>> appears inside an address
>> use of set mem:i114 by insn i131 in bb2 [ebb2] at point 108
>> defined in bb2 [ebb2] at point 104
>> new defs:
>> set r177:i131 (OO pseudo) in bb2 [ebb2] at point 108
>> used by insn i132 in bb2 [ebb2] at point 178
>> first insert-after candidate: insn i131 in bb2 [ebb2] at point 108
>> last insert-after candidate: insn i131 in bb2 [ebb2] at point 108
>
> Thanks. It looks like you're updating just the definitions,
> and then later updating the uses. That isn't the way that rtl-ssa
> is supposed to be used. Each change set -- in other words, each call
> to function_info::change_insns -- must go from a valid state to a valid
> state. That is, the RTL must be self-consistent before every individual
> call to function_info::change_insns and must be self-consistent after
> every individual call to function_info::change_insns.
>
> This is what I meant before about:
>
> ... if we're removing a definition, all uses in "real"
> debug and non-debug insns must be removed either earlier than the
> definition or at the same time as the definition. No such uses
> should remain.
>
> Since you want to update all uses of register 178, you need to include
> those updates in the same change set as the change to insns 130 and 131,
> rather than doing them later.
>
Thanks for the suggestions.
Addressed in v4 of the patch.
> Richard
Thanks & Regards
Ajit
