Skip to content

Conversation

@Lunderberg
Copy link
Contributor

Using the constructor applies all initialization steps and error-checking, where using CopyOnWrite() does not. This function is used as part of the legalization of relax.op.layout_tranform, which relies on the annotations produced in the PrimFunc constructor.

Copy link
Contributor

@slyubomirsky slyubomirsky left a comment

Choose a reason for hiding this comment

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

Good to see safer interfaces and checks for consistency.

@Lunderberg
Copy link
Contributor Author

CI failure is for tests/python/relay/test_auto_scheduler_tuning.py::test_tuning_cuda. It looks like a flaky test, which occurs whenever the randomized tuning fails to find any valid schedules. I can reproduce the failures on main, and have submitted #16837 to disable the test. After that PR lands, I'll re-run CI for this PR.

Using the constructor applies all initialization steps and
error-checking, where using `CopyOnWrite()` does not.  This function
is used as part of the legalization of `relax.op.layout_tranform`,
which relies on the annotations produced in the `PrimFunc`
constructor.
@Lunderberg Lunderberg force-pushed the tir_layout_transform_with_primfunc_constructor branch from c53b046 to 6312499 Compare April 5, 2024 12:24
@Lunderberg
Copy link
Contributor Author

Rebased this PR onto main as #16837 has now landed.

@quic-sanirudh quic-sanirudh merged commit 81a8506 into apache:main Apr 7, 2024
@Lunderberg Lunderberg deleted the tir_layout_transform_with_primfunc_constructor branch April 9, 2024 15:23
@Lunderberg
Copy link
Contributor Author

In the future, I'm wondering if the CopyOnWrite functionality should be extended to perform this type of initialization. Currently, regardless of what invariants are established by an object's constructor, they can be violated when overwritten with CopyOnWrite.

We could have CopyOnWrite return a proxy object, rather than T::ContainerType*. The proxy object would provide operator->, so the same obj.CopyOnWrite()->foo = new_foo would still function as intended. However, on destruction, the proxy object would call some T::Validate method. That way, we get the usability of direct access and CoW to mutating a single field, but we can still establish invariants for classes.

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