Skip to content

incr.comp.: Verify stability of incr. comp. hashes and clean up various other things. #45867

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Nov 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
70f9a0b
incr.comp.: Allow for forcing input nodes lazily.
michaelwoerister Nov 7, 2017
6674773
incr.comp.: Mark more input nodes as inputs.
michaelwoerister Nov 7, 2017
a174951
incr.comp.: Make assertion in try_mark_green() more targeted.
michaelwoerister Nov 7, 2017
102eaa5
incr.comp.: Always require Session when decoding Spans (as to avoid s…
michaelwoerister Nov 7, 2017
faad4ea
incr.comp.: Sort exported symbols list in order to achieve stable inc…
michaelwoerister Nov 7, 2017
81fd279
incr.comp.: Don't filter out StmtDecls from hir::Block during hashing…
michaelwoerister Nov 7, 2017
3c6f620
incr.comp.: Add -Zincremental-verify-ich, which allows to perform a c…
michaelwoerister Nov 7, 2017
a1364cd
incr.comp.: Acknowledge the fact that shift operations can panic at r…
michaelwoerister Nov 7, 2017
616b455
incr.comp.: Make DefSpan an input dep-node so it is not affected by t…
michaelwoerister Nov 8, 2017
fde0ca5
incr.comp.: Add some missing reads in HIR map.
michaelwoerister Nov 8, 2017
95b0849
incr.comp.: Adapt nested_items test to new HIR hashing rules.
michaelwoerister Nov 8, 2017
13bc7ad
incr.comp.: Always verify incr. comp. hashes when running incremental…
michaelwoerister Nov 8, 2017
702ce8f
incr.comp.: Remove outdated comment about TraitSelect dep-node.
michaelwoerister Nov 8, 2017
d948af1
incr.comp.: Remove unused DepKind::WorkProduct.
michaelwoerister Nov 8, 2017
d01b89b
incr.comp.: Provide session to some more decoding contexts.
michaelwoerister Nov 8, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 26 additions & 56 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,10 +459,6 @@ define_dep_nodes!( <'tcx>
// Represents metadata from an extern crate.
[input] CrateMetadata(CrateNum),

// Represents some artifact that we save to disk. Note that these
// do not have a def-id as part of their identifier.
[] WorkProduct(WorkProductId),

// Represents different phases in the compiler.
[] RegionScopeTree(DefId),
[eval_always] Coherence,
Expand Down Expand Up @@ -537,38 +533,19 @@ define_dep_nodes!( <'tcx>
// The set of impls for a given trait.
[] TraitImpls(DefId),

[] AllLocalTraitImpls,

// Trait selection cache is a little funny. Given a trait
// reference like `Foo: SomeTrait<Bar>`, there could be
// arbitrarily many def-ids to map on in there (e.g., `Foo`,
// `SomeTrait`, `Bar`). We could have a vector of them, but it
// requires heap-allocation, and trait sel in general can be a
// surprisingly hot path. So instead we pick two def-ids: the
// trait def-id, and the first def-id in the input types. If there
// is no def-id in the input types, then we use the trait def-id
// again. So for example:
//
// - `i32: Clone` -> `TraitSelect { trait_def_id: Clone, self_def_id: Clone }`
// - `u32: Clone` -> `TraitSelect { trait_def_id: Clone, self_def_id: Clone }`
// - `Clone: Clone` -> `TraitSelect { trait_def_id: Clone, self_def_id: Clone }`
// - `Vec<i32>: Clone` -> `TraitSelect { trait_def_id: Clone, self_def_id: Vec }`
// - `String: Clone` -> `TraitSelect { trait_def_id: Clone, self_def_id: String }`
// - `Foo: Trait<Bar>` -> `TraitSelect { trait_def_id: Trait, self_def_id: Foo }`
// - `Foo: Trait<i32>` -> `TraitSelect { trait_def_id: Trait, self_def_id: Foo }`
// - `(Foo, Bar): Trait` -> `TraitSelect { trait_def_id: Trait, self_def_id: Foo }`
// - `i32: Trait<Foo>` -> `TraitSelect { trait_def_id: Trait, self_def_id: Foo }`
//
// You can see that we map many trait refs to the same
// trait-select node. This is not a problem, it just means
// imprecision in our dep-graph tracking. The important thing is
// that for any given trait-ref, we always map to the **same**
// trait-select node.
[input] AllLocalTraitImpls,

[anon] TraitSelect,

[] ParamEnv(DefId),
[] DescribeDef(DefId),
[] DefSpan(DefId),

// FIXME(mw): DefSpans are not really inputs since they are derived from
// HIR. But at the moment HIR hashing still contains some hacks that allow
// to make type debuginfo to be source location independent. Declaring
// DefSpan an input makes sure that changes to these are always detected
// regardless of HIR hashing.
[input] DefSpan(DefId),
[] LookupStability(DefId),
[] LookupDeprecationEntry(DefId),
[] ItemBodyNestedBodies(DefId),
Expand All @@ -588,7 +565,7 @@ define_dep_nodes!( <'tcx>
[eval_always] LintLevels,
[] Specializes { impl1: DefId, impl2: DefId },
[input] InScopeTraits(DefIndex),
[] ModuleExports(DefId),
[input] ModuleExports(DefId),
[] IsSanitizerRuntime(CrateNum),
[] IsProfilerRuntime(CrateNum),
[] GetPanicStrategy(CrateNum),
Expand All @@ -598,37 +575,37 @@ define_dep_nodes!( <'tcx>
[] NativeLibraries(CrateNum),
[] PluginRegistrarFn(CrateNum),
[] DeriveRegistrarFn(CrateNum),
[] CrateDisambiguator(CrateNum),
[] CrateHash(CrateNum),
[] OriginalCrateName(CrateNum),
[input] CrateDisambiguator(CrateNum),
[input] CrateHash(CrateNum),
[input] OriginalCrateName(CrateNum),

[] ImplementationsOfTrait { krate: CrateNum, trait_id: DefId },
[] AllTraitImplementations(CrateNum),

[] IsDllimportForeignItem(DefId),
[] IsStaticallyIncludedForeignItem(DefId),
[] NativeLibraryKind(DefId),
[] LinkArgs,
[input] LinkArgs,

[] NamedRegion(DefIndex),
[] IsLateBound(DefIndex),
[] ObjectLifetimeDefaults(DefIndex),
[input] NamedRegion(DefIndex),
[input] IsLateBound(DefIndex),
[input] ObjectLifetimeDefaults(DefIndex),

[] Visibility(DefId),
[] DepKind(CrateNum),
[] CrateName(CrateNum),
[input] CrateName(CrateNum),
[] ItemChildren(DefId),
[] ExternModStmtCnum(DefId),
[] GetLangItems,
[input] GetLangItems,
[] DefinedLangItems(CrateNum),
[] MissingLangItems(CrateNum),
[] ExternConstBody(DefId),
[] VisibleParentMap,
[] MissingExternCrateItem(CrateNum),
[] UsedCrateSource(CrateNum),
[] PostorderCnums,
[] HasCloneClosures(CrateNum),
[] HasCopyClosures(CrateNum),
[input] PostorderCnums,
[input] HasCloneClosures(CrateNum),
[input] HasCopyClosures(CrateNum),

// This query is not expected to have inputs -- as a result, it's
// not a good candidate for "replay" because it's essentially a
Expand All @@ -638,19 +615,19 @@ define_dep_nodes!( <'tcx>
// may save a bit of time.
[anon] EraseRegionsTy { ty: Ty<'tcx> },

[] Freevars(DefId),
[] MaybeUnusedTraitImport(DefId),
[input] Freevars(DefId),
[input] MaybeUnusedTraitImport(DefId),
[] MaybeUnusedExternCrates,
[] StabilityIndex,
[] AllCrateNums,
[input] AllCrateNums,
[] ExportedSymbols(CrateNum),
[eval_always] CollectAndPartitionTranslationItems,
[] ExportName(DefId),
[] ContainsExternIndicator(DefId),
[] IsTranslatedFunction(DefId),
[] CodegenUnit(InternedString),
[] CompileCodegenUnit(InternedString),
[] OutputFilenames,
[input] OutputFilenames,
[anon] NormalizeTy,
// We use this for most things when incr. comp. is turned off.
[] Null,
Expand Down Expand Up @@ -800,13 +777,6 @@ impl WorkProductId {
hash: fingerprint
}
}

pub fn to_dep_node(self) -> DepNode {
DepNode {
kind: DepKind::WorkProduct,
hash: self.hash,
}
}
}

impl_stable_hash_for!(struct ::dep_graph::WorkProductId {
Expand Down
134 changes: 83 additions & 51 deletions src/librustc/dep_graph/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,60 +506,67 @@ impl DepGraph {
return None
}
None => {
if dep_dep_node.kind.is_input() {
// This input does not exist anymore.
debug_assert!(dep_dep_node.extract_def_id(tcx).is_none(),
"Encountered input {:?} without color",
dep_dep_node);
debug!("try_mark_green({:?}) - END - dependency {:?} \
was deleted input", dep_node, dep_dep_node);
return None;
// We don't know the state of this dependency. If it isn't
// an input node, let's try to mark it green recursively.
if !dep_dep_node.kind.is_input() {
debug!("try_mark_green({:?}) --- state of dependency {:?} \
is unknown, trying to mark it green", dep_node,
dep_dep_node);

if let Some(node_index) = self.try_mark_green(tcx, dep_dep_node) {
debug!("try_mark_green({:?}) --- managed to MARK \
dependency {:?} as green", dep_node, dep_dep_node);
current_deps.push(node_index);
continue;
}
} else if cfg!(debug_assertions) {
match dep_dep_node.kind {
DepKind::Hir |
DepKind::HirBody |
DepKind::CrateMetadata => {
assert!(dep_dep_node.extract_def_id(tcx).is_none(),
"Input {:?} should have been pre-allocated but wasn't.",
dep_dep_node);
}
_ => {
// For other kinds of inputs it's OK to be
// forced.
}
}
}

debug!("try_mark_green({:?}) --- state of dependency {:?} \
is unknown, trying to mark it green", dep_node,
dep_dep_node);

// We don't know the state of this dependency. Let's try to
// mark it green.
if let Some(node_index) = self.try_mark_green(tcx, dep_dep_node) {
debug!("try_mark_green({:?}) --- managed to MARK \
dependency {:?} as green", dep_node, dep_dep_node);
current_deps.push(node_index);
} else {
// We failed to mark it green, so we try to force the query.
debug!("try_mark_green({:?}) --- trying to force \
dependency {:?}", dep_node, dep_dep_node);
if ::ty::maps::force_from_dep_node(tcx, dep_dep_node) {
let dep_dep_node_color = data.colors
.borrow()
.get(dep_dep_node)
.cloned();
match dep_dep_node_color {
Some(DepNodeColor::Green(node_index)) => {
debug!("try_mark_green({:?}) --- managed to \
FORCE dependency {:?} to green",
dep_node, dep_dep_node);
current_deps.push(node_index);
}
Some(DepNodeColor::Red) => {
debug!("try_mark_green({:?}) - END - \
dependency {:?} was red after forcing",
dep_node,
dep_dep_node);
return None
}
None => {
bug!("try_mark_green() - Forcing the DepNode \
should have set its color")
}
// We failed to mark it green, so we try to force the query.
debug!("try_mark_green({:?}) --- trying to force \
dependency {:?}", dep_node, dep_dep_node);
if ::ty::maps::force_from_dep_node(tcx, dep_dep_node) {
let dep_dep_node_color = data.colors
.borrow()
.get(dep_dep_node)
.cloned();
match dep_dep_node_color {
Some(DepNodeColor::Green(node_index)) => {
debug!("try_mark_green({:?}) --- managed to \
FORCE dependency {:?} to green",
dep_node, dep_dep_node);
current_deps.push(node_index);
}
Some(DepNodeColor::Red) => {
debug!("try_mark_green({:?}) - END - \
dependency {:?} was red after forcing",
dep_node,
dep_dep_node);
return None
}
None => {
bug!("try_mark_green() - Forcing the DepNode \
should have set its color")
}
} else {
// The DepNode could not be forced.
debug!("try_mark_green({:?}) - END - dependency {:?} \
could not be forced", dep_node, dep_dep_node);
return None
}
} else {
// The DepNode could not be forced.
debug!("try_mark_green({:?}) - END - dependency {:?} \
could not be forced", dep_node, dep_dep_node);
return None
}
}
}
Expand Down Expand Up @@ -772,7 +779,30 @@ impl CurrentDepGraph {
read_set: _,
reads
} = popped_node {
debug_assert_eq!(node, key);
assert_eq!(node, key);

// If this is an input node, we expect that it either has no
// dependencies, or that it just depends on DepKind::CrateMetadata
// or DepKind::Krate. This happens for some "thin wrapper queries"
// like `crate_disambiguator` which sometimes have zero deps (for
// when called for LOCAL_CRATE) or they depend on a CrateMetadata
// node.
if cfg!(debug_assertions) {
if node.kind.is_input() && reads.len() > 0 &&
// FIXME(mw): Special case for DefSpan until Spans are handled
// better in general.
node.kind != DepKind::DefSpan &&
reads.iter().any(|&i| {
!(self.nodes[i].kind == DepKind::CrateMetadata ||
self.nodes[i].kind == DepKind::Krate)
})
{
bug!("Input node {:?} with unexpected reads: {:?}",
node,
reads.iter().map(|&i| self.nodes[i]).collect::<Vec<_>>())
}
}

self.alloc_node(node, reads)
} else {
bug!("pop_task() - Expected regular task to be popped")
Expand All @@ -793,6 +823,8 @@ impl CurrentDepGraph {
read_set: _,
reads
} = popped_node {
debug_assert!(!kind.is_input());

let mut fingerprint = self.anon_id_seed;
let mut hasher = StableHasher::new();

Expand Down
5 changes: 4 additions & 1 deletion src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,10 @@ impl<'a> LoweringContext<'a> {
return n;
}
assert!(!def_id.is_local());
let n = self.cstore.item_generics_cloned_untracked(def_id).regions.len();
let n = self.cstore
.item_generics_cloned_untracked(def_id, self.sess)
.regions
.len();
self.type_def_lifetime_params.insert(def_id, n);
n
});
Expand Down
12 changes: 12 additions & 0 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,12 @@ impl<'hir> Map<'hir> {
/// if the node is a body owner, otherwise returns `None`.
pub fn maybe_body_owned_by(&self, id: NodeId) -> Option<BodyId> {
if let Some(entry) = self.find_entry(id) {
if self.dep_graph.is_fully_enabled() {
let hir_id_owner = self.node_to_hir_id(id).owner;
let def_path_hash = self.definitions.def_path_hash(hir_id_owner);
self.dep_graph.read(def_path_hash.to_dep_node(DepKind::HirBody));
}

if let Some(body_id) = entry.associated_body() {
// For item-like things and closures, the associated
// body has its own distinct id, and that is returned
Expand Down Expand Up @@ -530,6 +536,12 @@ impl<'hir> Map<'hir> {
/// from a node to the root of the ast (unless you get the same id back here
/// that can happen if the id is not in the map itself or is just weird).
pub fn get_parent_node(&self, id: NodeId) -> NodeId {
if self.dep_graph.is_fully_enabled() {
let hir_id_owner = self.node_to_hir_id(id).owner;
let def_path_hash = self.definitions.def_path_hash(hir_id_owner);
self.dep_graph.read(def_path_hash.to_dep_node(DepKind::HirBody));
}

self.find_entry(id).and_then(|x| x.parent_node()).unwrap_or(id)
}

Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ich/hcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ impl<'gcx> StableHashingContext<'gcx> {
match binop {
hir::BiAdd |
hir::BiSub |
hir::BiShl |
hir::BiShr |
hir::BiMul => self.overflow_checks_enabled,

hir::BiDiv |
Expand All @@ -237,8 +239,6 @@ impl<'gcx> StableHashingContext<'gcx> {
hir::BiBitXor |
hir::BiBitAnd |
hir::BiBitOr |
hir::BiShl |
hir::BiShr |
hir::BiEq |
hir::BiLt |
hir::BiLe |
Expand Down
Loading