Skip to content
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

Fix issue when certain inputs/constants aren't properly declared during MLIR emit #203

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

nvukobratTT
Copy link
Contributor

@nvukobratTT nvukobratTT commented Aug 30, 2024

Previously, MLIR emit was hiting edge cases when declaring constant inputs. More precisely, they were mostly skipped. This fix redefines how inputs are recognized (using kInput node type), and properly distinguish regular and constant inputs vs model parameters.

Issue uncovered during #112 op bringup (reciprocal). At the same time, PR related to #112 is testing this case.

Additionally, this change includes:

  • Shape recalculation before lowering to MLIR; just to be certain that all shapes are correctly matched
  • Additional logs through MLIR emit logic
  • Uplifted MLIR version to the latest

Fixes #201

@nvukobratTT nvukobratTT force-pushed the nvukobrat/mlir_emit_fix branch 2 times, most recently from a9526c5 to e94507c Compare September 2, 2024 08:08
@nvukobratTT
Copy link
Contributor Author

pybuda/test/mlir/test_features.py Outdated Show resolved Hide resolved
pybuda/test/mlir/test_features.py Outdated Show resolved Hide resolved
@nvukobratTT nvukobratTT force-pushed the nvukobrat/mlir_emit_fix branch 5 times, most recently from 16c8175 to e24bbbd Compare September 3, 2024 09:45
@nvukobratTT nvukobratTT force-pushed the nvukobrat/mlir_emit_fix branch 3 times, most recently from 403db1c to 6505466 Compare September 4, 2024 08:57
@pilkicTT
Copy link
Contributor

pilkicTT commented Sep 4, 2024

nit: i would change [bug] to [fix], looks weird to me (as if you are adding a bug, hopefully not) 😄

@nvukobratTT
Copy link
Contributor Author

nit: i would change [bug] to [fix], looks weird to me (as if you are adding a bug, hopefully not) 😄

Hahaha, that makes sense!

@nvukobratTT nvukobratTT changed the title [Bug] Solves issue when certain inputs/constants aren't properly declared during MLIR emit Fix issue when certain inputs/constants aren't properly declared during MLIR emit Sep 4, 2024
@nvukobratTT nvukobratTT force-pushed the nvukobrat/mlir_emit_fix branch 3 times, most recently from 54f1c16 to 521e896 Compare September 4, 2024 09:25
@nvukobratTT nvukobratTT force-pushed the nvukobrat/mlir_emit_fix branch 2 times, most recently from 3e1e53a to 446f03e Compare September 4, 2024 11:18
…ng MLIR emit

Previously, MLIR emit was hiting edge cases when declaring constant inputs. More precisely,
they were mostly skipped. This fix redefines how inputs are recognized (using kInput node type),
and properly distinguish regular and constant inputs vs model parameters.

Issue uncovered during #112 op bringup (reciprocal). At the same time, PR related to #112 is
testing this case. Additionally, inference and training MNIST are also covering this feature
for functionality.

Additionally, this change includes:
- Shape recalculation before lowering to MLIR; just to be certain that all shapes are correctly matched
- Additional logs through MLIR emit logic
- Uplifted MLIR version to the latest

Fixes #201
@nvukobratTT nvukobratTT force-pushed the nvukobrat/mlir_emit_fix branch from 446f03e to 2e94530 Compare September 4, 2024 13:14
@nvukobratTT nvukobratTT merged commit a3c0441 into main Sep 4, 2024
2 checks passed
@nvukobratTT nvukobratTT deleted the nvukobrat/mlir_emit_fix branch September 9, 2024 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] MLIR emit invalid definition of regular and constant inputs
3 participants