hokein added inline comments.
================
Comment at: clang-tools-extra/pseudo/benchmarks/Benchmark.cpp:49
+static void runParseGrammar(benchmark::State &State) {
+ std::string GrammarText = readFile(GrammarFile);
+ for (auto _ : State)
----------------
sammccall wrote:
> rather than have each benchmark read the inputs and compile the grammar, it
> seems a bit less repetitive to do this in main() or a single setup function
> called from there, and share the const inputs.
>
> The only real downside I see is that some of the benchmarks don't require
> source, but source text could default to an empty string.
moved to a common setup which is invoked in the main function.
================
Comment at: clang-tools-extra/pseudo/benchmarks/Benchmark.cpp:64
+
+static void runGLRParse(benchmark::State &State) {
+ std::string GrammarText = readFile(GrammarFile);
----------------
sammccall wrote:
> if the benchmark is "runGLRParse" then everything apart from the glrParse
> call should be outside the test loop I think. (and such a benchmark is useful)
>
> If the benchmark is intended to be end-to-end (also useful) then it should
> have a more generic name, and we should expect to include disambiguation,
> syntax-tree forming etc in the future.
>
> I think we should probably have both!
yeah, my intent is to run an overall parse. Renamed to` runParseOverral`, and
added `runGLRParse` for the specific `glrParse` benchmark.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125226/new/
https://reviews.llvm.org/D125226
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
