[QNN] Register a bunch of unary elementwise ops#10086
Conversation
AndrewZhaoLuo
requested review from
MarisaKirisame,
ZihengJiang,
anijain2305,
areusch,
comaniac,
icemelon,
jroesch,
junrushao,
jwfromm,
mbrookhart,
merrymercy,
slyubomirsky,
tqchen,
vinx13,
wweic,
yzhliu and
zhiics
as code owners
AndrewZhaoLuo
changed the title
|
This is now ready for review, cc @anwang2009 @masahi @mbrookhart @anijain2305 |
masahi
left a comment
masahi
left a comment
There was a problem hiding this comment.
Very nice, I've been thinking that we should make "all" ops qnn-aware by auto-generating default lowering like this, so that we don't have to decide which op can run on int8.
| } | ||
|
|
||
| inline Expr Log(Expr e) { | ||
| static const Op& op = Op::Get("log"); |
There was a problem hiding this comment.
Oops, didn't know it was originally there.
At first, I added like 20 unary funcs but decided to scale it back to make review easier (and most of the unary funcs probably have little use). Didn't realize Log was already in pattern_utils so probably deleted it on accident
src/relay/qnn/op/op_common.h
Outdated
| * | ||
| * FloatingPointFunc is usually a handle from "src/relay/transforms/pattern_utils.h" | ||
| * | ||
| * \param OpName the name of registry. |
There was a problem hiding this comment.
nit: update this to "FloatingPointFunc" description
| relay.qnn.op.sqrt, | ||
| np.sqrt, | ||
| input_dtype="int8", | ||
| x_data=np.arange(1, 128, dtype="int8"), |
There was a problem hiding this comment.
any reason this test has x_data specified but the other int8 tests don't?
There was a problem hiding this comment.
Yeah, it's sqrt so we want to keep things in domain. Outside of the domain the function can return really anything (probably either 0 or max/min value of int8)
src/relay/qnn/op/op_common.h
Outdated
| for (size_t i = 1; i < 5; ++i) { \ | ||
| types.push_back(arg_types[i]); \ | ||
| } \ | ||
| auto dequantized_arg = Dequantize(args.x, args.scale, args.zero_point, types, -1); \ |
There was a problem hiding this comment.
Looks like Dequantize -> DequantizeLower expects types to start with the input data type, but in this code types starts with the scale type.
https://github.com/apache/tvm/blob/main/src/relay/qnn/op/dequantize.cc#L99-L105
There was a problem hiding this comment.
Good catch, you appear to be right. Moving to using the MakeDequantize and MakeQuantize to avoid issues with getting type right
|
@anwang2009 PTAL |
AndrewZhaoLuo
force-pushed
the
aluo/qnn/elementwise-unary-ops
branch
from
a473635 to
5456a51
Compare
|
cc @masahi we cool with merging this? |
* 0;276;0cinitial commit * register a bunch of ops * unary ops * add a bunch of tests * 0;276;0crefactor tests * add tests to qnn * comments on macros * add back in log to pattern utils * update floating point func description * proper creating of calls to quantize and dequantize * fix lowering process for using dequantize and quantize ops
3 participants
Adds a way to quickly add unary operators to QNN with default canonicalizations.
Also adds following: