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.
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? 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 > > > >> 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. > >> > > >
