[PATCH] D79215: [llvm][NFC] Inliner: simplify inlining decision logic

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat May 2 20:08:37 PDT 2020


dblaikie added a comment.

In D79215#2016572 <https://reviews.llvm.org/D79215#2016572>, @davidxl wrote:

> I believe there are other paths in the code that computes to None thus the use of Optional<..>. What I meant is the new dropping to None in the original version of the patch.  Never is used for noinline attribute so we don't want to convert other do not inline into it.


Oh! I see, you preferred the original version that differentiated between the two kinds of "no, don't inline" responses, as the original code did (the original code needed that differentiation to handle two different remarks - but now the new code doesn't, because the remark handling is internal - but I understand your goal better now, that you wanted to communicate those two states even though they are no longer/not currently needed to be differentiated by callers at the moment). My take on it was that, now that callers don't (currently) need to differentiate, it'd be simpler if the interface didn't expose that difference at all.

> Having a new wrapper class for 2 uses do look like a borderline overkill -- so revert to the original version is fine to me.

OK - in any case, I do appreciate the conversation/better understanding your perspective.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79215





More information about the llvm-commits mailing list