add sed script for operation renaming to allow xdsl-opt to handle maxf, fixes #142 #148
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
We can also investigate if this allows us to uncomment other tests. |
|
superlopuh
reviewed
Jan 22, 2024
superlopuh
reviewed
Jan 22, 2024
kernels/kernels.csv
Outdated
| @@ -27,13 +27,14 @@ fill,16x16xf64,snrt,299 | |||
| fill,16x16xf64,scf_xdsl,2676 | |||
| matmul,8x8xf64,baseline,4229 | |||
| matmul,8x8xf64,linalg,3816 | |||
| matmul,8x8xf64,snitch_stream,2336 | |||
| matmul,8x8xf64,snitch_stream,57 | |||
|
Do we need both? I think we should need only one, to translate to xDSL, we never go xDSL to MLIR, I don't think |
AntonLydike
commented
Jan 23, 2024
| @@ -1,5 +1,5 @@ | |||
| module { | |||
| func.func public @relu(%arg0: memref<16x16xf64>, %arg1: memref<16x16xf64>) -> memref<16x16xf64> { | |||
| func.func public @relu(%arg0: memref<16x16xf64> {llvm.noalias}, %arg1: memref<16x16xf64> {llvm.noalias}) -> memref<16x16xf64> { | |||
There was a problem hiding this comment.
I didn't introduce this change myself. Any idea how it got here?
There was a problem hiding this comment.
hmm, weird, what does git blame say?
There was a problem hiding this comment.
I'm still not sure what happened here, maybe I accidentally maked the file again from some other source? I didn't add them manually that's all.
After removing them and running make pivoted.csv again nothing changed, so I guess it's okay now?
compor
approved these changes
Jan 23, 2024
|
superlopuh
approved these changes
Jan 23, 2024
Interesting, I thought I had done that already. I'll give it another go. |
AntonLydike
force-pushed
the
anton/fix-maxf
branch
from
2db7c08 to
08af338
Compare
|
compor
reviewed
Jan 26, 2024
| find . -type d -name "*.x.logs" -print0 | xargs -0 rm -rf | ||
| find . -type f -name "*.S" -print0 | xargs -0 rm -f | ||
| find . -type f -name "*.ll" -print0 | xargs -0 rm -f | ||
| find . -type f -name "*.ll.mlir" -print0 | xargs -0 rm -f |
There was a problem hiding this comment.
Can you please also add *.bc? for good measure
AntonLydike
force-pushed
the
anton/fix-maxf
branch
from
08af338 to
bcfa451
Compare
|
Rebased, reran all the numbers, should all be green now! Thanks everyone for your help! |
|
superlopuh
approved these changes
Jan 26, 2024
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes #142
This can probably be improved a bit, but thus far it works, and I can understand why. It works like this:
sed 's/arith.maxf/arith.maximumf/g'before each invocation ofxdsl-opt, this translates the maxf we use everywhere else to themaximumfused by xdslsed 's/arith.maximumf/arith.maxf/g'after each invocation ofxdsl-opt, translatingmaximumfback tomaxf. Probably not needed, as the output seems to be.Sand not an MLIR file, but I could be wrong?