[PATCH] D101228: [InlineCost] CallAnalyzer: use TTI info for extractvalue - they are free (PR50099)
Arthur Eubanks via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 29 11:24:42 PDT 2021
aeubanks added a comment.
overall looks good now, just nits
================
Comment at: llvm/lib/Analysis/InlineCost.cpp:1782
+ // SROA can't look through these, but they may be free.
+ return visitInstruction(I);
}
----------------
lebedev.ri wrote:
> aeubanks wrote:
> > I think `Base::visitExtractValue()` is the proper way
> Well, then all the code that uses this "proper way" is likely broken,
> and either doesn't have test coverage,
> or the test changes weren't analyzed well enough.
>
> Because then we don't fall back to `CallAnalyzer::visitInstruction()`,
> and don't consider it as zero-cost as per costmodel...
I thought `Base::visitExtractValue()` ends up calling `CallAnalyzer::visitInstruction()`. `Base::visitExtractValue()` goes through InstVisitor's instruction hierarchy to figure out what to call next. Currently it always ends up calling `CallAnalyzer::visitInstruction()`, but if in the future InstVisitor put something in between `visitExtractValue()` and `visitInstruction()` then we'd want to respect that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101228/new/
https://reviews.llvm.org/D101228
More information about the llvm-commits
mailing list