Conversation
526f11d to
236e4c4
Compare
SDPAOpSDPAOp
|
!build |
naoyam
left a comment
There was a problem hiding this comment.
Is the root map change required for the scheduling support?
Yes. Otherwise, we noticed with the earlier Matmul and LinearOps, that the |
I suspect there might be problems with |
|
Thanks. Looks good to me. I'll let @jacobhinkle give a stamp. |
It would still not work since, the third dimension (L/S) only maps from query to output. |
|
!build |
jacobhinkle
left a comment
There was a problem hiding this comment.
Test looks good. My comments are mostly minor
|
|
||
| // Map N, H from any input (query/key/value) | ||
| for (auto idx : c10::irange(consumer_root.size())) { | ||
| if (idx < 2) { |
There was a problem hiding this comment.
For idx == 0 and consumer_tv_->sameAs(op->output(2)) || consumer_tv_->sameAs(op->output(3)), these should not map since the extents differ by 1. I would just put a check before this that consumer is the output or logsumexp. Btw you might want to add accessors like op->logSumExp() to make this new condition more readable.
There was a problem hiding this comment.
Even if I use resize?
I saw an error that there was no mapped iterdomain from producer.
There was a problem hiding this comment.
Oh ok so you are mapping the consumer root then you have an rfactor domain for that consumer which uses a Resize?
Co-authored-by: Jacob Hinkle <1454944+jacobhinkle@users.noreply.github.com>
Co-authored-by: Jacob Hinkle <1454944+jacobhinkle@users.noreply.github.com>
Co-authored-by: Jacob Hinkle <1454944+jacobhinkle@users.noreply.github.com>
|
!build |
Stacked on #2294.
SDPAOptoExprEvalScheduler.ExprEvalSched::canScheduleto skip computeAt checks and only use the compile time check since expression evaluator scheduler will only accept segments with a single expression of type MatmulOp / LinearOp / SdpaOp.,Issue #2278