Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

Hardcode84 (Ivan Butygin)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 2 2020, 2:39 AM (157 w, 1 d)

Recent Activity

Thu, Nov 23

Hardcode84 committed rGbf353a71a2c1: Revert "[mlir] Workaround for export lib generation on Windows for… (authored by Hardcode84).
Revert "[mlir] Workaround for export lib generation on Windows for…
Thu, Nov 23, 4:33 AM · Restricted Project, Restricted Project
Hardcode84 added a reverting change for rG6248c24876d8: [mlir] Workaround for export lib generation on Windows for…: rGbf353a71a2c1: Revert "[mlir] Workaround for export lib generation on Windows for….
Thu, Nov 23, 4:33 AM · Restricted Project, Restricted Project

Sep 7 2023

Hardcode84 committed rG5dce74817b71: [mlir][ub] Add poison support to CommonFolders.h (authored by Hardcode84).
[mlir][ub] Add poison support to CommonFolders.h
Sep 7 2023, 3:31 AM · Restricted Project, Restricted Project
Hardcode84 closed D159013: [mlir][ub] Add poison support to CommonFolders.h.
Sep 7 2023, 3:30 AM · Restricted Project, Restricted Project

Sep 6 2023

Hardcode84 added a comment to D159013: [mlir][ub] Add poison support to CommonFolders.h.

Now build failure looks like this:

[1/27] Building CXX object tools\mlir\lib\Dialect\Math\IR\CMakeFiles\obj.MLIRMathDialect.dir\MathOps.cpp.obj
FAILED: tools/mlir/lib/Dialect/Math/IR/CMakeFiles/obj.MLIRMathDialect.dir/MathOps.cpp.obj
C:\PROGRA~2\MICROS~2\2022\BUILDT~1\VC\Tools\MSVC\1435~1.322\bin\Hostx64\x64\cl.exe  /nologo /TP -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -DMLIR_CUDA_CONVERSIONS_ENABLED=0 -DMLIR_ROCM_CONVERSIONS_ENABLED=0 -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_ENABLE_HARDENED_MODE -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -ID:\projs\llvm\llvm-build\tools\mlir\lib\Dialect\Math\IR -ID:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR -ID:\projs\llvm\llvm-build\include -ID:\projs\llvm\llvm-project\llvm\include -ID:\projs\llvm\llvm-project\mlir\include -ID:\projs\llvm\llvm-build\tools\mlir\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj /permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -w14062 -we4238 /Gw /O2 /Ob2 -std:c++17 -MD  /EHs-c- /GR- -UNDEBUG /showIncludes /Fotools\mlir\lib\Dialect\Math\IR\CMakeFiles\obj.MLIRMathDialect.dir\MathOps.cpp.obj /Fdtools\mlir\lib\Dialect\Math\IR\CMakeFiles\obj.MLIRMathDialect.dir\ /FS -c D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp
D:\projs\llvm\llvm-project\mlir\include\mlir/IR/BuiltinAttributes.h(359): warning C4996: 'std::complex<llvm::APFloat>::complex': warning STL4037: The effect of instantiating the template std::complex for any type other than float, double, or long double is unspecified. You can define _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to suppress this warning.
D:\projs\llvm\llvm-project\mlir\include\mlir/Dialect/CommonFolders.h(200): error C2338: static_assert failed: 'PoisonAttr is undefined, either add a dependency on UB dialect or pass void as template argument to opt-out from poison semantics.'
D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp(59): note: see reference to function template instantiation 'mlir::Attribute mlir::constFoldUnaryOpConditional<mlir::FloatAttr,mlir::FloatAttr::ValueType,mlir::ub::PoisonAttr,mlir::math::AtanOp::fold::<lambda_1>>(llvm::ArrayRef<T>,CalculationT &&)' being compiled
        with
        [
            T=mlir::Attribute,
            CalculationT=mlir::math::AtanOp::fold::<lambda_1>
        ]
D:\projs\llvm\llvm-project\mlir\include\mlir/Dialect/CommonFolders.h(127): error C2338: static_assert failed: 'PoisonAttr is undefined, either add a dependency on UB dialect or pass void as template argument to opt-out from poison semantics.'
D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp(82): note: see reference to function template instantiation 'mlir::Attribute mlir::constFoldBinaryOpConditional<mlir::FloatAttr,mlir::FloatAttr::ValueType,mlir::ub::PoisonAttr,mlir::math::Atan2Op::fold::<lambda_1>>(llvm::ArrayRef<T>,CalculationT &&)' being compiled
        with
        [
            T=mlir::Attribute,
            CalculationT=mlir::math::Atan2Op::fold::<lambda_1>
        ]
ninja: build stopped: subcommand failed.
Sep 6 2023, 12:04 PM · Restricted Project, Restricted Project
Hardcode84 updated the diff for D159013: [mlir][ub] Add poison support to CommonFolders.h.

better error message

Sep 6 2023, 12:03 PM · Restricted Project, Restricted Project
Hardcode84 added a comment to D159013: [mlir][ub] Add poison support to CommonFolders.h.

I'm not a big fan of those SFINAE tricks.

Would you be more comfortable with it if instead of doing an one-of thing here, we first added a type completeness check to STLExtras?

Sep 6 2023, 7:50 AM · Restricted Project, Restricted Project
Hardcode84 added a comment to D159013: [mlir][ub] Add poison support to CommonFolders.h.

I'm not a big fan of those SFINAE tricks.

Sep 6 2023, 4:56 AM · Restricted Project, Restricted Project

Sep 5 2023

Hardcode84 added a comment to D159013: [mlir][ub] Add poison support to CommonFolders.h.
[1/27] Building CXX object tools\mlir\lib\Dialect\Math\IR\CMakeFiles\obj.MLIRMathDialect.dir\MathOps.cpp.obj
FAILED: tools/mlir/lib/Dialect/Math/IR/CMakeFiles/obj.MLIRMathDialect.dir/MathOps.cpp.obj
C:\PROGRA~2\MICROS~2\2022\BUILDT~1\VC\Tools\MSVC\1435~1.322\bin\Hostx64\x64\cl.exe  /nologo /TP -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -DMLIR_CUDA_CONVERSIONS_ENABLED=0 -DMLIR_ROCM_CONVERSIONS_ENABLED=0 -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_ENABLE_HARDENED_MODE -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -ID:\projs\llvm\llvm-build\tools\mlir\lib\Dialect\Math\IR -ID:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR -ID:\projs\llvm\llvm-build\include -ID:\projs\llvm\llvm-project\llvm\include -ID:\projs\llvm\llvm-project\mlir\include -ID:\projs\llvm\llvm-build\tools\mlir\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj /permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -w14062 -we4238 /Gw /O2 /Ob2 -std:c++17 -MD  /EHs-c- /GR- -UNDEBUG /showIncludes /Fotools\mlir\lib\Dialect\Math\IR\CMakeFiles\obj.MLIRMathDialect.dir\MathOps.cpp.obj /Fdtools\mlir\lib\Dialect\Math\IR\CMakeFiles\obj.MLIRMathDialect.dir\ /FS -c D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp
D:\projs\llvm\llvm-project\mlir\include\mlir/IR/BuiltinAttributes.h(359): warning C4996: 'std::complex<llvm::APFloat>::complex': warning STL4037: The effect of instantiating the template std::complex for any type other than float, double, or long double is unspecified. You can define _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to suppress this warning.
D:\projs\llvm\llvm-project\mlir\include\mlir/IR/Attributes.h(406): error C2027: use of undefined type 'mlir::ub::PoisonAttr'
D:\projs\llvm\llvm-project\mlir\include\mlir/Dialect/CommonFolders.h(26): note: see declaration of 'mlir::ub::PoisonAttr'
D:\projs\llvm\llvm-project\mlir\include\mlir/IR/Attributes.h(399): note: while compiling class template member function 'bool llvm::CastInfo<mlir::ub::PoisonAttr,const T,void>::isPossible(mlir::Attribute)'
        with
        [
            T=mlir::Attribute
        ]
D:\projs\llvm\llvm-project\llvm\include\llvm/Support/Casting.h(549): note: see reference to function template instantiation 'bool llvm::CastInfo<mlir::ub::PoisonAttr,const T,void>::isPossible(mlir::Attribute)' being compiled
        with
        [
            T=mlir::Attribute
        ]
D:\projs\llvm\llvm-project\llvm\include\llvm/Support/Casting.h(548): note: see reference to class template instantiation 'llvm::CastInfo<mlir::ub::PoisonAttr,const T,void>' being compiled
        with
        [
            T=mlir::Attribute
        ]
D:\projs\llvm\llvm-project\mlir\include\mlir/Dialect/CommonFolders.h(186): note: see reference to function template instantiation 'bool llvm::isa<mlir::ub::PoisonAttr,T>(const From &)' being compiled
        with
        [
            T=mlir::Attribute,
            From=mlir::Attribute
        ]
D:\projs\llvm\llvm-project\mlir\include\mlir/Dialect/CommonFolders.h(338): note: see reference to function template instantiation 'mlir::Attribute mlir::constFoldUnaryOpConditional<mlir::FloatAttr,mlir::FloatAttr::ValueType,mlir::ub::PoisonAttr,mlir::math::AtanOp::fold::<lambda_1>>(llvm::ArrayRef<T>,CalculationT &&)' being compiled
        with
        [
            T=mlir::Attribute,
            CalculationT=mlir::math::AtanOp::fold::<lambda_1>
        ]
D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp(59): note: see reference to function template instantiation 'mlir::Attribute mlir::constFoldUnaryOpConditionalPoison<mlir::FloatAttr,mlir::FloatAttr::ValueType,mlir::math::AtanOp::fold::<lambda_1>>(llvm::ArrayRef<T>,CalculationT &&)' being compiled
        with
        [
            T=mlir::Attribute,
            CalculationT=mlir::math::AtanOp::fold::<lambda_1>
        ]
D:\projs\llvm\llvm-project\mlir\include\mlir/IR/Attributes.h(406): error C3861: 'classof': identifier not found
ninja: build stopped: subcommand failed.

forgot to comment out materializer

Sep 5 2023, 1:40 PM · Restricted Project, Restricted Project
Hardcode84 added a comment to D159013: [mlir][ub] Add poison support to CommonFolders.h.
[1/27] Building CXX object tools\mlir\lib\Dialect\Math\IR\CMakeFiles\obj.MLIRMathDialect.dir\MathOps.cpp.obj
FAILED: tools/mlir/lib/Dialect/Math/IR/CMakeFiles/obj.MLIRMathDialect.dir/MathOps.cpp.obj
C:\PROGRA~2\MICROS~2\2022\BUILDT~1\VC\Tools\MSVC\1435~1.322\bin\Hostx64\x64\cl.exe  /nologo /TP -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -DMLIR_CUDA_CONVERSIONS_ENABLED=0 -DMLIR_ROCM_CONVERSIONS_ENABLED=0 -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_ENABLE_HARDENED_MODE -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -ID:\projs\llvm\llvm-build\tools\mlir\lib\Dialect\Math\IR -ID:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR -ID:\projs\llvm\llvm-build\include -ID:\projs\llvm\llvm-project\llvm\include -ID:\projs\llvm\llvm-project\mlir\include -ID:\projs\llvm\llvm-build\tools\mlir\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj /permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -w14062 -we4238 /Gw /O2 /Ob2 -std:c++17 -MD  /EHs-c- /GR- -UNDEBUG /showIncludes /Fotools\mlir\lib\Dialect\Math\IR\CMakeFiles\obj.MLIRMathDialect.dir\MathOps.cpp.obj /Fdtools\mlir\lib\Dialect\Math\IR\CMakeFiles\obj.MLIRMathDialect.dir\ /FS -c D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp
D:\projs\llvm\llvm-project\mlir\include\mlir/IR/BuiltinAttributes.h(359): warning C4996: 'std::complex<llvm::APFloat>::complex': warning STL4037: The effect of instantiating the template std::complex for any type other than float, double, or long double is unspecified. You can define _SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING to suppress this warning.
D:\projs\llvm\llvm-project\llvm\include\llvm/Support/Casting.h(657): error C2027: use of undefined type 'mlir::ub::PoisonAttr'
D:\projs\llvm\llvm-project\mlir\include\mlir/Dialect/CommonFolders.h(26): note: see declaration of 'mlir::ub::PoisonAttr'
D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp(526): note: see reference to function template instantiation 'decltype(auto) llvm::dyn_cast<mlir::ub::PoisonAttr,mlir::Attribute>(From &)' being compiled
        with
        [
            From=mlir::Attribute
        ]
D:\projs\llvm\llvm-project\llvm\include\llvm/Support/Casting.h(655): error C3487: 'void': all return expressions must deduce to the same type: previously it was 'To'
        with
        [
            To=mlir::ub::PoisonAttr
        ]
D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp(526): error C2027: use of undefined type 'mlir::ub::PoisonAttr'
D:\projs\llvm\llvm-project\mlir\include\mlir/Dialect/CommonFolders.h(26): note: see declaration of 'mlir::ub::PoisonAttr'
D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp(526): error C2027: use of undefined type 'mlir::ub::PoisonAttr'
D:\projs\llvm\llvm-project\mlir\include\mlir/Dialect/CommonFolders.h(26): note: see declaration of 'mlir::ub::PoisonAttr'
D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp(526): error C2451: a conditional expression of type 'To' is not valid
        with
        [
            To=mlir::ub::PoisonAttr
        ]
D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp(526): note: use of undefined type 'mlir::ub::PoisonAttr'
D:\projs\llvm\llvm-project\mlir\include\mlir/Dialect/CommonFolders.h(26): note: see declaration of 'mlir::ub::PoisonAttr'
D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp(527): error C2039: 'PoisonOp': is not a member of 'mlir::ub'
D:\projs\llvm\llvm-project\mlir\include\mlir/Dialect/CommonFolders.h(25): note: see declaration of 'mlir::ub'
D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp(527): error C2065: 'PoisonOp': undeclared identifier
D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp(527): error C2672: 'mlir::OpBuilder::create': no matching overloaded function found
D:\projs\llvm\llvm-project\mlir\include\mlir/IR/Builders.h(491): note: could be 'OpTy mlir::OpBuilder::create(mlir::Location,Args &&...)'
D:\projs\llvm\llvm-project\mlir\lib\Dialect\Math\IR\MathOps.cpp(527): note: 'mlir::OpBuilder::create': invalid template argument for 'OpTy', type expected
ninja: build stopped: subcommand failed.
Sep 5 2023, 1:37 PM · Restricted Project, Restricted Project
Hardcode84 added a comment to D159013: [mlir][ub] Add poison support to CommonFolders.h.

So, what the final decision?

Sep 5 2023, 5:25 AM · Restricted Project, Restricted Project

Aug 31 2023

Hardcode84 added a comment to D159013: [mlir][ub] Add poison support to CommonFolders.h.

@Hardcode84 : don't take this as blocking you from landing, but I'd still like to get to end of the argument with @zero9178 here, this is pretty important for any future direction about how we see UB in MLIR upstream (this "default" deferred UB question in particular may come up all the time!)

Aug 31 2023, 5:16 AM · Restricted Project, Restricted Project
Hardcode84 updated the diff for D159013: [mlir][ub] Add poison support to CommonFolders.h.

rebase, fix typo

Aug 31 2023, 5:12 AM · Restricted Project, Restricted Project

Aug 30 2023

Hardcode84 updated the diff for D159013: [mlir][ub] Add poison support to CommonFolders.h.

remove runtime dependecy on ub dialect

Aug 30 2023, 7:45 AM · Restricted Project, Restricted Project

Aug 29 2023

Hardcode84 added a comment to D159013: [mlir][ub] Add poison support to CommonFolders.h.

Implementation looks good to me, but I rather strongly believe this behaviour should be opt-in rather than opt-out. Otherwise you're by default redefining the semantics of all ops using these. Poison attributes are not the obvious way to handle undefined behaviour but rather a choice in the IR design.

Just for a list of users potentially impacted by opt-out, see: https://github.com/search?q=%22mlir%2FDialect%2FCommonFolders.h%22+language%3AC%2B%2B&type=code
This includes projects like Flang, ClangIR, Circt and IREE.

Aug 29 2023, 6:26 AM · Restricted Project, Restricted Project
Hardcode84 updated the diff for D159013: [mlir][ub] Add poison support to CommonFolders.h.

make poison opt-in

Aug 29 2023, 6:24 AM · Restricted Project, Restricted Project
Hardcode84 added inline comments to D159013: [mlir][ub] Add poison support to CommonFolders.h.
Aug 29 2023, 5:00 AM · Restricted Project, Restricted Project
Hardcode84 added a comment to D159013: [mlir][ub] Add poison support to CommonFolders.h.

Made PoisonAttr a template parameter as suggested but still left it on by default as I think in most cases this is a desired behavior. Replaced ub dialect include with forward decl, so any potential users will now need to add explicitly dependency on ub dialect or opt out to successfully compile.

Aug 29 2023, 4:50 AM · Restricted Project, Restricted Project
Hardcode84 updated the diff for D159013: [mlir][ub] Add poison support to CommonFolders.h.

make poison handling optional, add ub dialect as runtime dependency

Aug 29 2023, 4:44 AM · Restricted Project, Restricted Project

Aug 28 2023

Hardcode84 added a comment to D159013: [mlir][ub] Add poison support to CommonFolders.h.

I think the layering here is a bit problematic. The generic Dialect/CommonFolders.h suddenly depends on the UB dialect.
We are now also suddenly forcing the semantics of poisons on all users of these folders.
I am not sure what the best solution would be, but maybe versions/wrappers of those folders within the UB dialect? That way all op folders/dialects can also opt-into the poison behaviour. This also ensures correctness.

As a sidenote, since your new materializeConstant implementations may now create a ub.poison op, you need to add the UB dialect as a dialect dependency in the TableGen of the respective dialects as well.

Aug 28 2023, 2:29 PM · Restricted Project, Restricted Project
Hardcode84 added reviewers for D159013: [mlir][ub] Add poison support to CommonFolders.h: Mogball, ftynse, mehdi_amini, zero9178.
Aug 28 2023, 1:00 PM · Restricted Project, Restricted Project
Hardcode84 requested review of D159013: [mlir][ub] Add poison support to CommonFolders.h.
Aug 28 2023, 12:57 PM · Restricted Project, Restricted Project
Hardcode84 added a comment to D158986: [mlir][arith] Fold `select` with poison.

@zero9178

Note that this patch is the first to add a dependency on the UB dialect within Arith. Given this was inevitable (and part of the motivation) it should be fine I believe.

This is super exciting! Do you have some immediate plan for growing this set of folds?

Aug 28 2023, 10:34 AM · Restricted Project, Restricted Project
Hardcode84 accepted D158986: [mlir][arith] Fold `select` with poison.

LGTM

Aug 28 2023, 9:09 AM · Restricted Project, Restricted Project
Hardcode84 accepted D158982: [mlir][UBToLLVM] Do not arbitrarily restrict input types.

LGTM (FYI, we have a similar pattern in UBToSPIRV)

Aug 28 2023, 7:59 AM · Restricted Project, Restricted Project
Hardcode84 added a comment to D158986: [mlir][arith] Fold `select` with poison.

Which folding should applied first?

%val = select %true %poison %non_poison

or more complicated example

%1 = foldable_op %true
%2 = foldable_op %poison
%val = select %1 %2 %non_poison

Here we can end up with different code depending on order of folders.

Aug 28 2023, 7:42 AM · Restricted Project, Restricted Project
Hardcode84 added a comment to D158982: [mlir][UBToLLVM] Do not arbitrarily restrict input types.

My motivation for limiting this pattern was to avoid converting user types as user might have different lowering for poison. But it is purely theoretical, I don't have any specific examples.

Aug 28 2023, 7:29 AM · Restricted Project, Restricted Project

Aug 20 2023

Hardcode84 committed rG0db1ae3ed111: [mlir][CFGToSCF] Visit subregions in CFGToSCF pass (authored by Hardcode84).
[mlir][CFGToSCF] Visit subregions in CFGToSCF pass
Aug 20 2023, 10:36 AM · Restricted Project, Restricted Project
Hardcode84 closed D158349: [mlir][CFGToSCF] Visit subregions in CFGToSCF pass.
Aug 20 2023, 10:36 AM · Restricted Project, Restricted Project
Hardcode84 updated the diff for D158349: [mlir][CFGToSCF] Visit subregions in CFGToSCF pass.

improve error message and pass docs

Aug 20 2023, 5:29 AM · Restricted Project, Restricted Project
Hardcode84 added a comment to D158349: [mlir][CFGToSCF] Visit subregions in CFGToSCF pass.

So, I think if pass fails gracefully and we change error message to smth like Cannot create unreachable terminator for 'opname' it should be fine.

Aug 20 2023, 5:01 AM · Restricted Project, Restricted Project
Hardcode84 added a comment to D158349: [mlir][CFGToSCF] Visit subregions in CFGToSCF pass.

There is one concern that I have with this change, which is that if the subregion were to contain an infinite loop, forcing the creation of an unreachable terminator, the interface implementation would currently create a func.return with a poison value which IIRC is only allowed to be nested within a func.func.
Since the interface implementation cannot know (I think no such interface exists at the moment) what kind of terminator is required, it'd lead to invalid IR.
Example IR:

func.func private  @foo()

func.func @nested_region() {
  scf.execute_region {
    cf.br ^bb0
  ^bb0:
    func.call @foo() : () -> ()
    cf.br ^bb0
  }
  return
}

A solution to this would ideally be a ub.unreachable op. An interface or trait returning or helping build an appropiate terminator for a region op would also help but a lot more intrusive I imagine.

Aug 20 2023, 4:48 AM · Restricted Project, Restricted Project

Aug 19 2023

Hardcode84 updated the summary of D158349: [mlir][CFGToSCF] Visit subregions in CFGToSCF pass.
Aug 19 2023, 2:31 PM · Restricted Project, Restricted Project
Hardcode84 added reviewers for D158349: [mlir][CFGToSCF] Visit subregions in CFGToSCF pass: zero9178, definelicht, Dinistro.
Aug 19 2023, 2:29 PM · Restricted Project, Restricted Project
Hardcode84 requested review of D158349: [mlir][CFGToSCF] Visit subregions in CFGToSCF pass.
Aug 19 2023, 2:28 PM · Restricted Project, Restricted Project

Aug 13 2023

Hardcode84 added a comment to D156889: [mlir][cf] Add ControlFlow to SCF lifting pass.

@zero9178 I can make patches for subregion walk and exposing ControlFlowToSCFTransformation later this week.

Aug 13 2023, 3:49 PM · Restricted Project, Restricted Project, Restricted Project
Hardcode84 added a comment to D156889: [mlir][cf] Add ControlFlow to SCF lifting pass.

Ok, after lowering IndexSwitch to sequence of scf.if's all tests have passed. I guess, we are dropping our custom CFGToSCF and switching to upstream :)

Aug 13 2023, 5:46 AM · Restricted Project, Restricted Project, Restricted Project

Aug 12 2023

Hardcode84 added a comment to D156889: [mlir][cf] Add ControlFlow to SCF lifting pass.

Also, ControlFlowToSCFTransformation from pass impl probably should also be exposed, copypasted the entire thing for now.

Aug 12 2023, 5:07 PM · Restricted Project, Restricted Project, Restricted Project
Hardcode84 added a comment to D156889: [mlir][cf] Add ControlFlow to SCF lifting pass.
6 failed, 10990 passed, 3466 skipped, 445 xfailed, 46 warnings in 260.37s (0:04:20)

Much better, the rest if the failures seems to come from the fact we are not handling switch op correctly and not related to the pass directly.

Aug 12 2023, 5:06 PM · Restricted Project, Restricted Project, Restricted Project
Hardcode84 added a comment to D156889: [mlir][cf] Add ControlFlow to SCF lifting pass.

I used the pass https://github.com/numba/numba-mlir/pull/189
I'll try to hack recursive version on top of transformCFGToSCF and see what happens

Aug 12 2023, 4:36 PM · Restricted Project, Restricted Project, Restricted Project
Hardcode84 added a comment to D156889: [mlir][cf] Add ControlFlow to SCF lifting pass.

I run this pass on our python testsuite and results looking good

46 failed, 10950 passed, 3466 skipped, 445 xfailed, 46 warnings in 252.77s (0:04:12)

Look like most of the failures come from scf.execute_region, but I didn't looked in detail yet.
Example of failing IR https://gist.github.com/Hardcode84/8e55de4009e190af59cd2e7c25ebdbfa

Aug 12 2023, 4:12 PM · Restricted Project, Restricted Project, Restricted Project

Aug 10 2023

Hardcode84 committed rG793ee2bf0868: [mlir][gpu] Add DecomposeMemrefsPass (authored by Hardcode84).
[mlir][gpu] Add DecomposeMemrefsPass
Aug 10 2023, 1:31 PM · Restricted Project, Restricted Project
Hardcode84 closed D155247: [mlir][gpu] Add DecomposeMemrefsPass.
Aug 10 2023, 1:31 PM · Restricted Project, Restricted Project
Hardcode84 updated the diff for D155247: [mlir][gpu] Add DecomposeMemrefsPass.

move to IndexingUtils, do not use affine dialect

Aug 10 2023, 8:54 AM · Restricted Project, Restricted Project
Hardcode84 added a comment to D155247: [mlir][gpu] Add DecomposeMemrefsPass.

I can change computeLinearIndex to return an affineExpr + list of values so it will be user responsibility to pass it to makeComposedFoldedAffineApply. We won't need anything in Dialect/Affine/ViewLikeInterfaceUtils.h and interleaving and values order won't matter as long as user just forwards returned expr and values to makeComposedFoldedAffineApply.

Aug 10 2023, 5:38 AM · Restricted Project, Restricted Project
Hardcode84 added a comment to D155247: [mlir][gpu] Add DecomposeMemrefsPass.

Moved code to memrefUtils as it already have affine dependency. computeLinearIndex and getLinearizeMemRefAndOffset functions should be unified, but getLinearizeMemRefAndOffset is doing too many things at once and refactoring will be too big for unrelated patch.

Hmm not a fan of the landing place .. I suggest revisiting how you construct your AffineExpr using/extending IndexingUtils.
For instance, I am unclear why you need to interleave symbols/values for strides and indices.
Seems pretty easy to just use the following in IndexingUtils:

SmallVector<AffineExpr> computeElementwiseMul(ArrayRef<AffineExpr> v1, ArrayRef<AffineExpr> v2);
AffineExpr computeSum(MLIRContext *ctx, ArrayRef<AffineExpr> basis);

And build something resembling

OpFoldResult mlir::computeLinearIndex(OpBuilder &builder, Location loc,
                                      OpFoldResult sourceOffset,
                                      ArrayRef<OpFoldResult> strides,
                                      ArrayRef<OpFoldResult> indices) {
  assert(strides.size() == indices.size());
  auto sourceRank = static_cast<unsigned>(strides.size());

  // Hold the affine symbols and values for the computation of the offset.
  SmallVector<OpFoldResult> values{indices.begin(), indices.end()};
  llvm::append_range(values, strides);

  AffineExpr expr = computeTheAffineExprYouWantWithIndexingUtilsAndExtendThemAsNecessary();

  return affine::makeComposedFoldedAffineApply(builder, loc, expr, values);
}
Aug 10 2023, 5:09 AM · Restricted Project, Restricted Project

Aug 9 2023

Hardcode84 added a comment to D155247: [mlir][gpu] Add DecomposeMemrefsPass.

Moved code to memrefUtils as it already have affine dependency. computeLinearIndex and getLinearizeMemRefAndOffset functions should be unified, but getLinearizeMemRefAndOffset is doing too many things at once and refactoring will be too big for unrelated patch.

Aug 9 2023, 7:37 PM · Restricted Project, Restricted Project
Hardcode84 updated the diff for D155247: [mlir][gpu] Add DecomposeMemrefsPass.

moved to memrefutils

Aug 9 2023, 7:34 PM · Restricted Project, Restricted Project
Hardcode84 reopened D155247: [mlir][gpu] Add DecomposeMemrefsPass.
Aug 9 2023, 7:32 PM · Restricted Project, Restricted Project
Hardcode84 added a comment to D155247: [mlir][gpu] Add DecomposeMemrefsPass.

Reverted as it broke some bots.

Aug 9 2023, 6:11 PM · Restricted Project, Restricted Project
Hardcode84 added a reverting change for rG2b5b2bfef102: [mlir][gpu] Add DecomposeMemrefsPass: rGb13248f997dd: Revert "[mlir][gpu] Add DecomposeMemrefsPass".
Aug 9 2023, 6:08 PM · Restricted Project, Restricted Project
Hardcode84 committed rGb13248f997dd: Revert "[mlir][gpu] Add DecomposeMemrefsPass" (authored by Hardcode84).
Revert "[mlir][gpu] Add DecomposeMemrefsPass"
Aug 9 2023, 6:08 PM · Restricted Project, Restricted Project
Hardcode84 added a reverting change for D155247: [mlir][gpu] Add DecomposeMemrefsPass: rGb13248f997dd: Revert "[mlir][gpu] Add DecomposeMemrefsPass".
Aug 9 2023, 6:08 PM · Restricted Project, Restricted Project
Hardcode84 committed rG2b5b2bfef102: [mlir][gpu] Add DecomposeMemrefsPass (authored by Hardcode84).
[mlir][gpu] Add DecomposeMemrefsPass
Aug 9 2023, 5:28 PM · Restricted Project, Restricted Project
Hardcode84 closed D155247: [mlir][gpu] Add DecomposeMemrefsPass.
Aug 9 2023, 5:28 PM · Restricted Project, Restricted Project
Hardcode84 updated the diff for D155247: [mlir][gpu] Add DecomposeMemrefsPass.

rebase, review comments

Aug 9 2023, 4:58 PM · Restricted Project, Restricted Project
Hardcode84 added a comment to D155247: [mlir][gpu] Add DecomposeMemrefsPass.

ping

Aug 9 2023, 3:55 PM · Restricted Project, Restricted Project

Aug 7 2023

Hardcode84 accepted D156471: [mlir] Support fast-math friendly constants for identity value.

Thanks!

Aug 7 2023, 7:41 AM · Restricted Project, Restricted Project
Hardcode84 added inline comments to D156471: [mlir] Support fast-math friendly constants for identity value.
Aug 7 2023, 7:20 AM · Restricted Project, Restricted Project
Hardcode84 added inline comments to D156471: [mlir] Support fast-math friendly constants for identity value.
Aug 7 2023, 4:54 AM · Restricted Project, Restricted Project

Aug 3 2023

Hardcode84 added a comment to D156898: [mlir][memref] WIP - Reorganize conversions involving memref in the presence of backend-specific type conversions..

Does all this complexity really belongs to llvm conversion pass?
Maybe add a separate pass which decomposes memrefs index calculations into explicit ops (possibly inserting appropriate trunc casts depending on memory space) and then result is just straightforwardly converted to llvm.
As as bonus, you will be able to additional canonicalizations/transformations between this new pass and llvm conversion.

Aug 3 2023, 7:07 AM · Restricted Project, Restricted Project

Aug 2 2023

Hardcode84 added a comment to D156889: [mlir][cf] Add ControlFlow to SCF lifting pass.

Thanks for doing this! After this is merged I can try to run our e2e Python tests using pass.

Aug 2 2023, 12:15 PM · Restricted Project, Restricted Project, Restricted Project

Jul 31 2023

Hardcode84 added a comment to D155247: [mlir][gpu] Add DecomposeMemrefsPass.

ping

Jul 31 2023, 10:23 AM · Restricted Project, Restricted Project

Jul 29 2023

Hardcode84 added inline comments to D156471: [mlir] Support fast-math friendly constants for identity value.
Jul 29 2023, 5:53 PM · Restricted Project, Restricted Project

Jul 27 2023

Hardcode84 added a comment to D156061: [mlir][tensor] Improve verifiers: detect out-of-bounds accesses.

Which is still a valid program, but now will be wrongfully rejected by verifier

Consider following program

This is an interesting example. I'm not familiar with "poison". What does this mean for op verification? Does this mean that we should never verify anything? Can the same thing happen with things that the ExtractSliceOp/... verifiers are already checking at the moment?

-1, I would not call the transformed program valid. Local properties like non-ssa OOB indices should make ops invalid and fail in the verifier. And separately, any canonicalization/folds that would create invalid ops should instead either abort folding or replace them with poison/unreachable.

Jul 27 2023, 11:17 AM · Restricted Project, Restricted Project

Jul 26 2023

Hardcode84 added a comment to D156061: [mlir][tensor] Improve verifiers: detect out-of-bounds accesses.

Consider following program

%0 = ... tensor<5xf32>
%1 = tensor.cast %0 tensor<5xf32> to tensor<?xf32>
%dim = dim %1 0 : tensor<?xf32>
%cond = <nontrivial calculation depending on %dim>
%res = scf.if %cond {
  %2 = tensor.extract %1 [42] : tensor<?xf32>
  yield %2
} else {
  yield %default
}

which can be canonicalized to

%0 = ... tensor<5xf32>
%dim = dim %0 0 : tensor<?xf32>
%cond = <nontrivial calculation depending on %dim>
%res = scf.if %cond {
  %2 = tensor.extract %0 [42] : tensor<5xf32>
  yield %2
} else {
  yield %default
}

Which is still a valid program, but now will be wrongfully rejected by verifier
Moreover, when we detected invalid indices, it can be transformed further to

%0 = ... tensor<5xf32>
%dim = dim %0 0 : tensor<?xf32>
%cond = <nontrivial calculation depending on %dim>
%res = scf.if %cond {
  %2 = poison
  yield %2
} else {
  yield %default
}
Jul 26 2023, 11:21 AM · Restricted Project, Restricted Project
Hardcode84 committed rGc50f335ba556: [mlir][spirv] `memref.cast` to SPIR-V conversion (authored by Hardcode84).
[mlir][spirv] `memref.cast` to SPIR-V conversion
Jul 26 2023, 4:21 AM · Restricted Project, Restricted Project
Hardcode84 closed D156251: [mlir][spirv] `memref.cast` to SPIR-V conversion.
Jul 26 2023, 4:20 AM · Restricted Project, Restricted Project

Jul 25 2023

Hardcode84 updated the diff for D156251: [mlir][spirv] `memref.cast` to SPIR-V conversion.

negative tests

Jul 25 2023, 6:03 PM · Restricted Project, Restricted Project
Hardcode84 added inline comments to D156251: [mlir][spirv] `memref.cast` to SPIR-V conversion.
Jul 25 2023, 6:02 PM · Restricted Project, Restricted Project
Hardcode84 requested review of D156251: [mlir][spirv] `memref.cast` to SPIR-V conversion.
Jul 25 2023, 10:25 AM · Restricted Project, Restricted Project

Jul 24 2023

Hardcode84 added a comment to D156061: [mlir][tensor] Improve verifiers: detect out-of-bounds accesses.

I'm going to remove verification for offsets/sizes that are SSA values. According to the developer guide we should only verify static_sizes and static_offsets.

Jul 24 2023, 8:45 PM · Restricted Project, Restricted Project
Hardcode84 committed rG8568921d43b1: [mlir][spirv] Convert `ub.poison` to `spirv.undef` (authored by Hardcode84).
[mlir][spirv] Convert `ub.poison` to `spirv.undef`
Jul 24 2023, 3:30 PM · Restricted Project, Restricted Project
Hardcode84 closed D156163: [mlir][spirv] Convert `ub.poison` to `spirv.undef`.
Jul 24 2023, 3:30 PM · Restricted Project, Restricted Project
Hardcode84 updated the diff for D156163: [mlir][spirv] Convert `ub.poison` to `spirv.undef`.

vector negative test

Jul 24 2023, 3:19 PM · Restricted Project, Restricted Project
Hardcode84 updated the diff for D156163: [mlir][spirv] Convert `ub.poison` to `spirv.undef`.

review comments

Jul 24 2023, 2:59 PM · Restricted Project, Restricted Project
Hardcode84 added inline comments to D156163: [mlir][spirv] Convert `ub.poison` to `spirv.undef`.
Jul 24 2023, 2:55 PM · Restricted Project, Restricted Project
Hardcode84 added inline comments to D156163: [mlir][spirv] Convert `ub.poison` to `spirv.undef`.
Jul 24 2023, 2:41 PM · Restricted Project, Restricted Project
Hardcode84 requested review of D156163: [mlir][spirv] Convert `ub.poison` to `spirv.undef`.
Jul 24 2023, 12:37 PM · Restricted Project, Restricted Project
Hardcode84 committed rG0f446adf6782: [mlir] Convert `ub.poison` to `llvm.poison` (authored by Hardcode84).
[mlir] Convert `ub.poison` to `llvm.poison`
Jul 24 2023, 9:40 AM · Restricted Project, Restricted Project
Hardcode84 closed D155945: [mlir] Convert `ub.poison` to `llvm.poison`.
Jul 24 2023, 9:40 AM · Restricted Project, Restricted Project
Hardcode84 added inline comments to D154803: [mlir][GPU] Add gpu.create_queue op and add queue as an optional argument to gpu ops.
Jul 24 2023, 9:32 AM · Restricted Project, Restricted Project
Hardcode84 updated the diff for D155945: [mlir] Convert `ub.poison` to `llvm.poison`.

rebase

Jul 24 2023, 6:44 AM · Restricted Project, Restricted Project

Jul 22 2023

Hardcode84 added a comment to D154803: [mlir][GPU] Add gpu.create_queue op and add queue as an optional argument to gpu ops.

I don't think the discussion we had in the thread converged?

Jul 22 2023, 3:07 AM · Restricted Project, Restricted Project

Jul 21 2023

Hardcode84 added a comment to D154803: [mlir][GPU] Add gpu.create_queue op and add queue as an optional argument to gpu ops.

ping

Jul 21 2023, 1:40 PM · Restricted Project, Restricted Project
Hardcode84 added a comment to D155247: [mlir][gpu] Add DecomposeMemrefsPass.

ping

Jul 21 2023, 1:38 PM · Restricted Project, Restricted Project
Hardcode84 retitled D155945: [mlir] Convert `ub.poison` to `llvm.poison` from [mlir][arith] Convert `ub.poison` to `llvm.poison` for arith types to [mlir] Convert `ub.poison` to `llvm.poison`.
Jul 21 2023, 9:35 AM · Restricted Project, Restricted Project
Hardcode84 updated the diff for D155945: [mlir] Convert `ub.poison` to `llvm.poison`.

UBToLLVM conversion

Jul 21 2023, 9:35 AM · Restricted Project, Restricted Project
Hardcode84 added a comment to D155945: [mlir] Convert `ub.poison` to `llvm.poison`.

I was thinking of keeping poison-producing patterns in a separate populate function, at least initially.

Jul 21 2023, 7:24 AM · Restricted Project, Restricted Project
Hardcode84 added a comment to D155945: [mlir] Convert `ub.poison` to `llvm.poison`.

I think, the problem is more general here, as at some point populateArithToLLVM may become just unusable without populateUBToLLVM as it will be very hard for users not to trigger some poison-related folding accidentally. We can clarify this dependency via docs, but it is a quite weak guarantee.

Jul 21 2023, 6:50 AM · Restricted Project, Restricted Project
Hardcode84 added a comment to D155945: [mlir] Convert `ub.poison` to `llvm.poison`.

Why would it need to be implicitly included in the conversion?

Jul 21 2023, 6:24 AM · Restricted Project, Restricted Project
Hardcode84 added a comment to D155945: [mlir] Convert `ub.poison` to `llvm.poison`.

Integer and FP types are not arith types, they are builtin. I don't see a reason why we should add a dependency on UB dialect here. Feels like a separate UBToLLVM would offer a cleaner layering.

Jul 21 2023, 6:01 AM · Restricted Project, Restricted Project
Hardcode84 added a reviewer for D155945: [mlir] Convert `ub.poison` to `llvm.poison`: kuhar.
Jul 21 2023, 5:20 AM · Restricted Project, Restricted Project
Hardcode84 requested review of D155945: [mlir] Convert `ub.poison` to `llvm.poison`.
Jul 21 2023, 5:20 AM · Restricted Project, Restricted Project

Jul 20 2023

Hardcode84 committed rG9dec3fd81242: [mlir] Add `ub` dialect and `poison` op. (authored by Hardcode84).
[mlir] Add `ub` dialect and `poison` op.
Jul 20 2023, 2:20 AM · Restricted Project, Restricted Project
Hardcode84 closed D154248: [mlir] Add `ub` dialect and `poison` op..
Jul 20 2023, 2:19 AM · Restricted Project, Restricted Project

Jul 19 2023

Hardcode84 added a comment to D154839: [mlir][gpu] Add `gpu.alloc_managed`.

Also, do you have any specific usecase, which benefits from async allocs? It would be interesting to look at it.

Jul 19 2023, 3:29 PM · Restricted Project, Restricted Project
Hardcode84 added a comment to D154839: [mlir][gpu] Add `gpu.alloc_managed`.

The Op is designed to be async.

Jul 19 2023, 2:00 PM · Restricted Project, Restricted Project
Hardcode84 added a comment to D154839: [mlir][gpu] Add `gpu.alloc_managed`.

As I said, asynchronous allocation is too tied to specific runtime, the are other runtimes which doesn't support async allocations at all, for SYCL we are doing allocation synchronously regardless of async tokens passed (which may be suboptimal, but will still produce correct result), you can do the same (ignore stream if host_shared is passed). IMO, adding a new op in addition to host_shared just bloats the code and API (btw, your copypasted canonicalization still lacking a tests and won't actually work).

Jul 19 2023, 12:03 PM · Restricted Project, Restricted Project

Jul 18 2023

Hardcode84 updated the diff for D154248: [mlir] Add `ub` dialect and `poison` op..

fix more license headers

Jul 18 2023, 2:43 PM · Restricted Project, Restricted Project