I'm not sure if it's worth defining these macros especially if this is the only place where they are used. We'd need a more coordinated effort to replace all 32's with named constants where it refers to a register size, and I'm not even sure if we could classify whether a given 32 is tied to a register size or not.

Replaced by a recent D75932.

It looks like @zbrid has already LGTMd this.

This obviously looks great to me, but I'll let someone else give the final approval.

The Hexagon changes LGTM.

Merged in https://reviews.llvm.org/rGc51b0bede82b.

This commit causes errors when linking mlir-opt (I'm not using shared libraries):

[...]
/w/bld/org/lib/libMLIRGPUtoCUDATransforms.a(ConvertKernelFuncToCubin.cpp.o): In function `(anonymous namespace)::GpuKernelToCubinPass::runOnOperation()':
ConvertKernelFuncToCubin.cpp:(.text._ZN12_GLOBAL__N_120GpuKernelToCubinPass14runOnOperationEv+0x44): undefined reference to `LLVMInitializeNVPTXTarget'
ConvertKernelFuncToCubin.cpp:(.text._ZN12_GLOBAL__N_120GpuKernelToCubinPass14runOnOperationEv+0x49): undefined reference to `LLVMInitializeNVPTXTargetInfo'
ConvertKernelFuncToCubin.cpp:(.text._ZN12_GLOBAL__N_120GpuKernelToCubinPass14runOnOperationEv+0x4e): undefined reference to `LLVMInitializeNVPTXTargetMC'
ConvertKernelFuncToCubin.cpp:(.text._ZN12_GLOBAL__N_120GpuKernelToCubinPass14runOnOperationEv+0x53): undefined reference to `LLVMInitializeNVPTXAsmPrinter'
ConvertKernelFuncToCubin.cpp:(.text._ZN12_GLOBAL__N_120GpuKernelToCubinPass14runOnOperationEv+0x60): undefined reference to `mlir::translateModuleToNVVMIR(mlir::Operation*)'
ConvertKernelFuncToCubin.cpp:(.text._ZN12_GLOBAL__N_120GpuKernelToCubinPass14runOnOperationEv+0x2d0): undefined reference to `llvm::MCTargetOptions::MCTargetOptions()'
clang-9: error: linker command failed with exit code 1 (use -v to see invocation)
tools/mlir/tools/mlir-opt/CMakeFiles/mlir-opt.dir/build.make:114: recipe for target 'bin/mlir-opt' failed
make[2]: *** [bin/mlir-opt] Error 1
make[2]: Target 'tools/mlir/tools/mlir-opt/CMakeFiles/mlir-opt.dir/build' not remade because of errors.
[...]

There is also the issue of warnings about the size of the generated string. This would cause -Werror builds to fail.

LGTM

And the vasrh one (that I missed) in e8d1578131247d089209952476ba9191ad0295be. Hexagon tests should now pass.

Fixed the Hexagon failures in 824b25fc02dc4544adae55e6451d355f4c6d7055.

There are custom implementations of these functions for 64-bit unsigned values, so you should make it uint64_t. There is no connection with the pointer size.

There is no guarantee that size_t will match the size of a pointer either. Why do you want that anyway?

I'm in favor of treating signed saturation as canonical. The issue in delaying detection of such cases to instruction selection is the volatility of the IR: there is no guarantee that the IR will remain in the same form (expected by isel) from one day to the next. For example, some optimization may decide to just promote the operations to the wider type and only do the extension/truncate once, depending on how many saturating operations may be near one another. Handling this variability in isel is just not feasible.

Looks good to me.

The Hexagon part looks ok.

To clarify: clang-8 hangs on HexagonInstrInfo.cpp when compiling current master branch with this patch applied.

This seems to hang when compiling HexagonInstrInfo.cpp.