WIP: Adding TRT options/task #435
Conversation
| // TODO (pranavm): Figure out a better way to reuse TRT translation options - | ||
| // maybe move to options providers? | ||
| struct TensorRTOptions | ||
| : public mlirtrt::compiler::OptionsProvider<TensorRTOptions> { | ||
| mlir::tensorrt::TensorRTTranslationOptions options; | ||
|
|
||
| void addToOptions(mlir::OptionsContext &context) { | ||
| options.addToOptions(context); | ||
| } | ||
| }; |
There was a problem hiding this comment.
I can move TensorRTTranslationOptions to make them an options provider if that makes sense to do.
| // TODO (pranavm): Check if this needs to be conditional - the TRT passes | ||
| // above are not. | ||
| #ifdef MLIR_TRT_TARGET_TENSORRT | ||
| mlirtrt::compiler::registerTensorRTToExecutableTask(); | ||
| #endif |
There was a problem hiding this comment.
I'm not sure what exactly needs to be guarded. In a lot of places we guard TRT things with MLIR_TRT_TARGET_TENSORRT but that doesn't seem to be the case for the TRT passes registered above.
There was a problem hiding this comment.
The TensorRT dialect and MLIR passes are always built. It doesn't depend on actually having TensorRT binaries or headers to build against (or at least, that was the idea, we haven't been enforcing it). The MLIR_TRT_TARGET_TENSORRT is basically a guard for anything that relies on having actual TRT headers or libraries to link against, for example the translation from MLIR to TensorRT, hence the TARGET_TENSORRT in the name.
| TensorRTToExecutableOptions::TensorRTToExecutableOptions( | ||
| TaskExtensionRegistry extensions) { | ||
| // TODO (pranavm): Do we need to support extensions? | ||
| } |
There was a problem hiding this comment.
Not sure if we want to require extensions for all options types or if we need to handle both cases in the options registry. If it's the former, then I can just assert that the extensions are empty here (or maybe even just add support?). If it's the latter, we could have a setExtensions method so it becomes optional instead of having it part of the constructor.
mlir-tensorrt/compiler/include/mlir-tensorrt/Compiler/OptionsProviders.h
Outdated
Show resolved
Hide resolved
mlir-tensorrt/compiler/include/mlir-tensorrt/Compiler/PassManagerUtils.h
Outdated
Show resolved
Hide resolved
pranavm-nvidia
force-pushed
the
trt-task
branch
from
137caf5 to
08f90f3
Compare
pranavm-nvidia
force-pushed
the
trt-task
branch
from
08f90f3 to
88d48b9
Compare
| //===----------------------------------------------------------------------===// | ||
| // OutlineTensorRTOpPass | ||
| //===----------------------------------------------------------------------===// | ||
| // TODO: what are the dependent dialects? what are the options? |
There was a problem hiding this comment.
What are the required dependent dialects/options?
There was a problem hiding this comment.
Also I got error during compilation:
error: ‘createOutlineTensorRTOpPass’ was not declared in this scope
Did I missing anything for the tablegen config?
| /// Helper function to call the `makeRegionIsolatedFromAbove` to capture all | ||
| /// required arguments into the InlineGroupOp region. | ||
| // static LogicalResult | ||
| // makeIsolatedFromAboveImpl(RewriterBase &rewriter, plan::InlineGroupOp regionOp, |
There was a problem hiding this comment.
Is this function still useful?
| SymbolTableCollection symbolTable; | ||
| IRRewriter rewriter(&getContext()); | ||
| // what are these? are they needed? | ||
| // DataFlowSolver solver; |
There was a problem hiding this comment.
We just decide if an op is tensorrt dialect or not. is solver still needed then?
| emitError(module.getLoc()) << "failed to create clustering options"; | ||
| return signalPassFailure(); | ||
| } | ||
| // What do they do here? |
There was a problem hiding this comment.
Did not figure out this part, regionOps is not used either.
No description provided.