Fix extract_slice causing compilation errors
#17519
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
IanWood1
added
benchmarks:cuda
Abbreviated Benchmark Summary@ commit a108d2c016256b0e08484ce739e78f17399d5f68 (no previous benchmark results to compare) Data-Tiling Comparison TableClick to show
Raw Latencies
[Top 3 out of 135 results showed] No improved or regressed compilation metrics 🏖️ For more information: |
IanWood1
removed
the
benchmarks:comp-stats
Signed-off-by: Ian Wood <[email protected]>
IanWood1
removed
benchmarks:cuda
zjgarvey
reviewed
Jun 3, 2024
There was a problem hiding this comment.
Thanks for working on this, @IanWood1.
I'm a bit worried about the generic ops produced in this pattern. For example, if the scale of the initial dequant op is very small so that the results are all very close to zero, does your generated generic op for casting f32 -> i8 just give a zero tensor?
| sourceGenericOp.getIteratorTypesArray(), | ||
| [&](OpBuilder &nestedBuilder, Location loc, ValueRange args) { | ||
| // Custom region for f32 -> i8 conversion | ||
| auto castOp = nestedBuilder.create<arith::FPToSIOp>( |
There was a problem hiding this comment.
How does modification by the quantization scale come into play here? If you just cast the fp to si, I can't imagine you will retain any useful information.
Is it possible to copy the payload of a generic op, but modify the other information?
There was a problem hiding this comment.
Yeah your right, this is definitely a problem. I think that solution might work, I'll take a look
compiler/src/iree/compiler/Dialect/Flow/Transforms/FusionPreprocessing.cpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Ian Wood <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses this SHARK-TestSuite issue related to
ExtractSliceOpcausing compilation failures. The issue originated from a pattern ofdequantize-like->tensor.extract_slice->quantize-like(within a dispatch). This means that thetensor.extract_sliceop was operating on f32 instead of the quantized i8 types. MLIR doesn't handle bufferization on extractslice/differing bit-width casts (see mlir/*/EmptyTensorElimination.cpp), causing stack allocations in the middle of dispatches.This is addressed by adding
linalg.genericops before and after the extract_slice op that cast back to the low bit-width type, eliminating the f32 tensors (after getting merged with thedequantize-like/quantize genericTODOs:
Related issues:
nod-ai/SHARK-TestSuite#182
nod-ai/SHARK-ModelDev#683