mtrofin marked 3 inline comments as done.
mtrofin added inline comments.
================
Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52
+namespace {
+using namespace llvm::ore;
----------------
wenlei wrote:
> mtrofin wrote:
> > wenlei wrote:
> > > curious why do we need anonymous namespace here?
> > iiuc it's preferred we place file-local types inside an anonymous
> > namespace.
> >
> > Looking now at the [[
> > https://llvm.org/docs/CodingStandards.html#anonymous-namespaces | style
> > guideline ]], it touts their benefits but also says I should have only
> > placed de decl there and the impl of those members out... but the members
> > are quite trivial. Happy to move them out though.
> Thanks for the pointer. I don't have a strong opinion but slightly leaning
> towards moving out of anonymous namespace be consistent with how other
> InlineAdvice is organized (DefaultInlineAdvice, MLInlineAdvice not in
> anonymous namespace).
Ah, those are public (i.e. in a .h file)
================
Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:72-89
+ void recordUnsuccessfulInliningImpl(const InlineResult &Result) override {
+ if (IsInliningRecommended)
+ ORE.emit([&]() {
+ return OptimizationRemarkMissed(DEBUG_TYPE, "NotInlined", DLoc, Block)
+ << "'" << NV("Callee", Callee) << "' is not AlwaysInline into '"
+ << NV("Caller", Caller)
+ << "': " << NV("Reason", Result.getFailureReason());
----------------
aeubanks wrote:
> can we add a test for these?
I think that would be tricky, because they should not actually happen - the way
we determine whether a site is alwaysinlinable checks (but not thoroughly) for
legality. Let me see if I can find a regression test. It may be we can
synthesize such a case in IR only, though, so not much of a help for the
frontend tests?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110891/new/
https://reviews.llvm.org/D110891
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
