[TOSA] Add lowering for aten.expm1 #3949
Conversation
* Add Torch to TOSA legalization for aten.expm1 * Update xfail_sets with new test results * Add new LIT tests Signed-off-by: Justin Ngo <[email protected]> Change-Id: I834d0c7416341f884612053aebf9fcc90bcb3b53
| // If input is not a float type then cast it to result element type | ||
| auto selfElemTy = selfType.getElementType(); | ||
| if (!isa<mlir::FloatType>(selfElemTy)) | ||
| self = tosa::promoteType(rewriter, self, resultType); |
There was a problem hiding this comment.
Do you know why there are tosaCastTensorToType and promoteType? tosaCastTensorToType seems to be a super-set of promoteType in terms of functionality -- so is there a need to maintain promoteType?
There was a problem hiding this comment.
After some discussion with @sjarus, I've decided to combine tosaCastTensorToType and promoteType together into one function wrapped around the tosa.cast op creation process. The combined function will still be called tosaCastTensorToType as this name is more descriptive about what this function does. I plan to replace all usages of tosa.cast creation with this "new" function to make everything uniform.
The following are the differences between the current promoteType and tosaCastTensorToType functions:
promoteTypechecks if the input and output types are same before performing the cast, which will prevent unnecessary cast operation if both types are the same.tosaCastTensorToTypehas the cast validity check.
My plan is to take the same-type check from promoteType and add it to tosaCastTensorToType, then remove the promoteType function altogether. As for the cast validity check, I will tighten it more so that it will strictly follow the TOSA v1.0 spec (i.e. removing all I64 and F64 casting as they are not allowed in TOSA). I will update that in #3948. However, with this tightening, many e2e tests will fail. This is not desirable as these operations, although are not congruent with the spec,are still permissible. Therefore, after tightening up the checkValidityOfCast function, I will leave it out of the tosaCastTensorToType function for now and enable it later with a potential --strict mode that is defaulted to off. TOSA validation should flag illegal constructs based on each profile anyway, so this --strict mode is just another guard before that.
I will begin the new tosaCastTensorToType work in a separate PR from #3948, so that it is easier to keep track of changes and progress.
Please let me know what you think @sahas3. Thanks!
There was a problem hiding this comment.
Hi @justin-ngo-arm, thanks for looking into this. Your plan sounds good to me.
I agree that the --strict mode shouldn't be enabled by default. In addition to existing e2e tests in this repo, there are standard models like deeplabv3 from torchvision that generates tosa.cast with 64bit types during the torch-to-tosa conversion process. Allowing non-conformance of tosa.cast with the spec in terms of 64bit types helps in supporting such models.
Change-Id: I834d0c7416341f884612053aebf9fcc90bcb3b53