Re: [PATCH v2 1/3] Move prepare_decl_rtl to expr.[ch] as extern

["Kewen.Lin" <linkw at linux dot ibm dot com>]

on 2019/5/15 上午10:11, Kewen.Lin wrote:
> 
> on 2019/5/14 下午3:18, Richard Biener wrote:
>> Hum.  The function is somewhat of a hack, trying to produce
>> "reasonable" DECL_RTL, exposing it in expr.[ch] with this
>> name is eventually misleading.  Also you fail to "outline"
>> the most important part:
>>
>>   FOR_EACH_VEC_ELT (decl_rtl_to_reset, i, obj)
>>     SET_DECL_RTL (obj, NULL_RTX);
>>
>> which IMHO would warrant making this machinery a class
>> with the above done in its destructor?
>>
> 
> Good suggestion!  In the IVOPTS implementation, it has one 
> interface free_loop_data to clean up some data structures
> including this decl_rtl_to_reset.  While for the use in 
> "PATCH v2 2/3", we have to clean it artificially once 
> expanding finishes.
> 
> It's better to make it as a class and ensure the clean of
> the vector in its destructor.
> 

Hi Bin,

Could you kindly comment this?  For the use in "PATCH v2 2/3",
it's still the same with destructor change.  But for the use 
in IVOPTs, I don't know the historical reason why to clean
up in free_loop_data instead of in place?  If it's possible to
reuse, for example some object was prepared and we don't need
to prepare for it again during the same loop handling, then
the destructor proposal will clean it up too early and 
inefficient.  If so, one flag in constructor to control this 
might be required.

Thanks a lot in advance!

Kewen


>> Maybe name the functions prepare_guessed_decl_rtl ()
>> and the new class guessed_decl_rtl?
>>
> OK.  Or "tmp" instead of "guessed"?
> 
> 
> Thanks,
> Kewen
> 
>> Now looking how you'll end up using this...
>>
>> Richard.
>>
>