FEAT-#128: oneDNN: Implement compilation and execution #129
Conversation
AndreyPavlenko
force-pushed
the
dnnl-compile
branch
3 times, most recently
from
b90bbae to
3464718
Compare
|
@AndreyPavlenko #97 is merged, could you please rebase so only relevant changes would be shown in diffs |
AndreyPavlenko
force-pushed
the
dnnl-compile
branch
from
3464718 to
2022935
Compare
Done. |
| const std::string_view &json) | ||
| : inputIds(), outputIds(), jmod() { | ||
| auto mod = JsonParser::parse(context, json, inputIds, outputIds, strides); | ||
| auto jmod = gc::JitModule::create(mod); |
There was a problem hiding this comment.
jmod is the name of private class member. It would be better to add prefix _ to class member just as JsonParser to avoid confusion.
There was a problem hiding this comment.
I think, we need a code style convention. Currently, different styles are used in different parts of the project. Regarding the _ prefix, there are opinions that it's reserved in C++, but I haven't found any evidence of this.
src/dnnl/dnnl_graph_compiler.cpp
Outdated
| std::make_pair(&outputIds, outputs)}) { | ||
| auto ids = pair.first; | ||
| auto tensors = pair.second; | ||
| for (auto id : *ids) { |
There was a problem hiding this comment.
I think it's not necessary to use O(N^2) algorithms here, and since this is execution API, we want to have less check and thus less overhead. please simplify this.
There was a problem hiding this comment.
The jmod will respect the argument order set from mod. It means if we use same argument order for kernel compilation, then we don't need to reorder it during kernel execution.
There was a problem hiding this comment.
We use the same order, but the arguments are passed from oneDNN and I'm not sure if the order is always the same as in mod.
There was a problem hiding this comment.
Now it's O(N), if the arguments order matches.
AndreyPavlenko
force-pushed
the
dnnl-compile
branch
2 times, most recently
from
20f1f55 to
37c7247
Compare
AndreyPavlenko
force-pushed
the
dnnl-compile
branch
2 times, most recently
from
f0e6fdd to
0726837
Compare
kurapov-peter
left a comment
kurapov-peter
left a comment
There was a problem hiding this comment.
Looks good, minor comments.
AndreyPavlenko
force-pushed
the
dnnl-compile
branch
from
f2a5f90 to
8b8555d
Compare
6 participants
Depends on #97