Skip to content

Conversation

@wweic
Copy link
Contributor

@wweic wweic commented Jun 6, 2019

  1. Fix prelude compilation in VM. We need to optimize functions in prelude the same as entry func.
  2. Fix ConstructorValueNode's tag and add optional constructor tag.

cc: @jroesch @MarisaKirisame @icemelon9 @zhiics @slyubomirsky @tqchen

@slyubomirsky
Copy link
Contributor

slyubomirsky commented Jun 6, 2019

What will be the mechanism for assigning a tag to a constructor? (Forgive me if I've missed it)

@wweic
Copy link
Contributor Author

wweic commented Jun 7, 2019

What will be the mechanism for assigning a tag to a constructor? (Forgive me if I've missed it)

@slyubomirsky a brute force approach would be to search through all constructors and compare tag to implement tag -> constructor mapping. Right now there is no data structure specifically for this.

I added a new function in prelude to map tag to constructor.

Copy link
Member

@zhiics zhiics left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@icemelon icemelon left a comment

Choose a reason for hiding this comment

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

LGTM

@jroesch
Copy link
Member

jroesch commented Jun 11, 2019

I'm not sure if we should use a global constructor mapping upon further thought. Many implementations of tags only number the constructors for a given type. I think it might make more sense to add an optional debugging field if we want to track the constructors.

@wweic
Copy link
Contributor Author

wweic commented Jun 11, 2019

@jroesch agree it's better to have local tag scope for each type. I'll change it back. Do you mean add debug constructor field to ContrustorValueNode?

@wweic wweic force-pushed the adt-value branch 2 times, most recently from a25ed43 to 878da97 Compare June 11, 2019 02:01
@wweic
Copy link
Contributor Author

wweic commented Jun 11, 2019

@jroesch @slyubomirsky Addressed comments. Please take another look. Thanks.

@slyubomirsky
Copy link
Contributor

LGTM, thanks

@apivovarov
Copy link
Contributor

@slyubomirsky Can you approve it in addition to saying LGTM?

@MarisaKirisame @icemelon9 @zhiics @slyubomirsky Can it be merged ASAP? I'm constantly getting test_sum_loop Segmentation fault errors #3352

@jroesch jroesch merged commit 713fc73 into apache:master Jun 13, 2019
wweic added a commit to wweic/tvm that referenced this pull request Jun 26, 2019
* Support export ADT value in Python

* Cache original functions

* Cleanup

* Cleanup
wweic added a commit to neo-ai/tvm that referenced this pull request Jun 27, 2019
* Support export ADT value in Python

* Cache original functions

* Cleanup

* Cleanup
@wweic wweic deleted the adt-value branch July 18, 2019 19:57
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.

7 participants