Fix alias analysis #185
Fix alias analysis #185
Conversation
Set was replaced with LoadStoreOp, which was not recognized as UnaryOp, and thus the alias analysis failed to detect safe aliasing
csrc/ir_utils.cpp
Outdated
| @@ -572,6 +572,11 @@ bool isReductionTvOp(const Expr* expr) { | |||
| return ir_utils::isTvOp(expr) && isReductionOp(expr); | |||
| } | |||
|
|
|||
| bool isPointwiseTvOp(const Expr* expr) { | |||
| return isTvOp(expr) && | |||
| expr->isOneOf<UnaryOp, BinaryOp, TernaryOp, LoadStoreOp>(); | |||
There was a problem hiding this comment.
Just raise a question for discussion: Should we disable inner sharing for LoadStoreOp with consumer that has rfactor domain (that is, permutation ops)? I am not sure, but I think it should be OK to share.
There was a problem hiding this comment.
T2 = transpose(T1)
T3 = sin(T2)
For the example above, I think it is OK to share T3 with T1.
We shouldn't share T2 with T1, but I think we are already disallowing this type of sharing.
There was a problem hiding this comment.
That's a good point. Is it really safe? Suppose t1 = transpose(t0), and both of them are on shared memory, code like below seemed to be generated:
__shared__ t0[N] = ...;
auto& t1 = t0;
t1[i * k + j] = t0[j *k + i];
This isn't valid an it's not really pointwise, is it?
There was a problem hiding this comment.
Wait what, is t1 sharing with t0? I think we previously disabled it. csarofeen/pytorch#2490
There was a problem hiding this comment.
That is right. But should we call it a pointwise op?
|
!build |
|
!build |
Set was replaced with LoadStoreOp, which was not recognized as UnaryOp, and thus the alias analysis failed to detect safe aliasing
Fixes #163