[PATCH] D88215: Add llvm.loop.unroll.threshold metadata

[Tim Corringham via Phabricator via llvm-commits]

timcorringham added a comment.

I guess the loop unroll threshold metadata could be consumed directly by the LoopUnrollPass.cpp, but that would cut out some functionality that is useful (at least for AMDGPU) - the target specific unroll options code for AMDGPU has heuristics that can adjust the unroll threshold. The metadata gives the starting point, but the value may be increased in he presence of certain constructs.  If the metadata was used as the final threshold in LoopUnrollPass.cpp then to get the same behaviour the front-end would have to perform the same analysis to adjust the threshold value - which would have an impact on compile time, and duplicate code, etc.

As for whether the metadata should be llvm generic or amdgpu specific, I guess the question is whether any other targets would find this useful?  My original approach was to make it amdgpu specific, but reviewers considered that it could be generic - hence this second review.

I'm not sure I'd describe the threshold semantics as undefined - I would describe it as imprecise. If a change has the effect of pushing code over a threshold (in either direction), the effect may be undesirable or unexpected, but equally it can mitigate the impact of changes that would otherwise be undesirable.  In any case, the threshold approach is used in the loop unroll pass, so  using any other control (such as a precise loop unroll count) would require significantly more effort (the unroll count would have to be calculated in advance, which may still use a threshold).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88215