Skip to content

[mlir][bufferization] Alternate op bufferization approach#199898

Draft
andrey-golubev wants to merge 2 commits into
llvm:mainfrom
andrey-golubev:get_buffer_type_with_fallback
Draft

[mlir][bufferization] Alternate op bufferization approach#199898
andrey-golubev wants to merge 2 commits into
llvm:mainfrom
andrey-golubev:get_buffer_type_with_fallback

Conversation

@andrey-golubev
Copy link
Copy Markdown
Contributor

Make op bufferization go through type bufferization, by differentiating
between "known" and "unknown" cases better: when the type is known, i.e.
it is upstream-MLIR-aligned, TensorLikeType::getBufferType() would
dispatch to an op-provided bufferization callback; when the type is
unknown, unknown type converter would be called instead.

In return, this allows operation bufferization to be extended with
custom semantics without sacrificing the upstream's default behaviour.

The implementation for a builtin TensorType's TensorLikeType interface
is a bit overly complex for no reason: it calls `getMemRefType()`
whereas in reality `options.unknownTypeConverterFn()` would directly be
called due to the way the current code is written. Remove what is
effectively a wrapper function so that the current semantics is a bit
clearer.
Make op bufferization go through type bufferization, by differentiating
between "known" and "unknown" cases better: when the type is known, i.e.
it is upstream-MLIR-aligned, TensorLikeType::getBufferType() would
dispatch to an op-provided bufferization callback; when the type is
unknown, unknown type converter would be called instead.

In return, this allows operation bufferization to be extended with
custom semantics without sacrificing the upstream's default behaviour.
@github-actions
Copy link
Copy Markdown

🐧 Linux x64 Test Results

The build failed before running any tests. Click on a failure below to see the details.

lib/Target/RISCV/CMakeFiles/LLVMRISCVCodeGen.dir/RISCVISelLowering.cpp.o
FAILED: lib/Target/RISCV/CMakeFiles/LLVMRISCVCodeGen.dir/RISCVISelLowering.cpp.o
sccache /opt/llvm/bin/clang++ -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GLIBCXX_USE_CXX11_ABI=1 -D_GNU_SOURCE -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_EXTENSIVE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/gha/actions-runner/_work/llvm-project/llvm-project/build/lib/Target/RISCV -I/home/gha/actions-runner/_work/llvm-project/llvm-project/llvm/lib/Target/RISCV -I/home/gha/actions-runner/_work/llvm-project/llvm-project/build/include -I/home/gha/actions-runner/_work/llvm-project/llvm-project/llvm/include -gmlt -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wno-pass-failed -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -std=c++17 -fvisibility=hidden -UNDEBUG -fno-exceptions -funwind-tables -fno-rtti -MD -MT lib/Target/RISCV/CMakeFiles/LLVMRISCVCodeGen.dir/RISCVISelLowering.cpp.o -MF lib/Target/RISCV/CMakeFiles/LLVMRISCVCodeGen.dir/RISCVISelLowering.cpp.o.d -o lib/Target/RISCV/CMakeFiles/LLVMRISCVCodeGen.dir/RISCVISelLowering.cpp.o -c /home/gha/actions-runner/_work/llvm-project/llvm-project/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
/home/gha/actions-runner/_work/llvm-project/llvm-project/llvm/lib/Target/RISCV/RISCVISelLowering.cpp:5228:10: error: no matching function for call to 'isLegalVTForZvzipOperand'
5228 |         !isLegalVTForZvzipOperand(VT, Subtarget,
|          ^~~~~~~~~~~~~~~~~~~~~~~~
/home/gha/actions-runner/_work/llvm-project/llvm-project/llvm/lib/Target/RISCV/RISCVISelLowering.cpp:5206:13: note: candidate function not viable: requires 2 arguments, but 3 were provided
5206 | static bool isLegalVTForZvzipOperand(MVT VT, const RISCVSubtarget &Subtarget) {
|             ^                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

If these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the infrastructure label.

@github-actions
Copy link
Copy Markdown

🪟 Windows x64 Test Results

The build failed before running any tests. Click on a failure below to see the details.

[code=1] lib/Target/RISCV/CMakeFiles/LLVMRISCVCodeGen.dir/RISCVISelLowering.cpp.obj
FAILED: [code=1] lib/Target/RISCV/CMakeFiles/LLVMRISCVCodeGen.dir/RISCVISelLowering.cpp.obj
sccache C:\clang\clang-msvc\bin\clang-cl.exe  /nologo -TP -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_EXTENSIVE -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -IC:\_work\llvm-project\llvm-project\build\lib\Target\RISCV -IC:\_work\llvm-project\llvm-project\llvm\lib\Target\RISCV -IC:\_work\llvm-project\llvm-project\build\include -IC:\_work\llvm-project\llvm-project\llvm\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:__cplusplus /Oi /Brepro /bigobj /permissive- -Werror=unguarded-availability-new /W4  -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wno-pass-failed -Wmisleading-indentation -Wctad-maybe-unsupported /Gw /O2 /Ob2  -std:c++17 -MD -UNDEBUG /EHs-c- /GR- /showIncludes /Folib\Target\RISCV\CMakeFiles\LLVMRISCVCodeGen.dir\RISCVISelLowering.cpp.obj /Fdlib\Target\RISCV\CMakeFiles\LLVMRISCVCodeGen.dir\LLVMRISCVCodeGen.pdb -c -- C:\_work\llvm-project\llvm-project\llvm\lib\Target\RISCV\RISCVISelLowering.cpp
C:\_work\llvm-project\llvm-project\llvm\lib\Target\RISCV\RISCVISelLowering.cpp(5228,10): error: no matching function for call to 'isLegalVTForZvzipOperand'
5228 |         !isLegalVTForZvzipOperand(VT, Subtarget,
|          ^~~~~~~~~~~~~~~~~~~~~~~~
C:\_work\llvm-project\llvm-project\llvm\lib\Target\RISCV\RISCVISelLowering.cpp(5206,13): note: candidate function not viable: requires 2 arguments, but 3 were provided
5206 | static bool isLegalVTForZvzipOperand(MVT VT, const RISCVSubtarget &Subtarget) {
|             ^                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

If these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the infrastructure label.

Comment on lines +256 to +257
return cast<TensorLikeType>(getType()).getBufferType(
options, [&]() { return emitError(); }, defaultGetBufferType);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matthias-springer this would be the way I'd love to see bufferization done (or somehow similarly) for "owners" of values. what we have from this is 2 things:
a) custom type bufferization (e.g. for non-upstream-tensors) is now "natively" supported - i.e. we don't need any special dispatch, it just works due to an interface
b) "unknown" encoding causes unknown type conversion behind the scenes for upstream tensor; and users can define what "unknown encoding" means also.

at the same time, the "fallback" bufferization also exists and can still be used if desired.

I am not 100% sure about the new API, but this feels to be the direction.

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.

1 participant