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

Revert "[Unity] Split DecomposeOpsForTraining into two steps" #16442

Merged
merged 1 commit into from
Jan 21, 2024

Conversation

tqchen
Copy link
Member

@tqchen tqchen commented Jan 21, 2024

Reverts #15954

to avoid use of regex for now

@Hzfengsy Hzfengsy merged commit b0b8746 into unity Jan 21, 2024
18 of 19 checks passed
@tqchen tqchen deleted the revert-15954-unity_decompose_ops branch January 21, 2024 20:42
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Jan 24, 2024
This is a reapplication of apache#15954,
after resolving the breakages that required reverting in
apache#16442.  The regex matching is now
implemented without the `#include <regex>` from the C++ stdlib, to
avoid ABI incompatibility with pytorch.

Prior to this commit, the `DecomposeOpsForTraining` transform directly
replaced `relax.nn.batch_norm` into more primitive relax operations.
This required the decomposed form of `relax.nn.batch_norm` to be
duplicated with `DecomposeOpsForInference`.  This commit refactors the
pass to occur in two steps, first to apply training-specific
mutations, and then to decompose.

Having a clear `DecomposeOps` pass also has a clear single location
for operator decomposition, which may be migrated into the operator
definition in the future, similar to `FLegalize`.
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Jan 24, 2024
This is a reapplication of apache#15954,
after resolving the breakages that required reverting in
apache#16442.  The regex matching is now
implemented without the `#include <regex>` from the C++ stdlib, to
avoid ABI incompatibility with pytorch.

Prior to this commit, the `DecomposeOpsForTraining` transform directly
replaced `relax.nn.batch_norm` into more primitive relax operations.
This required the decomposed form of `relax.nn.batch_norm` to be
duplicated with `DecomposeOpsForInference`.  This commit refactors the
pass to occur in two steps, first to apply training-specific
mutations, and then to decompose.

Having a clear `DecomposeOps` pass also has a clear single location
for operator decomposition, which may be migrated into the operator
definition in the future, similar to `FLegalize`.
Lunderberg added a commit that referenced this pull request Feb 6, 2024
* [Support] Add PackedFunc "tvm.support.regex_match"

This function should be used instead of `std::regex` within C++ call
sites, to avoid ABI incompatibilities with pytorch.

Currently, the pytorch wheels available through pip install use the
pre-C++11 ABI by setting `-DUSE_CXX11_ABI=0` [0]. If TVM were to user
the pre-C++11 ABI, this would cause breakages with dynamically-linked
LLVM environments.

Use of the `<regex>` header in TVM should be avoided, as its
implementation is not supported by gcc's dual ABI. This ABI
incompatibility results in runtime errors either when `std::regex` is
called from TVM, or when `std::regex` is called from pytorch,
depending on which library was loaded first.  This restriction can be
removed when a version of pytorch compiled using `-DUSE_CXX11_ABI=1`
is available from PyPI.

[0] pytorch/pytorch#51039

* [Redo][Unity] Split DecomposeOpsForTraining into two steps

This is a reapplication of #15954,
after resolving the breakages that required reverting in
#16442.  The regex matching is now
implemented without the `#include <regex>` from the C++ stdlib, to
avoid ABI incompatibility with pytorch.

Prior to this commit, the `DecomposeOpsForTraining` transform directly
replaced `relax.nn.batch_norm` into more primitive relax operations.
This required the decomposed form of `relax.nn.batch_norm` to be
duplicated with `DecomposeOpsForInference`.  This commit refactors the
pass to occur in two steps, first to apply training-specific
mutations, and then to decompose.

Having a clear `DecomposeOps` pass also has a clear single location
for operator decomposition, which may be migrated into the operator
definition in the future, similar to `FLegalize`.
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.

2 participants