Enable non-packed inputs for mlir #3541
Conversation
pfultz2
requested review from
shivadbhavsar and
krzysz00
and removed request for
causten
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3541 +/- ##
========================================
Coverage 92.17% 92.17%
========================================
Files 512 512
Lines 21393 21393
========================================
Hits 19720 19720
Misses 1673 1673 ☔ View full report in Codecov by Sentry. |
This build is not recommended to merge 🔴 |
🔴bert_large_uncased_fp16: FAILED: MIGraphX is not within tolerance - check verbose output |
| stride_ratios.end(), | ||
| ns.lens().begin() + 1, | ||
| [](auto ratio, auto len) { return ratio >= len; }); | ||
| } |
There was a problem hiding this comment.
@krzysz00 This is checking if the shapes can be used by mlir(ie can be constructed from slice/reshape/transpose/broadcast). I know there are similar checks in mlir. Are you able to check that this is checking the shapes correctly?
There was a problem hiding this comment.
https://github.com/ROCm/rocMLIR/blob/develop/mlir/lib/Dialect/MIGraphX/IR/MIGraphX.cpp#L182 is our logic.
This code feels similar, though I'd need to run through some examples with broadcast dimensions
| auto last = std::find(ns.strides().begin(), ns.strides().end(), 0); | ||
| if(*std::prev(last) != 1) | ||
| return false; | ||
| std::adjacent_difference(ns.strides().begin(), |
There was a problem hiding this comment.
Do we want to possible add a parallel execution policy here or are we not concerned about this difference being to large? Looks like adjacent difference has the ability to be parallelized, or is the intent here since shape is const we want this expression to be const so execution policy doesn't matter?
Disregard, stride_ratio shouldn't be const and order matters here due to back inserter not being valid for parallelism
There was a problem hiding this comment.
Looks good unless @krzysz00 has any concerns
CI is all green not sure why github isn't reporting that correctly in jenkins here.
No description provided.