Why is this necessary? The return type is always index, ODS should not how to build it. If something is not working without this, it is probably a missing ODS functionality, we shouldn't need to write such boilerplate.
Nit: put double indentation for function arguments, otherwise it's hard to differentiate them from the body.
Nit: drop trivial braces.
Please avoid magic numbers.
TypeRange should be implicitly constructible from a single type.
.getResult(0) looks shorter.
Please avoid nested calls to Builder::create. Beyond being harder to read, it leads to a platform-dependent order of operations since the order of evaluation for function call arguments is implementation-defined in C++. I suspect this is the reason why the test is broken on Windows (makeSwitch creates operations and so does the other argument of MulFOp).
Nit: why do we even emit this conditional if we know the condition?
Please avoid matching SSA value names, even for constants.
I'd prefer to see : type($result) here, even if ODS can infer it, explicit typing makes it easier to read, and if ODS can't do it now then inferReturnTypes is too much.
nit: empty line before the next comment
For numWorkerThreads == 0 this pass should be no-op, otherwise I assume it will crash with some division by zero
Can you send this change in a separate PR, not to mix it with straightforward async runtime change
Ah, just realized that numWorkerThreads is no longer a constant.
Instead of generating nested scf.if you can do that with a short number of cmpi and select, I think it will be easier to match in tests and read in mlir dumps, and no need to to recursive builders, just a loop over brackets will do the work
%threads = async.runtime.num_threads %init = constant 8.0 : float // (0, 4] %p4 = cmpi sgt %threads %c4 // num_threads > 4 %p8 = cmpi sgt %threads %c8 // num_threads > 8 %p16 = cmpi sgt %threads %c16 // num_threads > 16 %p32 = cmpi sgt %threads %c32 // num_threads > 32 %p64 = cmpi sgt %threads %c64 // num_threads > 64 %s0 = select %p4 %c4.0 %c8.0 %s1 = select %p8 %c2.0 %s0 %s2 = select %p16 %c1.0 %s1 %s3 = select %p32 %c0.8 %s2 %scale = select %p64 %c0.6 %s3
Revert changes to Bazel file because it doesn't belong in this CL.
I was confused about interfaces: I didn't realize that merely importing the interface module is sufficient for it to take effect. I thought that is is necessary to use it as a trait on an op.
Definition of maxComputeBlock prevents that. I'm not changing logic in this regard. I suppose an early exit is possible, but I'd argue it belongs in a separate PR.
I'm assuming this is withdrawn.
Obsolete. FWIW, makeSwitch needed to be passed by copy, because it was supposed to replace past self, so I opted for maximum explicitness. But it's moot point now.
You can include "mlir/Support/LLVM.h" to bring APFloat into mlir namespace (for consistency with other mlir files), also a separate makeFloat doesn't make much sense now, its used just twice, and "creating custom mini DSLs" is frowned upon in MLIR
auto *runtime = getDefaultAsyncRuntime(); runtime->getThreadPool()-> <get the real number of worker threads> ``` (snippet from `mlirAsyncRuntimeExecute`
I get this error locally:
AILED: lib/libMLIRAsync.so.14git : && /usr/bin/c++ -fPIC -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wmisleading-indentation -fdiagnostics-color -g -Wl,-z,defs -Wl,-z,nodelete -Wl,-rpath-link,/usr/local/google/home/ezhulenev/projects/llvm-project/build/./lib -shared -Wl,-soname,libMLIRAsync.so.14git -o lib/libMLIRAsync.so.14git tools/mlir/lib/Dialect/Async/IR/CMakeFiles/obj.MLIRAsync.dir/Async.cpp.o -Wl,-rpath,"\$ORIGIN/../lib" lib/libMLIRDialect.so.14git lib/libMLIRIR.so.14git lib/libMLIRSupport.so.14git -lpthread lib/libLLVMSupport.so.14git -Wl,-rpath-link,/usr/local/google/home/ezhulenev/projects/llvm-project/build/lib && : /usr/bin/ld: tools/mlir/lib/Dialect/Async/IR/CMakeFiles/obj.MLIRAsync.dir/Async.cpp.o: in function `mlir::detail::InferTypeOpInterfaceTrait<mlir::async::AddToGroupOp>::verifyTrait(mlir::Operation*)': /usr/local/google/home/ezhulenev/projects/llvm-project/build/tools/mlir/include/mlir/Interfaces/InferTypeOpInterface.h.inc:141: undefined reference to `mlir::detail::verifyInferredResultTypes(mlir::Operation*)'
I think CMakeLists has to be updated to be consistent with BUILD.bazel, though it seems that InferTypeOpInterface is no longer used and can be deleted