Просмотр исходного кода

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.
Jon Ross-Perkins 10 месяцев назад
Родитель
Сommit
57ef976802

+ 1 - 0
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",
     ],

+ 66 - 0
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<Unit> units,
+    llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> 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<Unit> units,
     llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> tree_and_subtrees_getters,
@@ -436,6 +500,8 @@ auto CheckParseTrees(
       }
     }
   }
+
+  MaybeDumpSemIR(units, tree_and_subtrees_getters, options);
 }
 
 }  // namespace Carbon::Check

+ 23 - 0
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<bool> 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

+ 0 - 1
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:

+ 0 - 2
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 {

+ 0 - 1
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 {

+ 0 - 3
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",

+ 26 - 68
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);

+ 2 - 7
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<llvm::StringRef> input_filenames;

+ 0 - 1
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 {

+ 39 - 0
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

+ 1 - 0
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",

+ 13 - 1
toolchain/lex/lex.cpp

@@ -10,6 +10,7 @@
 #include <utility>
 
 #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

+ 9 - 0
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.

+ 1 - 0
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",

+ 1 - 7
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<llvm::Module> {
+auto Context::Finalize() && -> std::unique_ptr<llvm::Module> {
   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_);
 }
 

+ 2 - 4
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<llvm::Module>;
+  // cannot be used further after calling this.
+  auto Finalize() && -> std::unique_ptr<llvm::Module>;
 
   // Returns location information for use with DebugInfo.
   auto GetLocForDI(SemIR::AbsoluteNodeId abs_node_id) -> LocForDI;

+ 20 - 1
toolchain/lower/lower.cpp

@@ -7,6 +7,8 @@
 #include <memory>
 #include <optional>
 
+#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<llvm::Module> 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

+ 3 - 0
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.

+ 16 - 7
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;

+ 6 - 0
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`.