Auto merge of rust-lang#144722 - ywxt:parallel-reproducibile, r=SparrowLii

Fix parallel rustc not being reproducible due to unstable sorts of items

Currently, A tuple `(DefId, SymbolName)` is used to determine the order of items in the final binary. However `DefId` is expected as non-deterministic, which leads to some not reproducible issues under parallel compilation. (See rust-lang#140425 (comment))

Theoretically, we don't need the sorting because the order of these items is already deterministic.

However, codegen tests reply on the same order of items  between in binary and source.

So here we added a new option `codegen-source-order` to indicate whether sorting based on the order in source. For codegen tests, items are sorted according to the order in the source code, whereas in the normal path, no sorting is performed.

Specially, for codegen tests, in preparation for parallel compilation potentially being enabled by default in the future,  we use `Span` replacing `DefId` to make the order deterministic.

This PR is purposed to fix rust-lang#140425, but seemly works on rust-lang#140413 too.

This behavior hasn't added into any test until we have a test suit for the parallel frontend. (See rust-lang#143953)

Related discussion: [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/187679-t-compiler.2Fparallel-rustc/topic/Async.20closures.20not.20reproducible.28.23140425.29) rust-lang#144576

Update rust-lang#113349

r? `@oli-obk`
cc `@lqd` `@cramertj` `@matthiaskrgr` `@Zoxc` `@SparrowLii` `@bjorn3` `@cjgillot` `@joshtriplett`

Author: bors

compiler/rustc_interface/src/tests.rs

@@ -689,6 +689,7 @@ fn test_unstable_options_tracking_hash() {
     // Make sure that changing an [UNTRACKED] option leaves the hash unchanged.
     // tidy-alphabetical-start
     untracked!(assert_incr_state, Some(String::from("loaded")));
+    untracked!(codegen_source_order, true);
     untracked!(deduplicate_diagnostics, false);
     untracked!(dump_dep_graph, true);
     untracked!(dump_mir, Some(String::from("abc")));

compiler/rustc_middle/src/mir/mono.rs

@@ -12,7 +12,6 @@ use rustc_hashes::Hash128;
 use rustc_hir::ItemId;
 use rustc_hir::attrs::InlineAttr;
 use rustc_hir::def_id::{CrateNum, DefId, DefIdSet, LOCAL_CRATE};
-use rustc_index::Idx;
 use rustc_macros::{HashStable, TyDecodable, TyEncodable};
 use rustc_query_system::ich::StableHashingContext;
 use rustc_session::config::OptLevel;
@@ -526,44 +525,50 @@ impl<'tcx> CodegenUnit<'tcx> {
         tcx: TyCtxt<'tcx>,
     ) -> Vec<(MonoItem<'tcx>, MonoItemData)> {
         // The codegen tests rely on items being process in the same order as
-        // they appear in the file, so for local items, we sort by node_id first
+        // they appear in the file, so for local items, we sort by span first
         #[derive(PartialEq, Eq, PartialOrd, Ord)]
-        struct ItemSortKey<'tcx>(Option<usize>, SymbolName<'tcx>);
-
+        struct ItemSortKey<'tcx>(Option<Span>, SymbolName<'tcx>);
+
+        // We only want to take HirIds of user-defines instances into account.
+        // The others don't matter for the codegen tests and can even make item
+        // order unstable.
+        fn local_item_id<'tcx>(item: MonoItem<'tcx>) -> Option<DefId> {
+            match item {
+                MonoItem::Fn(ref instance) => match instance.def {
+                    InstanceKind::Item(def) => def.as_local().map(|_| def),
+                    InstanceKind::VTableShim(..)
+                    | InstanceKind::ReifyShim(..)
+                    | InstanceKind::Intrinsic(..)
+                    | InstanceKind::FnPtrShim(..)
+                    | InstanceKind::Virtual(..)
+                    | InstanceKind::ClosureOnceShim { .. }
+                    | InstanceKind::ConstructCoroutineInClosureShim { .. }
+                    | InstanceKind::DropGlue(..)
+                    | InstanceKind::CloneShim(..)
+                    | InstanceKind::ThreadLocalShim(..)
+                    | InstanceKind::FnPtrAddrShim(..)
+                    | InstanceKind::AsyncDropGlue(..)
+                    | InstanceKind::FutureDropPollShim(..)
+                    | InstanceKind::AsyncDropGlueCtorShim(..) => None,
+                },
+                MonoItem::Static(def_id) => def_id.as_local().map(|_| def_id),
+                MonoItem::GlobalAsm(item_id) => Some(item_id.owner_id.def_id.to_def_id()),
+            }
+        }
         fn item_sort_key<'tcx>(tcx: TyCtxt<'tcx>, item: MonoItem<'tcx>) -> ItemSortKey<'tcx> {
             ItemSortKey(
-                match item {
-                    MonoItem::Fn(ref instance) => {
-                        match instance.def {
-                            // We only want to take HirIds of user-defined
-                            // instances into account. The others don't matter for
-                            // the codegen tests and can even make item order
-                            // unstable.
-                            InstanceKind::Item(def) => def.as_local().map(Idx::index),
-                            InstanceKind::VTableShim(..)
-                            | InstanceKind::ReifyShim(..)
-                            | InstanceKind::Intrinsic(..)
-                            | InstanceKind::FnPtrShim(..)
-                            | InstanceKind::Virtual(..)
-                            | InstanceKind::ClosureOnceShim { .. }
-                            | InstanceKind::ConstructCoroutineInClosureShim { .. }
-                            | InstanceKind::DropGlue(..)
-                            | InstanceKind::CloneShim(..)
-                            | InstanceKind::ThreadLocalShim(..)
-                            | InstanceKind::FnPtrAddrShim(..)
-                            | InstanceKind::AsyncDropGlue(..)
-                            | InstanceKind::FutureDropPollShim(..)
-                            | InstanceKind::AsyncDropGlueCtorShim(..) => None,
-                        }
-                    }
-                    MonoItem::Static(def_id) => def_id.as_local().map(Idx::index),
-                    MonoItem::GlobalAsm(item_id) => Some(item_id.owner_id.def_id.index()),
-                },
+                local_item_id(item)
+                    .map(|def_id| tcx.def_span(def_id).find_ancestor_not_from_macro())
+                    .flatten(),
                 item.symbol_name(tcx),
             )
         }
 
         let mut items: Vec<_> = self.items().iter().map(|(&i, &data)| (i, data)).collect();
+        if !tcx.sess.opts.unstable_opts.codegen_source_order {
+            // It's already deterministic, so we can just use it.
+            return items;
+        }
         items.sort_by_cached_key(|&(i, _)| item_sort_key(tcx, i));
         items
     }

compiler/rustc_session/src/options.rs

@@ -2165,6 +2165,8 @@ options! {
         "hash algorithm of source files used to check freshness in cargo (`blake3` or `sha256`)"),
     codegen_backend: Option<String> = (None, parse_opt_string, [TRACKED],
         "the backend to use"),
+    codegen_source_order: bool = (false, parse_bool, [UNTRACKED],
+        "emit mono items in the order of spans in source files (default: no)"),
     contract_checks: Option<bool> = (None, parse_opt_bool, [TRACKED],
         "emit runtime checks for contract pre- and post-conditions (default: no)"),
     coverage_options: CoverageOptions = (CoverageOptions::default(), parse_coverage_options, [TRACKED],

src/doc/unstable-book/src/compiler-flags/codegen-source-order.md

@@ -0,0 +1,12 @@
+# `codegen-source-order`
+
+---
+
+This feature allows you to have a predictive and
+deterministic order for items after codegen, which
+is the same as in source code.
+
+For every `CodegenUnit`, local `MonoItem`s would
+be sorted by `(Span, SymbolName)`, which
+makes codegen tests rely on the order of items in
+source files work.

src/tools/compiletest/src/runtest.rs

@@ -1695,6 +1695,10 @@ impl<'test> TestCx<'test> {
             }
             TestMode::Assembly | TestMode::Codegen => {
                 rustc.arg("-Cdebug-assertions=no");
+                // For assembly and codegen tests, we want to use the same order
+                // of the items of a codegen unit as the source order, so that
+                // we can compare the output with the source code through filecheck.
+                rustc.arg("-Zcodegen-source-order");
             }
             TestMode::Crashes => {
                 set_mir_dump_dir(&mut rustc);

src/tools/run-make-support/src/external_deps/rustc.rs

@@ -405,6 +405,12 @@ impl Rustc {
         };
         self
     }
+
+    /// Make that the generated LLVM IR is in source order.
+    pub fn codegen_source_order(&mut self) -> &mut Self {
+        self.cmd.arg("-Zcodegen-source-order");
+        self
+    }
 }
 
 /// Query the sysroot path corresponding `rustc --print=sysroot`.

tests/run-make/pgo-branch-weights/rmake.rs

@@ -17,15 +17,21 @@ use run_make_support::{llvm_filecheck, llvm_profdata, rfs, run_with_args, rustc}
 fn main() {
     let path_prof_data_dir = Path::new("prof_data_dir");
     let path_merged_profdata = path_prof_data_dir.join("merged.profdata");
-    rustc().input("opaque.rs").run();
+    rustc().input("opaque.rs").codegen_source_order().run();
     rfs::create_dir_all(&path_prof_data_dir);
     rustc()
         .input("interesting.rs")
         .profile_generate(&path_prof_data_dir)
         .opt()
         .codegen_units(1)
+        .codegen_source_order()
+        .run();
+    rustc()
+        .input("main.rs")
+        .profile_generate(&path_prof_data_dir)
+        .opt()
+        .codegen_source_order()
         .run();
-    rustc().input("main.rs").profile_generate(&path_prof_data_dir).opt().run();
     run_with_args("main", &["aaaaaaaaaaaa2bbbbbbbbbbbb2bbbbbbbbbbbbbbbbcc"]);
     llvm_profdata().merge().output(&path_merged_profdata).input(path_prof_data_dir).run();
     rustc()
@@ -34,6 +40,7 @@ fn main() {
         .opt()
         .codegen_units(1)
         .emit("llvm-ir")
+        .codegen_source_order()
         .run();
     llvm_filecheck()
         .patterns("filecheck-patterns.txt")

tests/run-make/pgo-indirect-call-promotion/rmake.rs

@@ -14,11 +14,17 @@ use run_make_support::{llvm_filecheck, llvm_profdata, rfs, run, rustc};
 
 fn main() {
     // We don't compile `opaque` with either optimizations or instrumentation.
-    rustc().input("opaque.rs").run();
+    rustc().input("opaque.rs").codegen_source_order().run();
     // Compile the test program with instrumentation
     rfs::create_dir("prof_data_dir");
-    rustc().input("interesting.rs").profile_generate("prof_data_dir").opt().codegen_units(1).run();
-    rustc().input("main.rs").profile_generate("prof_data_dir").opt().run();
+    rustc()
+        .input("interesting.rs")
+        .profile_generate("prof_data_dir")
+        .opt()
+        .codegen_units(1)
+        .codegen_source_order()
+        .run();
+    rustc().input("main.rs").profile_generate("prof_data_dir").opt().codegen_source_order().run();
     // The argument below generates to the expected branch weights
     run("main");
     llvm_profdata().merge().output("prof_data_dir/merged.profdata").input("prof_data_dir").run();
@@ -28,6 +34,7 @@ fn main() {
         .opt()
         .codegen_units(1)
         .emit("llvm-ir")
+        .codegen_source_order()
         .run();
     llvm_filecheck()
         .patterns("filecheck-patterns.txt")

tests/run-make/pgo-use/rmake.rs

@@ -22,6 +22,7 @@ fn main() {
         .opt_level("2")
         .codegen_units(1)
         .arg("-Cllvm-args=-disable-preinline")
+        .codegen_source_order()
         .profile_generate(cwd())
         .input("main.rs")
         .run();
@@ -40,6 +41,7 @@ fn main() {
         .arg("-Cllvm-args=-disable-preinline")
         .profile_use("merged.profdata")
         .emit("llvm-ir")
+        .codegen_source_order()
         .input("main.rs")
         .run();
     // Check that the generate IR contains some things that we expect.

tests/ui/intrinsics/bad-intrinsic-monomorphization.stderr

@@ -1,20 +1,20 @@
-error[E0511]: invalid monomorphization of `cttz` intrinsic: expected basic integer type, found `Foo`
-  --> $DIR/bad-intrinsic-monomorphization.rs:16:5
+error[E0511]: invalid monomorphization of `simd_add` intrinsic: expected SIMD input type, found non-SIMD `Foo`
+  --> $DIR/bad-intrinsic-monomorphization.rs:26:5
    |
-LL |     intrinsics::cttz(v)
-   |     ^^^^^^^^^^^^^^^^^^^
+LL |     intrinsics::simd::simd_add(a, b)
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error[E0511]: invalid monomorphization of `fadd_fast` intrinsic: expected basic float type, found `Foo`
   --> $DIR/bad-intrinsic-monomorphization.rs:21:5
    |
 LL |     intrinsics::fadd_fast(a, b)
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error[E0511]: invalid monomorphization of `simd_add` intrinsic: expected SIMD input type, found non-SIMD `Foo`
-  --> $DIR/bad-intrinsic-monomorphization.rs:26:5
+error[E0511]: invalid monomorphization of `cttz` intrinsic: expected basic integer type, found `Foo`
+  --> $DIR/bad-intrinsic-monomorphization.rs:16:5
    |
-LL |     intrinsics::simd::simd_add(a, b)
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     intrinsics::cttz(v)
+   |     ^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 3 previous errors