[CK_BUILDER] Replace reference conv with old ck implementation #3604
+284
−1,060
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.
Proposed changes
In #3381, reference algorithms were added to CK Tile. But it turns out that there already was a reference implementation in old CK. This PR replaces the reference conv implementation used by CK-Builder by the one from CK for the following reasons:
This is still not a complete reference implementation though (no multi A/B/D support), but we can add that when required.
Details
This mainly just replaces the implementation that is called by the reference convolution factory, but there are a few extra details in this PR that should be noted:
ckt::run()anyway. I also removed the corresponding tests.ConvTensorLayoutsto not require theSPATIAL_DIM. Its passed viaSIGNATUREanyway, so it doesn't make sense to pass it extra. This also cleans up some of the use cases of that type.Open Questions