-
Notifications
You must be signed in to change notification settings - Fork 386
Extend the verifyInputTensors to shape information provided with compilation option #3346
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
chentong319
commented
Dec 9, 2025
- Add two new options: shapeInformationUB and shapeInformationLB. They are in the similar format to shapeInformation but specify the upper bound and lower bound of the dimensions of input. One use case is the max sequence length. Another possible use case is minimum batch size.
- Insert runtime check on the actual inputs if the UB or LB is specified.
- I tested cases. Not sure how to add such runtime test cases which may generate assertion error into our test set.
Signed-off-by: Tong Chen <[email protected]>
tungld
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to put upper and lower bounds into the argument attributes when importing a model and then to the input signature when lowering onnx.EntryPoint to krnl.entry_point.
For example, the input argument is like this:
func.func @main_graph(%arg0: tensor<?xf32> {onnx.name = "x",
onnx.dim_params = "0:a", onnx.dim_lbs="0:5", onnx.dim_ubs="0:50"}When lowering onnx.EntryPoint to krnl, krnl.entry_pointwe will have this json info for input signature:
{"type" : "f32" , "dims" : [-1] , "lbs": [5], "ubs": [50], "name": "x"}We can even replace "dims": [-1] by "dims": ['a'].
|
|
||
| switch (boundType) { | ||
| case 0: // UB | ||
| noLessOrFailed(module, rewriter, loc, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look like this should be for LB.
| "verifyInputTensors failed: the upper bound for the input " + | ||
| std::to_string(inputID) + " of dimension " + | ||
| std::to_string(dimID) + | ||
| " is set by --shapeInformationLB as " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shapeInformationUB?
| noGreaterOrFailed(module, rewriter, loc, | ||
| create.llvm.constant(int64Ty, static_cast<int64_t>(bound)), | ||
| actualDim, | ||
| "verifyInputTensors failed: the upper bound for the input " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "the upper bound" -> "the lower bound"
|
Yes, we can transfer the UB and LB info to function op. Then we support information coming from command line and from model (.mlir or .onnx). |
|
To avoid the confusion of UB or LB, I introduced a enumerate type. |
@chentong319 please let us know if you plan to implement this here or in another PR. Thanks! |