From 57ef97680282de91622ffb07d0a0bf407b752682 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Tue, 1 Jul 2025 08:51:41 -0700 Subject: [PATCH] Move dumping into the phase factory functions (#5747) By moving dumping, we can have dumping occur before verification that might CHECK-fail (e.g. parse tree and llvm IR verification). I'm dropping vlogging of raw semir. It was only done when dumping, so `-v` would print zero copies and `-v --dump-raw-sem-ir` would print two copies. The lack of complaints about this suggests it's not needed. I'm making a small change to drop newlines between textual and raw semir. This is an edge case so I don't expect people to really notice in general, but it seemed unusually aware of what's on a stream, and it made it harder to do the dump_stream/raw_dump_stream approach, which I felt would be decent in general, since check is the only phase that can emit two different things (which I could also just drop -- we don't really use raw semir anymore, it doesn't seem like a big need to be able to print it with textual semir, but I'm assuming to just maintain existing behavior). In parse, we were previously dumping the tree on verification errors. I'm removing that because now `--dump-parse-tree` should work fine, where previously it wouldn't. --- toolchain/check/BUILD | 1 + toolchain/check/check.cpp | 66 +++++++++++++ toolchain/check/check.h | 23 +++++ .../basics/raw_sem_ir/builtins.carbon | 1 - .../multifile_with_textual_ir.carbon | 2 - .../one_file_with_textual_ir.carbon | 1 - toolchain/driver/BUILD | 3 - toolchain/driver/compile_subcommand.cpp | 94 +++++-------------- toolchain/driver/compile_subcommand.h | 9 +- toolchain/driver/testdata/stdin.carbon | 1 - toolchain/driver/testdata/verbose.carbon | 39 ++++++++ toolchain/lex/BUILD | 1 + toolchain/lex/lex.cpp | 14 ++- toolchain/lex/lex.h | 9 ++ toolchain/lower/BUILD | 1 + toolchain/lower/context.cpp | 8 +- toolchain/lower/context.h | 6 +- toolchain/lower/lower.cpp | 21 ++++- toolchain/lower/lower.h | 3 + toolchain/parse/parse.cpp | 23 +++-- toolchain/parse/parse.h | 6 ++ 21 files changed, 229 insertions(+), 103 deletions(-) create mode 100644 toolchain/driver/testdata/verbose.carbon diff --git a/toolchain/check/BUILD b/toolchain/check/BUILD index 661d6639d999..2921e83b2baf 100644 --- a/toolchain/check/BUILD +++ b/toolchain/check/BUILD @@ -202,6 +202,7 @@ cc_library( "//toolchain/sem_ir:entry_point", "//toolchain/sem_ir:expr_info", "//toolchain/sem_ir:file", + "//toolchain/sem_ir:formatter", "//toolchain/sem_ir:typed_insts", "@llvm-project//llvm:Support", ], diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index 00c5dcfbb5f8..07b5a9592dc8 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -19,6 +19,7 @@ #include "toolchain/parse/node_ids.h" #include "toolchain/parse/tree.h" #include "toolchain/sem_ir/file.h" +#include "toolchain/sem_ir/formatter.h" #include "toolchain/sem_ir/typed_insts.h" namespace Carbon::Check { @@ -322,6 +323,69 @@ static auto BuildApiMapAndDiagnosePackaging( return api_map; } +// Handles printing of formatted SemIR. +static auto MaybeDumpFormattedSemIR( + const SemIR::File& sem_ir, + Parse::GetTreeAndSubtreesFn tree_and_subtrees_getter, bool include_in_dumps, + const CheckParseTreesOptions& options) -> void { + bool dump = options.dump_stream && include_in_dumps; + if (!options.vlog_stream && !dump) { + return; + } + + bool has_ranges = sem_ir.parse_tree().tokens().has_dump_sem_ir_ranges(); + if (options.dump_sem_ir_ranges == + CheckParseTreesOptions::DumpSemIRRanges::Only && + !has_ranges) { + return; + } + + bool use_dump_sem_ir_ranges = + options.dump_sem_ir_ranges != + CheckParseTreesOptions::DumpSemIRRanges::Ignore && + has_ranges; + SemIR::Formatter formatter(&sem_ir, tree_and_subtrees_getter, + options.include_in_dumps, use_dump_sem_ir_ranges); + formatter.Format(); + if (options.vlog_stream) { + CARBON_VLOG_TO(options.vlog_stream, "*** SemIR::File ***\n"); + formatter.Write(*options.vlog_stream); + } + if (dump) { + formatter.Write(*options.dump_stream); + } +} + +// Handles options for dumping SemIR, including verbose output. +static auto MaybeDumpSemIR( + llvm::ArrayRef units, + llvm::ArrayRef tree_and_subtrees_getters, + const CheckParseTreesOptions& options) -> void { + if (!options.vlog_stream && !options.dump_stream && + !options.raw_dump_stream) { + return; + } + + // Flush diagnostics before printing. + for (const auto& unit : units) { + unit.consumer->Flush(); + } + + for (const auto& unit : units) { + bool include_in_dumps = + options.include_in_dumps[unit.sem_ir->check_ir_id().index]; + if (include_in_dumps && options.raw_dump_stream) { + unit.sem_ir->Print(*options.raw_dump_stream, + options.dump_raw_sem_ir_builtins); + } + + MaybeDumpFormattedSemIR( + *unit.sem_ir, + tree_and_subtrees_getters[unit.sem_ir->check_ir_id().index], + include_in_dumps, options); + } +} + auto CheckParseTrees( llvm::MutableArrayRef units, llvm::ArrayRef tree_and_subtrees_getters, @@ -436,6 +500,8 @@ auto CheckParseTrees( } } } + + MaybeDumpSemIR(units, tree_and_subtrees_getters, options); } } // namespace Carbon::Check diff --git a/toolchain/check/check.h b/toolchain/check/check.h index 4e31f3b8a0e4..fe918e2877e3 100644 --- a/toolchain/check/check.h +++ b/toolchain/check/check.h @@ -44,6 +44,29 @@ struct CheckParseTreesOptions { // Whether fuzzing is being run. Used to disable features we don't want to // fuzz. bool fuzzing = false; + + // Whether to include each unit in dumps. This is required when dumping + // (either of `dump_stream` or `raw_dump_stream`), and must have entries based + // on CheckIRId. + llvm::ArrayRef include_in_dumps = {}; + + // If set, SemIR will be dumped to this. + llvm::raw_ostream* dump_stream = nullptr; + + // When dumping textual SemIR (or printing it to for verbose output), whether + // to use ranges. + enum class DumpSemIRRanges : int8_t { + IfPresent, + Only, + Ignore, + }; + DumpSemIRRanges dump_sem_ir_ranges = DumpSemIRRanges::IfPresent; + + // If set, raw SemIR will be dumped to this. + llvm::raw_ostream* raw_dump_stream = nullptr; + + // When dumping raw SemIR, whether to include builtins. + bool dump_raw_sem_ir_builtins = false; }; // Checks a group of parse trees. This will use imports to decide the order of diff --git a/toolchain/check/testdata/basics/raw_sem_ir/builtins.carbon b/toolchain/check/testdata/basics/raw_sem_ir/builtins.carbon index 09e5a602ad04..53452efc1734 100644 --- a/toolchain/check/testdata/basics/raw_sem_ir/builtins.carbon +++ b/toolchain/check/testdata/basics/raw_sem_ir/builtins.carbon @@ -75,4 +75,3 @@ // CHECK:STDOUT: inst_block4: // CHECK:STDOUT: 0: inst14 // CHECK:STDOUT: ... -// CHECK:STDOUT: diff --git a/toolchain/check/testdata/basics/raw_sem_ir/multifile_with_textual_ir.carbon b/toolchain/check/testdata/basics/raw_sem_ir/multifile_with_textual_ir.carbon index 62f6785a8115..888ed1c8b393 100644 --- a/toolchain/check/testdata/basics/raw_sem_ir/multifile_with_textual_ir.carbon +++ b/toolchain/check/testdata/basics/raw_sem_ir/multifile_with_textual_ir.carbon @@ -82,7 +82,6 @@ fn B() { // CHECK:STDOUT: 0: inst14 // CHECK:STDOUT: 1: inst15 // CHECK:STDOUT: ... -// CHECK:STDOUT: // CHECK:STDOUT: --- a.carbon // CHECK:STDOUT: // CHECK:STDOUT: constants { @@ -188,7 +187,6 @@ fn B() { // CHECK:STDOUT: 1: inst15 // CHECK:STDOUT: 2: inst17 // CHECK:STDOUT: ... -// CHECK:STDOUT: // CHECK:STDOUT: --- b.carbon // CHECK:STDOUT: // CHECK:STDOUT: constants { diff --git a/toolchain/check/testdata/basics/raw_sem_ir/one_file_with_textual_ir.carbon b/toolchain/check/testdata/basics/raw_sem_ir/one_file_with_textual_ir.carbon index 43b123d867be..ad6c275b4f5a 100644 --- a/toolchain/check/testdata/basics/raw_sem_ir/one_file_with_textual_ir.carbon +++ b/toolchain/check/testdata/basics/raw_sem_ir/one_file_with_textual_ir.carbon @@ -186,7 +186,6 @@ fn Foo(n: ()) -> ((), ()) { // CHECK:STDOUT: 0: inst14 // CHECK:STDOUT: 1: inst36 // CHECK:STDOUT: ... -// CHECK:STDOUT: // CHECK:STDOUT: --- one_file_with_textual_ir.carbon // CHECK:STDOUT: // CHECK:STDOUT: constants { diff --git a/toolchain/driver/BUILD b/toolchain/driver/BUILD index a965e9347706..74c58a4614ef 100644 --- a/toolchain/driver/BUILD +++ b/toolchain/driver/BUILD @@ -139,10 +139,7 @@ cc_library( "//toolchain/lower", "//toolchain/parse", "//toolchain/parse:tree", - "//toolchain/sem_ir:dump", "//toolchain/sem_ir:file", - "//toolchain/sem_ir:formatter", - "//toolchain/sem_ir:inst_namer", "//toolchain/source:source_buffer", "@llvm-project//llvm:Core", "@llvm-project//llvm:Support", diff --git a/toolchain/driver/compile_subcommand.cpp b/toolchain/driver/compile_subcommand.cpp index 2a63be8a8283..b145082f731d 100644 --- a/toolchain/driver/compile_subcommand.cpp +++ b/toolchain/driver/compile_subcommand.cpp @@ -24,9 +24,6 @@ #include "toolchain/lower/lower.h" #include "toolchain/parse/parse.h" #include "toolchain/parse/tree_and_subtrees.h" -#include "toolchain/sem_ir/formatter.h" -#include "toolchain/sem_ir/import_ir.h" -#include "toolchain/sem_ir/inst_namer.h" #include "toolchain/source/source_buffer.h" namespace Carbon { @@ -196,6 +193,7 @@ prints full SemIR. )""", }, [&](auto& arg_b) { + using DumpSemIRRanges = Check::CheckParseTreesOptions::DumpSemIRRanges; arg_b.SetOneOf( { arg_b.OneOfValue("if-present", DumpSemIRRanges::IfPresent) @@ -432,9 +430,6 @@ class CompilationUnit { // significant overhead. Avoid constructing it when unused. auto GetParseTreeAndSubtrees() -> const Parse::TreeAndSubtrees&; - // Handles printing of formatted SemIR. - auto MaybePrintFormattedSemIR() -> void; - // Wraps a call with log statements to indicate start and end. Typically logs // with the actual function name, but marks timings with the appropriate // phase. @@ -609,17 +604,16 @@ auto CompilationUnit::RunLex() -> void { LogCall("Lex::Lex", "lex", [&] { Lex::LexOptions options; options.consumer = consumer_; + options.vlog_stream = vlog_stream_; + if (options_->dump_tokens && IncludeInDumps()) { + options.dump_stream = driver_env_->output_stream; + options.omit_file_boundary_tokens = options_->omit_file_boundary_tokens; + } tokens_ = Lex::Lex(value_stores_, *source_, options); }); - if (options_->dump_tokens && IncludeInDumps()) { - consumer_->Flush(); - tokens_->Print(*driver_env_->output_stream, - options_->omit_file_boundary_tokens); - } if (mem_usage_) { mem_usage_->Collect("tokens_", *tokens_); } - CARBON_VLOG("*** Lex::TokenizedBuffer ***\n{0}", tokens_); if (tokens_->has_errors()) { success_ = false; } @@ -630,21 +624,15 @@ auto CompilationUnit::RunParse() -> void { Parse::ParseOptions options; options.consumer = consumer_; options.vlog_stream = vlog_stream_; + if (options_->dump_parse_tree && IncludeInDumps()) { + options.dump_stream = driver_env_->output_stream; + options.dump_preorder_parse_tree = options_->preorder_parse_tree; + } parse_tree_ = Parse::Parse(*tokens_, options); }); - if (options_->dump_parse_tree && IncludeInDumps()) { - consumer_->Flush(); - const auto& tree_and_subtrees = GetParseTreeAndSubtrees(); - if (options_->preorder_parse_tree) { - tree_and_subtrees.PrintPreorder(*driver_env_->output_stream); - } else { - tree_and_subtrees.Print(*driver_env_->output_stream); - } - } if (mem_usage_) { mem_usage_->Collect("parse_tree_", *parse_tree_); } - CARBON_VLOG("*** Parse::Tree ***\n{0}", parse_tree_); if (parse_tree_->has_errors()) { success_ = false; } @@ -667,33 +655,6 @@ auto CompilationUnit::GetCheckUnit() -> Check::Unit { .cpp_ast = &cpp_ast_}; } -auto CompilationUnit::MaybePrintFormattedSemIR() -> void { - bool print = options_->dump_sem_ir && IncludeInDumps(); - if (!vlog_stream_ && !print) { - return; - } - - if (options_->dump_sem_ir_ranges == CompileOptions::DumpSemIRRanges::Only && - !tokens_->has_dump_sem_ir_ranges()) { - return; - } - - bool use_dump_sem_ir_ranges = - options_->dump_sem_ir_ranges != CompileOptions::DumpSemIRRanges::Ignore && - tokens_->has_dump_sem_ir_ranges(); - SemIR::Formatter formatter(&*sem_ir_, *tree_and_subtrees_getter_, - cache_->include_in_dumps(), - use_dump_sem_ir_ranges); - formatter.Format(); - if (vlog_stream_) { - CARBON_VLOG("*** SemIR::File ***\n"); - formatter.Write(*vlog_stream_); - } - if (print) { - formatter.Write(*driver_env_->output_stream); - } -} - auto CompilationUnit::PostCheck() -> void { CARBON_CHECK(sem_ir_, "Must call GetCheckUnit first"); @@ -706,15 +667,6 @@ auto CompilationUnit::PostCheck() -> void { mem_usage_->Collect("sem_ir_", *sem_ir_); } - if (options_->dump_raw_sem_ir && IncludeInDumps()) { - CARBON_VLOG("*** Raw SemIR::File ***\n{0}\n", *sem_ir_); - sem_ir_->Print(*driver_env_->output_stream, options_->builtin_sem_ir); - if (options_->dump_sem_ir) { - *driver_env_->output_stream << "\n"; - } - } - - MaybePrintFormattedSemIR(); if (sem_ir_->has_errors()) { success_ = false; } @@ -728,20 +680,13 @@ auto CompilationUnit::RunLower() -> void { options_->run_llvm_verifier ? driver_env_->error_stream : nullptr; options.want_debug_info = options_->include_debug_info; options.vlog_stream = vlog_stream_; + if (options_->dump_llvm_ir && IncludeInDumps()) { + options.dump_stream = driver_env_->output_stream; + } module_ = Lower::LowerToLLVM(*llvm_context_, driver_env_->fs, cache_->tree_and_subtrees_getters(), *sem_ir_, options); }); - if (vlog_stream_) { - CARBON_VLOG("*** llvm::Module ***\n"); - module_->print(*vlog_stream_, /*AAW=*/nullptr, - /*ShouldPreserveUseListOrder=*/false, - /*IsForDebug=*/true); - } - if (options_->dump_llvm_ir && IncludeInDumps()) { - module_->print(*driver_env_->output_stream, /*AAW=*/nullptr, - /*ShouldPreserveUseListOrder=*/true); - } } auto CompilationUnit::RunCodeGen() -> void { @@ -985,6 +930,19 @@ auto CompileSubcommand::Run(DriverEnv& driver_env) -> DriverResult { options.prelude_import = options_.prelude_import; options.vlog_stream = driver_env.vlog_stream; options.fuzzing = driver_env.fuzzing; + if (options.vlog_stream || options_.dump_sem_ir || options_.dump_raw_sem_ir) { + options.include_in_dumps = cache.include_in_dumps(); + if (options_.dump_sem_ir) { + options.dump_stream = driver_env.output_stream; + } + if (options.vlog_stream || options_.dump_sem_ir) { + options.dump_sem_ir_ranges = options_.dump_sem_ir_ranges; + } + if (options_.dump_raw_sem_ir) { + options.raw_dump_stream = driver_env.output_stream; + options.dump_raw_sem_ir_builtins = options_.builtin_sem_ir; + } + } Check::CheckParseTrees(check_units, cache.tree_and_subtrees_getters(), driver_env.fs, options_.codegen_options.target, options); diff --git a/toolchain/driver/compile_subcommand.h b/toolchain/driver/compile_subcommand.h index a31bd3811dec..c8fb5d96bc95 100644 --- a/toolchain/driver/compile_subcommand.h +++ b/toolchain/driver/compile_subcommand.h @@ -10,6 +10,7 @@ #include "common/ostream.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" +#include "toolchain/check/check.h" #include "toolchain/driver/codegen_options.h" #include "toolchain/driver/driver_env.h" #include "toolchain/driver/driver_subcommand.h" @@ -28,12 +29,6 @@ struct CompileOptions { CodeGen, }; - enum class DumpSemIRRanges : int8_t { - IfPresent, - Only, - Ignore, - }; - friend auto operator<<(llvm::raw_ostream& out, Phase phase) -> llvm::raw_ostream&; @@ -42,7 +37,7 @@ struct CompileOptions { CodegenOptions codegen_options; Phase phase; - DumpSemIRRanges dump_sem_ir_ranges; + Check::CheckParseTreesOptions::DumpSemIRRanges dump_sem_ir_ranges; llvm::StringRef output_filename; llvm::SmallVector input_filenames; diff --git a/toolchain/driver/testdata/stdin.carbon b/toolchain/driver/testdata/stdin.carbon index 5e5c7576766b..76bb6a4ab3f5 100644 --- a/toolchain/driver/testdata/stdin.carbon +++ b/toolchain/driver/testdata/stdin.carbon @@ -59,7 +59,6 @@ // CHECK:STDOUT: inst_block4: // CHECK:STDOUT: 0: inst14 // CHECK:STDOUT: ... -// CHECK:STDOUT: // CHECK:STDOUT: --- - // CHECK:STDOUT: // CHECK:STDOUT: file { diff --git a/toolchain/driver/testdata/verbose.carbon b/toolchain/driver/testdata/verbose.carbon new file mode 100644 index 000000000000..4e4917eec00b --- /dev/null +++ b/toolchain/driver/testdata/verbose.carbon @@ -0,0 +1,39 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/none.carbon +// ARGS: -v compile --phase=codegen --target=x86_64-unknown-linux-gnu --output=%t %s +// +// Verifies that various phases are included in vlog output. +// +// SET-CHECK-SUBSET +// NOAUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/verbose.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/verbose.carbon +// CHECK:STDERR: *** SourceBuffer *** +// CHECK:STDERR: *** Lex::Lex: empty.carbon *** +// CHECK:STDERR: *** Lex::TokenizedBuffer *** +// CHECK:STDERR: - filename: empty.carbon +// CHECK:STDERR: *** Lex::Lex done *** +// CHECK:STDERR: *** Parse::Parse: empty.carbon *** +// CHECK:STDERR: *** Parse::Tree *** +// CHECK:STDERR: - filename: empty.carbon +// CHECK:STDERR: *** Parse::Parse done *** +// CHECK:STDERR: *** Check::CheckParseTrees *** +// CHECK:STDERR: *** SemIR::File *** +// CHECK:STDERR: --- empty.carbon +// CHECK:STDERR: *** Check::CheckParseTrees done *** +// CHECK:STDERR: *** Lower::LowerToLLVM: empty.carbon *** +// CHECK:STDERR: *** llvm::Module *** +// CHECK:STDERR: ; ModuleID = 'empty.carbon' +// CHECK:STDERR: source_filename = "empty.carbon" +// CHECK:STDERR: *** Lower::LowerToLLVM done *** +// CHECK:STDERR: *** CodeGen: empty.carbon *** +// CHECK:STDERR: *** Assembly *** +// CHECK:STDERR: {{\t}}.file{{\t}}"empty.carbon" +// CHECK:STDERR: *** CodeGen done *** + +// --- empty.carbon diff --git a/toolchain/lex/BUILD b/toolchain/lex/BUILD index 39bd15f36986..bc3399453625 100644 --- a/toolchain/lex/BUILD +++ b/toolchain/lex/BUILD @@ -192,6 +192,7 @@ cc_library( ":token_kind", ":tokenized_buffer", "//common:check", + "//common:vlog", "//toolchain/base:kind_switch", "//toolchain/base:shared_value_stores", "//toolchain/diagnostics:diagnostic_emitter", diff --git a/toolchain/lex/lex.cpp b/toolchain/lex/lex.cpp index 8d37cb170055..9904b235f53e 100644 --- a/toolchain/lex/lex.cpp +++ b/toolchain/lex/lex.cpp @@ -10,6 +10,7 @@ #include #include "common/check.h" +#include "common/vlog.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/Support/Compiler.h" @@ -1697,7 +1698,18 @@ auto Lex(SharedValueStores& value_stores, SourceBuffer& source, LexOptions options) -> TokenizedBuffer { auto* consumer = options.consumer ? options.consumer : &Diagnostics::ConsoleConsumer(); - return Lexer(value_stores, source, *consumer).Lex(); + auto tokens = Lexer(value_stores, source, *consumer).Lex(); + + if (options.vlog_stream || options.dump_stream) { + // Flush diagnostics before printing. + consumer->Flush(); + } + CARBON_VLOG_TO(options.vlog_stream, "*** Lex::TokenizedBuffer ***\n{0}", + tokens); + if (options.dump_stream) { + tokens.Print(*options.dump_stream, options.omit_file_boundary_tokens); + } + return tokens; } } // namespace Carbon::Lex diff --git a/toolchain/lex/lex.h b/toolchain/lex/lex.h index eeee53d16509..ec45029106e6 100644 --- a/toolchain/lex/lex.h +++ b/toolchain/lex/lex.h @@ -18,6 +18,15 @@ struct LexOptions { // If set, a consumer for diagnostics. Otherwise, diagnostics go to stderr. Diagnostics::Consumer* consumer = nullptr; + + // If set, enables verbose output. + llvm::raw_ostream* vlog_stream = nullptr; + + // If set, tokens will be dumped to this. + llvm::raw_ostream* dump_stream = nullptr; + + // When dumping, whether to omit `FileStart` and `FileEnd` in output. + bool omit_file_boundary_tokens = false; }; // Lexes a buffer of source code into a tokenized buffer. diff --git a/toolchain/lower/BUILD b/toolchain/lower/BUILD index 7fccc0277956..06c6dbf679d8 100644 --- a/toolchain/lower/BUILD +++ b/toolchain/lower/BUILD @@ -17,6 +17,7 @@ cc_library( hdrs = ["lower.h"], deps = [ ":context", + "//common:vlog", "//toolchain/parse:tree", "//toolchain/sem_ir:file", "//toolchain/sem_ir:inst_namer", diff --git a/toolchain/lower/context.cpp b/toolchain/lower/context.cpp index f08248f48c08..167afd336ecd 100644 --- a/toolchain/lower/context.cpp +++ b/toolchain/lower/context.cpp @@ -8,7 +8,6 @@ #include "common/growing_range.h" #include "common/raw_string_ostream.h" #include "common/vlog.h" -#include "llvm/IR/Verifier.h" #include "llvm/Transforms/Utils/ModuleUtils.h" #include "toolchain/lower/file_context.h" #include "toolchain/sem_ir/inst_namer.h" @@ -51,17 +50,12 @@ auto Context::LowerPendingDefinitions() -> void { } } -auto Context::Finalize( - llvm::raw_ostream* - llvm_verifier_stream) && -> std::unique_ptr { +auto Context::Finalize() && -> std::unique_ptr { LowerPendingDefinitions(); file_contexts_.ForEach( [](auto, auto& file_context) { file_context->Finalize(); }); - if (llvm_verifier_stream) { - CARBON_CHECK(!llvm::verifyModule(*llvm_module_, llvm_verifier_stream)); - } return std::move(llvm_module_); } diff --git a/toolchain/lower/context.h b/toolchain/lower/context.h index c02a2e3966ae..7546e28b14bb 100644 --- a/toolchain/lower/context.h +++ b/toolchain/lower/context.h @@ -62,10 +62,8 @@ class Context { } // Finishes lowering and takes ownership of the LLVM module. The context - // cannot be used further after calling this. If `llvm_verifier_stream` is - // non-null, verifies the LLVM module before returning it. - auto Finalize(llvm::raw_ostream* - llvm_verifier_stream) && -> std::unique_ptr; + // cannot be used further after calling this. + auto Finalize() && -> std::unique_ptr; // Returns location information for use with DebugInfo. auto GetLocForDI(SemIR::AbsoluteNodeId abs_node_id) -> LocForDI; diff --git a/toolchain/lower/lower.cpp b/toolchain/lower/lower.cpp index cea8f67ae096..8009cf827c31 100644 --- a/toolchain/lower/lower.cpp +++ b/toolchain/lower/lower.cpp @@ -7,6 +7,8 @@ #include #include +#include "common/vlog.h" +#include "llvm/IR/Verifier.h" #include "toolchain/lower/context.h" #include "toolchain/lower/file_context.h" @@ -27,7 +29,24 @@ auto LowerToLLVM( SemIR::InstNamer inst_namer(&sem_ir); context.GetFileContext(&sem_ir, &inst_namer).LowerDefinitions(); - return std::move(context).Finalize(options.llvm_verifier_stream); + std::unique_ptr module = std::move(context).Finalize(); + + if (options.vlog_stream) { + CARBON_VLOG_TO(options.vlog_stream, "*** llvm::Module ***\n"); + module->print(*options.vlog_stream, /*AAW=*/nullptr, + /*ShouldPreserveUseListOrder=*/false, + /*IsForDebug=*/true); + } + if (options.dump_stream) { + module->print(*options.dump_stream, /*AAW=*/nullptr, + /*ShouldPreserveUseListOrder=*/true); + } + + if (options.llvm_verifier_stream) { + CARBON_CHECK(!llvm::verifyModule(*module, options.llvm_verifier_stream)); + } + + return module; } } // namespace Carbon::Lower diff --git a/toolchain/lower/lower.h b/toolchain/lower/lower.h index 674c5ecfbf10..31b7f8c97007 100644 --- a/toolchain/lower/lower.h +++ b/toolchain/lower/lower.h @@ -26,6 +26,9 @@ struct LowerToLLVMOptions { // If set, enables verbose output. llvm::raw_ostream* vlog_stream = nullptr; + + // If set, LLVM IR will be dumped to this in textual form. + llvm::raw_ostream* dump_stream = nullptr; }; // Lowers SemIR to LLVM IR. diff --git a/toolchain/parse/parse.cpp b/toolchain/parse/parse.cpp index 447d3b19247d..5c837781ff7b 100644 --- a/toolchain/parse/parse.cpp +++ b/toolchain/parse/parse.cpp @@ -9,6 +9,7 @@ #include "toolchain/parse/context.h" #include "toolchain/parse/handle.h" #include "toolchain/parse/node_kind.h" +#include "toolchain/parse/tree_and_subtrees.h" namespace Carbon::Parse { @@ -48,14 +49,22 @@ auto Parse(Lex::TokenizedBuffer& tokens, ParseOptions options) -> Tree { // from the tokenized buffer or we diagnosed new errors. tree.set_has_errors(tokens.has_errors() || context.has_errors()); + if (options.vlog_stream || options.dump_stream) { + // Flush diagnostics before printing. + consumer->Flush(); + } + CARBON_VLOG_TO(options.vlog_stream, "*** Parse::Tree ***\n{0}", tree); + if (options.dump_stream) { + Parse::TreeAndSubtrees tree_and_subtrees(tokens, tree); + if (options.dump_preorder_parse_tree) { + tree_and_subtrees.PrintPreorder(*options.dump_stream); + } else { + tree_and_subtrees.Print(*options.dump_stream); + } + } + if (auto verify = tree.Verify(); !verify.ok()) { - // TODO: This is temporarily printing to stderr directly during development. - // If we can, restrict this to a subtree with the error and add it to the - // stack trace (such as with PrettyStackTraceFunction). Otherwise, switch - // back to vlog_stream prior to broader distribution so that end users are - // hopefully comfortable copy-pasting stderr when there are bugs in tree - // construction. - tree.Print(llvm::errs()); + // TODO: Consider printing a subtree as part of the error. CARBON_FATAL("Invalid tree returned by Parse(): {0}", verify.error()); } return tree; diff --git a/toolchain/parse/parse.h b/toolchain/parse/parse.h index afbf568e9775..c266199bd550 100644 --- a/toolchain/parse/parse.h +++ b/toolchain/parse/parse.h @@ -21,6 +21,12 @@ struct ParseOptions { // If set, enables verbose output. llvm::raw_ostream* vlog_stream = nullptr; + + // If set, the parse tree will be dumped to this. + llvm::raw_ostream* dump_stream = nullptr; + + // When dumping, whether to dump in preorder; otherwise, postorder is used. + bool dump_preorder_parse_tree = false; }; // Parses the token buffer into a `Tree`.