Skip to content

Commit

Permalink
Add lifetime to Resolved so it is non-trivial to persist it
Browse files Browse the repository at this point in the history
  • Loading branch information
aakoshh committed Nov 19, 2024
1 parent 72ad1db commit a203cf1
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 23 deletions.
4 changes: 2 additions & 2 deletions compiler/noirc_evaluator/src/brillig/brillig_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ impl IsResolved for FinalResolved {}

type FinalValueId = ValueId<FinalResolved>;

impl From<ResolvedValueId> for FinalValueId {
fn from(value: ResolvedValueId) -> Self {
impl From<ResolvedValueId<'_>> for FinalValueId {
fn from(value: ResolvedValueId<'_>) -> Self {
ValueId::new(value.raw())
}
}
Expand Down
11 changes: 7 additions & 4 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,13 +248,16 @@ impl DataFlowGraph {
/// `ValueId`, this function will return the `ValueId` from which the substitution was taken.
/// If `original_value_id`'s underlying `Value` has not been substituted, the same `ValueId`
/// is returned.
pub(crate) fn resolve<R: Resolution>(&self, original_value_id: ValueId<R>) -> ResolvedValueId {
pub(crate) fn resolve<'a, R: Resolution>(
&'a self,
original_value_id: ValueId<R>,
) -> ResolvedValueId<'a> {
if R::is_resolved() {
return ResolvedValueId::new(original_value_id.raw());
return ValueId::new(original_value_id.raw());
}
match self.replaced_value_ids.get(original_value_id.as_ref()) {
Some(id) => self.resolve(*id),
None => ResolvedValueId::new(original_value_id.raw()),
None => ValueId::new(original_value_id.raw()),
}
}

Expand Down Expand Up @@ -350,7 +353,7 @@ impl DataFlowGraph {

/// Resolve and get a value by ID
fn resolve_value<R: Resolution>(&self, original_value_id: ValueId<R>) -> &Value {
let id = self.resolve(original_value_id.unresolved()).raw();
let id = self.resolve(original_value_id).raw();
&self.values[id]
}

Expand Down
8 changes: 4 additions & 4 deletions compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ impl<'f> FunctionInserter<'f> {
/// Resolves a ValueId to its new, updated value.
/// If there is no updated value for this id, this returns the same
/// ValueId that was passed in.
pub(crate) fn resolve(&mut self, value: ValueId) -> ResolvedValueId {
pub(crate) fn resolve(&mut self, value: ValueId) -> ResolvedValueId<'f> {
let value = self.function.dfg.resolve(value);
match self.values.get(&value.raw()) {
Some(value) => self.resolve(*value),
None => value,
None => ValueId::new(value.raw()),
}
}

Expand Down Expand Up @@ -119,7 +119,7 @@ impl<'f> FunctionInserter<'f> {
call_stack: CallStack,
) -> InsertInstructionResult {
let results = self.function.dfg.instruction_results(id);
let results = vecmap(results, |id| self.function.dfg.resolve(*id));
let results = vecmap(results, |id| self.function.dfg.resolve(*id).detach());

let ctrl_typevars = instruction
.requires_ctrl_typevars()
Expand Down Expand Up @@ -209,7 +209,7 @@ impl<'f> FunctionInserter<'f> {
/// ValueId (from the source_function) and its new ValueId in the destination function.
pub(crate) fn insert_new_instruction_results(
values: &mut HashMap<RawValueId, ValueId>,
old_results: &[ResolvedValueId],
old_results: &[ResolvedValueId<'f>],
new_results: &InsertInstructionResult,
) {
assert_eq!(old_results.len(), new_results.len());
Expand Down
32 changes: 23 additions & 9 deletions compiler/noirc_evaluator/src/ssa/ir/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,24 @@ use super::{
#[derive(Debug, Clone, Serialize, Deserialize)]
pub(crate) enum Unresolved {}

/// Resolved marker; doesn't implement `Hash` so it can't be stored in maps.
/// Marker for resolved status.
///
/// Doesn't implement `Hash` so it can't be stored in maps.
/// It has a lifetime so it's not easy to store it in data structures,
/// where it could become stale. Instead we can implement module specific
/// variants when we can prove that persisting them is safe because the
/// IDs are not going to be changed between use.
///
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize /* PartialOrd, Ord, Hash */)]
pub(crate) enum Resolved {}
pub(crate) struct Resolved<'a> {
_marker: PhantomData<&'a ()>,
}

pub(crate) type ResolvedValueId<'a> = ValueId<Resolved<'a>>;

pub(crate) trait IsResolved {}

impl IsResolved for Resolved {}
impl<'a> IsResolved for Resolved<'a> {}

pub(crate) trait Resolution {
fn is_resolved() -> bool;
Expand All @@ -39,9 +50,6 @@ impl<R: IsResolved> Resolution for R {
}
}

/// A resolved value ID is something we can directly compare.
pub(crate) type ResolvedValueId = ValueId<Resolved>;

/// A raw value ID that can be used as a key in maps.
pub(crate) type RawValueId = Id<Value>;

Expand Down Expand Up @@ -79,8 +87,15 @@ impl ValueId<Unresolved> {
self.id == other.id
}
/// Promote an unresolved ID into a resolved one.
pub(crate) fn resolved(self) -> ValueId<Resolved> {
ValueId::new(Id::new(self.id.to_usize()))
pub(crate) fn resolved(self) -> ValueId<Resolved<'static>> {
ValueId::new(self.id)
}
}

impl<'a> ValueId<Resolved<'a>> {
/// Change the lifetime of a resolution.
pub(crate) fn detach<'b>(self) -> ValueId<Resolved<'b>> {
ValueId::new(self.id)
}
}

Expand Down Expand Up @@ -119,7 +134,6 @@ impl<R: IsResolved> From<ValueId<R>> for ValueId<Unresolved> {
value.unresolved()
}
}

impl From<Id<Value>> for ValueId<Unresolved> {
fn from(value: Id<Value>) -> Self {
ValueId::new(value)
Expand Down
4 changes: 1 addition & 3 deletions compiler/noirc_evaluator/src/ssa/opt/array_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,7 @@ impl Function {
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub(super) enum ContextResolved {}

//impl IsResolved for ContextResolved {}

impl From<ResolvedValueId> for ValueId<ContextResolved> {
impl From<ResolvedValueId<'_>> for ValueId<ContextResolved> {
fn from(value: ResolvedValueId) -> Self {
ValueId::new(value.raw())
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/opt/mem2reg/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub(super) enum ContextResolved {}

impl IsResolved for ContextResolved {}

impl From<ResolvedValueId> for ValueId<ContextResolved> {
impl From<ResolvedValueId<'_>> for ValueId<ContextResolved> {
fn from(value: ResolvedValueId) -> Self {
ValueId::new(value.raw())
}
Expand Down

0 comments on commit a203cf1

Please sign in to comment.