Skip to content
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

Mutable state using evidence #248

Merged
merged 6 commits into from
Jul 11, 2023
Merged

Conversation

serkm
Copy link
Contributor

@serkm serkm commented Mar 16, 2023

Here is an alternative implementation for mutable state in the LLVM backend that should work with #244. It doesn't refer to regions by name and instead uses evidence for allocation and access.

This comes with the cost of not having direct pointers as references anymore. On the other hand it makes memory management a lot simpler.

@serkm
Copy link
Contributor Author

serkm commented Mar 16, 2023

@marzipankaiser I encountered an issue with boxed types while implementing this.

var x = 0
x = 1

The generated core tree for above statements looks like this:

List(Let(tmp124_139, Literal(0, Data(Int_98, Nil)))),
State(
  x_116,
  ValueVar(tmp124_139, Data(Int_98, Nil)),
  this_127,
  App(
    Member(
      BlockVar(
        x_116,
        Interface(Ref_118, List(Data(BoxedInt_82, Nil))),
        Set(this)
      ),
      put_140,
      Function(
        Nil,
        Nil,
        List(Data(Int_98, Nil)),
        Nil,
        Data(Unit_119, Nil)
      )
    ),
    Nil,
    List(Literal(1, Data(Int_98, Nil))),
    Nil
  )
)

The var declaration uses Int_98 and the access uses BoxedInt_82. This lead to usage of the wrong arena, because Ints and Objects are allocated in separate arenas.
As a workaround I hardcoded the boxed types for now (see 9bb9c1a)

@marzipankaiser
Copy link
Contributor

@marzipankaiser I encountered an issue with boxed types while implementing this.

var x = 0
x = 1

The generated core tree for above statements looks like this:

List(Let(tmp124_139, Literal(0, Data(Int_98, Nil)))),
State(
  x_116,
  ValueVar(tmp124_139, Data(Int_98, Nil)),
  this_127,
  App(
    Member(
      BlockVar(
        x_116,
        Interface(Ref_118, List(Data(BoxedInt_82, Nil))),
        Set(this)
      ),
      put_140,
      Function(
        Nil,
        Nil,
        List(Data(Int_98, Nil)),
        Nil,
        Data(Unit_119, Nil)
      )
    ),
    Nil,
    List(Literal(1, Data(Int_98, Nil))),
    Nil
  )
)

The var declaration uses Int_98 and the access uses BoxedInt_82. This lead to usage of the wrong arena, because Ints and Objects are allocated in separate arenas. As a workaround I hardcoded the boxed types for now (see 9bb9c1a)

Ah, sorry, I completely forgot about answering here. In principle, it should be easy enough to fix this in PolymorphismBoxing, depending on what's the correct behavior, i.e.: should Ref[Int] be special-cased and allowed? Or should state always be boxed?

@serkm serkm force-pushed the state-with-evidence branch 2 times, most recently from 2232d74 to e20d2b1 Compare May 28, 2023 11:28
@serkm
Copy link
Contributor Author

serkm commented May 28, 2023

I special-cased references for now.
@phischu I also added support for nested handlers here.

@phischu phischu self-requested a review June 1, 2023 11:21
Copy link
Collaborator

@phischu phischu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good job!

Comment on lines +67 to +74
case Allocate(name, init, ev, rest) =>
toDoc(name.tpe) <+> name <> "<" <> ev <> ">" <+> "=" <+> init <> ";" <> line <> toDoc(rest)

case Load(name, ref, rest) =>
name <+> "=" <+> "*" <> ref <> ";" <> line <> toDoc(rest)
case Load(name, ref, ev, rest) =>
name <+> "=" <+> "*" <> ref <> "<" <> ev <> ">" <> ";" <> line <> toDoc(rest)

case Store(ref, value, rest) =>
"*" <> ref <+> "=" <+> value <> ";" <> line <> toDoc(rest)
case Store(ref, value, ev, rest) =>
"*" <> ref <> "<" <> ev <> ">" <+> "=" <+> value <> ";" <> line <> toDoc(rest)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have used keywords like alloc load store but this is fine.

; | Next | Size | Payload ... |
; +---------------+--------------+
%State = type ptr
%Region = type [ 3 x %Mem ]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite like this solution and would prefer storing a sharer and an eraser with each object. But ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would become obsolete if we switch to monomorphic regions.

%regionval = load %RegionVal, %Region %region
%oldstate = extractvalue %RegionVal %regionval, 0
%oldsize = extractvalue %RegionVal %regionval, 1
realloc:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later we could perhaps move this reallocation into its own function.

}

define ptr @getPtr(%Ref %ref, i64 %idx, i64 %evidence) alwaysinline {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The %idx is for the 3 different kinds of values, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, perhaps it should be renamed to %kind or something similar.

ret %Stk %newstk
%objectsbase = extractvalue %Region %region, 1, 1
%objectssp = extractvalue %Region %region, 1, 0
call void @forEachObject(%Base %objectsbase, %Sp %objectssp, %Eraser @sharePositive)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Higher-order functions in LLVM.

store %Stk %nextnew, ptr %newstkrest
br label %loop

closecycle:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot why the meta stack is cyclic...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The meta-stack isn't cyclic but first-class stacks are.
This representation saves us a few operations on PushStack/PopStack, because we only have to swap the heads of the meta-stack and the first-class stack.

@phischu phischu merged commit c785207 into effekt-lang:master Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants