[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