-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[INFA][IR] Build and Evolve Low-level IR. Remove HalideIR dep. #3533
Conversation
*/ | ||
static Range make_by_min_extent(Expr min, Expr extent); | ||
// declare range. | ||
TVM_DEFINE_NODE_REF_METHODS(Range, NodeRef, RangeNode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we unify the RELAY and TVM macros into one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, likely we will need to use these macros instead in relay and remove relay specific macros.
}; | ||
|
||
/*! \brief Create a vector where all the elements are value. */ | ||
class Broadcast : public ExprNode { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be out of the scope for now, but I think we should use the XNode
and X
for the Node and NodeRef subclasses, it might be a good time to stop using raw pointers in the functor as well
static constexpr const char* shift_right = "shift_right"; | ||
static constexpr const char* popcount = "popcount"; | ||
static constexpr const char* likely = "likely"; | ||
static constexpr const char* glsl_texture_store = "glsl_texture_store"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we use this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do have a semi-functioning webgl backend that uses this intrinsic
}); | ||
|
||
TVM_STATIC_IR_FUNCTOR(IRPrinter, vtable) | ||
.set_dispatch<Free>([](const Free* op, IRPrinter* p) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should unify both printers and start using a layout algorithm, we already have some lines which are incredibly ugly in current printing strategy, probably out of scope of this PR though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a lot of work in the printer. I am still trying to wrap up roundtripping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a couple comments, but otherwise looks good
} | ||
|
||
Stmt ProducerConsumer::make(FunctionRef func, bool is_producer, Stmt body) { | ||
CHECK(body.defined()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#define CHECK_DEFINE(x) CHECK(x.defined()) << #x << " is not defined";
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message of CHECK will automatically display the content, which is self-explained
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will be CHECK: body.defined() failed
}); | ||
|
||
TVM_STATIC_IR_FUNCTOR(IRPrinter, vtable) | ||
.set_dispatch<Free>([](const Free* op, IRPrinter* p) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a lot of work in the printer. I am still trying to wrap up roundtripping.
Co-Authored-By: Jared Roesch <roeschinc@gmail.com>
Co-Authored-By: Jared Roesch <roeschinc@gmail.com>
Thanks @jroesch @MarisaKirisame , this PR is now merged |
Is this an effort to replace the Halide IR with something else? Is there a discussion somewhere? |
@kparzysz-quic see RFC #3474 |
…e#3533) * [INFA][IR] Build and Evolve Low-level IR. Remove dep from HalideIR. * Update include/tvm/node/ir_functor.h Co-Authored-By: Jared Roesch <roeschinc@gmail.com> * Update include/tvm/node/ir_functor.h Co-Authored-By: Jared Roesch <roeschinc@gmail.com>
…e#3533) * [INFA][IR] Build and Evolve Low-level IR. Remove dep from HalideIR. * Update include/tvm/node/ir_functor.h Co-Authored-By: Jared Roesch <roeschinc@gmail.com> * Update include/tvm/node/ir_functor.h Co-Authored-By: Jared Roesch <roeschinc@gmail.com>
This parameter is nullable for cases where the else block isn't present. Previously, it was represented as a `Stmt` holding `nullptr`, because `IfThenElse` (apache#3533) predates the `Optional` utility (apache#5314). This commit updates to use `Optional<Stmt>` instead, and updates all usages of `else_case`.
This parameter is nullable for cases where the else block isn't present. Previously, it was represented as a `Stmt` holding `nullptr`, because `IfThenElse` (apache#3533) predates the `Optional` utility (apache#5314). This commit updates to use `Optional<Stmt>` instead, and updates all usages of `else_case`.
This parameter is nullable for cases where the else block isn't present. Previously, it was represented as a `Stmt` holding `nullptr`, because `IfThenElse` (apache#3533) predates the `Optional` utility (apache#5314). This commit updates to use `Optional<Stmt>` instead, and updates all usages of `else_case`.
#3474