Michael Kruse via llvm-dev
2018-Dec-12 17:35 UTC
[llvm-dev] RFC: LoopIDs are not identifiers (and better loop-parallel metadata)
Am Mi., 12. Dez. 2018 um 11:11 Uhr schrieb Finkel, Hal J. <hfinkel at anl.gov>:> > On 12/12/18 10:29 AM, Michael Kruse wrote: > > Am Mi., 12. Dez. 2018 um 10:10 Uhr schrieb Finkel, Hal J. <hfinkel at anl.gov>: > >>> As we have seen, > >>> there are other reasons for loops to have identical LoopIDs. With > >>> patches [3,4], llvm.loop metadata can be collapsed (unlike access > >>> groups), thus the 'distinct' is not necessary anymore. Unfortunately, > >>> there is code in LLVM (and maybe elsewhere) that depends on LoopIDs' > >>> first item, i.e. we cannot get rid of it that easily. > >> I don't think it's worth changing this first element, unless we have > >> some other reason to do so. > > Would it be worthwhile to update the metadata uniquing algorithm to > > consider shallow self-references? > > What benefit would that bring?Fewer metadata nodes by uniquing them. I would expect there might be quite a few loops that have identical metadata, e.g. all loops that have "setAlreaduUnrolled()" after loop unrolling. Michael
Finkel, Hal J. via llvm-dev
2018-Dec-12 17:44 UTC
[llvm-dev] RFC: LoopIDs are not identifiers (and better loop-parallel metadata)
On 12/12/18 11:35 AM, Michael Kruse wrote:> Am Mi., 12. Dez. 2018 um 11:11 Uhr schrieb Finkel, Hal J. <hfinkel at anl.gov>: >> On 12/12/18 10:29 AM, Michael Kruse wrote: >>> Am Mi., 12. Dez. 2018 um 10:10 Uhr schrieb Finkel, Hal J. <hfinkel at anl.gov>: >>>>> As we have seen, >>>>> there are other reasons for loops to have identical LoopIDs. With >>>>> patches [3,4], llvm.loop metadata can be collapsed (unlike access >>>>> groups), thus the 'distinct' is not necessary anymore. Unfortunately, >>>>> there is code in LLVM (and maybe elsewhere) that depends on LoopIDs' >>>>> first item, i.e. we cannot get rid of it that easily. >>>> I don't think it's worth changing this first element, unless we have >>>> some other reason to do so. >>> Would it be worthwhile to update the metadata uniquing algorithm to >>> consider shallow self-references? >> What benefit would that bring? > Fewer metadata nodes by uniquing them. I would expect there might be > quite a few loops that have identical metadata, e.g. all loops that > have "setAlreaduUnrolled()" after loop unrolling.Okay. I have no idea how much code this would break (from none to a lot). Any thoughts? I think that, in general, you should have a separate RFC if you'd like to change them metadata uniquing algorithm. Thanks again, Hal> > Michael-- Hal Finkel Lead, Compiler Technology and Programming Languages Leadership Computing Facility Argonne National Laboratory
Michael Kruse via llvm-dev
2018-Dec-12 18:25 UTC
[llvm-dev] RFC: LoopIDs are not identifiers (and better loop-parallel metadata)
Am Mi., 12. Dez. 2018 um 11:44 Uhr schrieb Finkel, Hal J. <hfinkel at anl.gov>:> > Fewer metadata nodes by uniquing them. I would expect there might be > > quite a few loops that have identical metadata, e.g. all loops that > > have "setAlreaduUnrolled()" after loop unrolling. > > Okay. I have no idea how much code this would break (from none to a > lot). Any thoughts?My guess: nothing breaks. I have only seen this kind of nodes with LoopIDs and this RFC would remove the need for them to be unique (as explained in (a), this wasn't even a guaranteed property). Using self-referencing nodes seem to be a workaround when no 'distinct' nodes existed, exploiting that the uniquing is not using a graph minimization algorithm.> I think that, in general, you should have a separate > RFC if you'd like to change them metadata uniquing algorithm.I am not yet sure myself whether I want this. As a maybe easier alternative to changing the first element of a loop metadata node (e.g. to 'null'), it is strongly related to this RFC. Michael