-
Notifications
You must be signed in to change notification settings - Fork 645
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
Handle default inlining on funcOp and callOp #19498
Comments
Update LLVM to llvm/llvm-project@3f136f7 (#19479) Carrying the following reverts - llvm/llvm-project#116470 - llvm/llvm-project#117424 - llvm/llvm-project#119671 - llvm/llvm-project#119970 First two are carry over from previous-previous integrate. It is being fixed in #19451 . The last one is a from the previous integrate. The last one is a new error being tracked in #19498 --------- Signed-off-by: Stanley Winata <[email protected]>
Update LLVM to llvm/llvm-project@b07e7b76c5d532a6 (llvm/llvm-project#120002) Carrying the following reverts - llvm/llvm-project#116470 - llvm/llvm-project#117424 - llvm/llvm-project#119671 - llvm/llvm-project#119970 First two are carry over from previous-previous integrate. It is being fixed in iree-org#19451 . The last one is a from the previous integrate. The last one is a new error being tracked in iree-org#19498 Signed-off-by: Stanley Winata <[email protected]>
Update LLVM to llvm/llvm-project@b07e7b76c5d532a6 (llvm/llvm-project#120002) Carrying the following reverts - llvm/llvm-project#116470 - llvm/llvm-project#117424 - llvm/llvm-project#119671 - llvm/llvm-project#119970 First two are carry over from previous-previous integrate. It is being fixed in iree-org#19451 . The last one is a from the previous integrate. The last one is a new error being tracked in iree-org#19498 Signed-off-by: Stanley Winata <[email protected]>
I find it strange that adding a new optional attribute broke parsing, but only for MLIR bytecode and not MLIR text. Is that the expected behavior? Sample logs: https://github.com/iree-org/iree/actions/runs/12358432483 We can update our tests as needed... and this could be a good forcing function.
|
Sent nod-ai/SHARK-TestSuite#417 and nod-ai/SHARK-TestSuite#418 to delete some old tests that are (almost) redundant at this point. |
Cross posting from our convo on discord: looks like the root issue It is to do with the inline attribute(per the reverted commit), i.e we'd need the util.globalOp to have that noinline attribute on i.e:
I think we just need to regenerate the IR without modifying any of the code and should generate the I tested the hypothesis above locally, i.e:
|
Not familiar with these side of the stack so not sure also, all I know is bytecode == smaller memory footprint + non-readable. 😅
Yeah that's unfortunate, what do you think if I just do an iree-opt on it to get some noinline attributes on it and then convert it to back to an mlirbc? Or would that be too hacky? |
util.global should use inlining_policy, e.g. util.global private @_params.classifier.weight {inlining_policy = #util.inline.never} : tensor<30x20xf32> |
Ah interesting, need to figure out if this gets autogenerated now from sharktank (unless here knows). But in that case, we may need to either: a) regenerate with correct inlining policy (a bit more challenging because regeneration process is not documented) LMK if you guys have a preference on which solution works better, or if there' something else we can do to resolve this |
Changes in C++ mostly handle changes done in llvm/llvm-project@4b56345 that combines SCFTilingResult and SCFReductionTilingResult. This PR also carries the following reverts - llvm/llvm-project#119671 - llvm/llvm-project#119970 The first one is a from the previous integrate. The last one is a new error being tracked in iree-org#19498. Signed-off-by: Stanley Winata <[email protected]>
Changes in C++ mostly handle changes done in llvm/llvm-project@4b56345 that combines SCFTilingResult and SCFReductionTilingResult. This PR also carries the following reverts - llvm/llvm-project#119671 - llvm/llvm-project#119970 The first one is a from the previous integrate. The last one is a new error being tracked in iree-org#19498. Signed-off-by: Stanley Winata <[email protected]>
Changes in C++ mostly handle changes done in llvm/llvm-project@4b56345 that combines SCFTilingResult and SCFReductionTilingResult. This PR also carries the following reverts - llvm/llvm-project#119970 The issue related to the revert is being tracked in iree-org#19498. Signed-off-by: Stanley Winata <[email protected]>
Update LLVM to llvm/llvm-project@b13592219c421820b (llvm/llvm-project#85376) Changes done to resolve mlirbc issue in iree-org#19498 This PR also carries the following reverts: llvm/llvm-project#120115 The main issue with this PR is it breaks matvec codegen generating scf.if instead of scf.for(s). An issue will be pushed up for repro. Signed-off-by: Stanley Winata <[email protected]>
Update LLVM to llvm/llvm-project@b13592219c421820b (llvm/llvm-project#85376) Changes done to resolve mlirbc issue in iree-org#19498 This PR also carries the following reverts: llvm/llvm-project#120115 The main issue with this PR is it breaks matvec codegen generating scf.if instead of scf.for(s). An issue will be pushed up for repro. Signed-off-by: Stanley Winata <[email protected]>
Hey @ScottTodd, fyi I have regenerated the MLIR and uploaded them to the Azure, plus regenerated the mlirbc in SHARK-TestSuite to the latest version here https://github.com/nod-ai/SHARK-TestSuite/tree/raikonen/integrate/llvm20241220. |
Update LLVM to llvm/llvm-project@b13592219c421820b (llvm/llvm-project#85376) Changes done to resolve mlirbc issue in iree-org#19498 This PR also carries the following reverts: llvm/llvm-project#120115 The main issue with this PR is it breaks matvec codegen generating scf.if instead of scf.for(s). An issue will be pushed up for repro. Signed-off-by: Stanley Winata <[email protected]>
Signed-off-by: Stanley Winata <[email protected]>
After llvm/llvm-project#119970 landed upstream, we are running issues compiling some of our models. It throws off:
A good way to repro this issue is to do:
The text was updated successfully, but these errors were encountered: