[PATCH] D123400: [LoopCacheAnalysis] Consider dimension depth of the subscript reference when calculating cost

Mon Apr 11 14:30:12 PDT 2022

bmahjour added a comment.

In D123400#3442881 <https://reviews.llvm.org/D123400#3442881>, @amehsan wrote:

> 1. Boiling down various parameters to one final "cost" value has a disadvantage. In fact we discussed this with @congzhe as the first solution, but we discarded it because essentially you lose some informaiton when you boil everything down to one number. The reason that Congzhe decided to go with a two component cost was that extra detail, carries more useful information and helps to make more accurate decision. Now this was a theoretical concern and I am not sure if in practice it holds or not. But that is the background on congzhe's work.

The cost returned by the analysis must consider the impact of outer dimension strides incurred by each reference group, otherwise it will not be accurate. If, for whatever reason, we need extra details in future we could provide extra interfaces to the analysis to query those details.

> 1. Wouldn't it be more appropriate to continue discussion on congzhe's patch rather than posting a parallel patch to address the same problem? I think that will be more constructive given congzhe has already identified the problem, investigated some solutions and developed a patch? I suggest we continue the discussion, as review comments on his patch. Once there is an agreement on the cost function, Congzhe can update the patch (assuming it is needed) to reflect the agreed upon solution.

I explained my reasoning for needing a different approach in D122776 <https://reviews.llvm.org/D122776>. I just figured it would be a lot easier to show what I mean by posting a new patch instead of trying to describe it as a change request on top of D122776 <https://reviews.llvm.org/D122776>. I'm happy to continue further discussion on this wherever seems more appropriate to you or other reviewers.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123400/new/

https://reviews.llvm.org/D123400