[MLIR][TosaValidation] Enable float element-types on default

[miritb]

After change in tosa-validation profiles to list-option, float element types where no more allowed in default (where profile is/contains undefined enumeration value). This change allows floats when undefined value is contained in the profile.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-tosa

Author: None (miritb)

Changes

After change in tosa-validation profiles to list-option, float element types where no more allowed in default (where profile is/contains undefined enumeration value). This change allows floats when undefined value is contained in the profile.

---

Full diff: https://github.com/llvm/llvm-project/pull/114376.diff

1 Files Affected:

• (modified) mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp (+1-1)

diff --git a/mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp b/mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp
index e390a613b58077..e836c383557f03 100644
--- a/mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp
+++ b/mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp
@@ -521,7 +521,7 @@ LogicalResult TosaValidation::applyVariableCheck(Operation *op) {
 
 bool TosaValidation::isValidElementType(Type type) {
   if (isa<FloatType>(type)) {
-    if (!isEnabledProfile(TosaProfileEnum::MainInference))
+    if (!isEnabledProfile(TosaProfileEnum::Undefined) && !isEnabledProfile(TosaProfileEnum::MainInference))
       return false;
     return type.isF32() || type.isF16() || type.isBF16();
   }

[miritb]

@tatwaichong can you please review?

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[GeorgeARM]

@miritb thanks for you patch. It looks good on my side but before proceeding am really curious why there is an option for TosaProfileEnum::Undefined. Not quite sure how someone can perform validation on something undefined. If the purpose of this option is to perform full specification compliance then probably a different name will be more appropriate. @Tai78641 do you have any insight on this?

[tatwaichong]

hi @miritb, is there any lit or regresssion test failure related to this in MLIR?

Profile validation should be default off, and should be explicitly turned on for specific profile requirements. Since

• some users want to skip the TOSA validation pass, please see PR 91742.
• why we consider MI the default rather than BI? tosa profile is optional implemented by backends.
  Thus set the profile option to none (Unknown) by default. Does it make sense?

@GeorgeARM
Currently we don't have full specification compliance option in the validation pass, perhaps it worths to add.

Can you try to use --tosa-validate="profile=mi" to pass the float types?

[GeorgeARM]

@tatwaichong trying to reason if having undefined as an option in the profile validation does makes sense or serves any purpose. Would prefer to avoid having default values and actually expect that when validation is requested a valid option is provided. Apologies if I am missing something.

[miritb]

One of our teams uses 'addTosaToLinalgPasses', w/o providing an explicit value for 'validationOptions' argument. Since the change in '[mlir][tosa] Change the type of profile option to ListOption ([#111214]' this caused the code not to accept tensors with float element-type. In current change we only want to return the behavior back to what it was before #111214

[tatwaichong]

Thanks for your sharing. Sorry for inconvenience. As mentioned, TOSA profiles and extensions are now optional implemented, thus the validation should to be enabled explicitly. Hope you can understand. :)

The team is managing to upstream PRs to fill the gap between MLIR TOSA implementation and TOSA specification.

[GeorgeARM]

Hello @tatwaichong what is the status with helping unblocking this?
Have you upstream a fix that @miritb can use?