[Diffusion] rL360444: [LSR] Tweak setup cost depth threshold to 10.

Sjoerd Meijer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 13 09:04:27 PDT 2019


SjoerdMeijer added a comment.

> Fair enough, I've reverted this in r360589. The test case seemed cumbersome to reduce and commit just for a threshold change, but I'm not sure what the best approach is here yet beyond a depth limit change.

Mmm, okay. To be honest, I also don't know what the right approach is here, and am not sure a revert was necessary. But from a quick glance at our results this morning, there were quite some changes, positive and negative, but mostly down overall.

>From what I understand, this `SetupCostDepthLimit` is a bit of a funny one, because it says something about the cost of instructions outside the loop. If changes in this magic number causes big differences in performance, then probably something funny is going on with the modeling of instructions inside the loop. I guess this means we need to understand what exactly is going on here, but the fact we can have these big swings in performance is perhaps not really a good sign.

> As for rdar, that's been accepted in LLVM for a long time in commit messages, as long as they're not in code comments.

Okay, fair enough, although I see absolutely no value for this to others in the community.


Users:
  aemerson (Author)

https://reviews.llvm.org/rL360444





More information about the llvm-commits mailing list