on 2019/5/15 下午4:50, Bin.Cheng wrote: > On Wed, May 15, 2019 at 4:38 PM Kewen.Lin <li...@linux.ibm.com> wrote: >> >> 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 > This is quite old code in ivopts, I suppose it's for caching results > as you guessed. I didn't measure if how much the caching affects > performance. It can be simply removed if not as useful as expected. > Of course we need experiment data here.
Got it, thanks! > > Please correct me if I am wrong. This is factored out of ivopts in > order to reuse it in the new target hook? As discussed in another > thread, most target hook code can be moved to ivopts, so this > refactoring and in-place freeing would be unnecessary, and the new > code can also take advantage of caching? > Yes, you are absolutely right. If we move the generic part ( especially costly_iter_for_doloop_p) to IVOPTs, the factoring isn't necessary any more. I thought it may be still nice to refactor it even if we will go with that moving of generic part. But the code exists for a long time but there is no requests to factor it out all the time, it looks useless to expose it speculatively? OK, I'll cancel factoring it then. Thanks, Kewen > Thanks, > bin >> 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 >> >>
