Page MenuHomePhabricator

kdobros (Konrad Dobros)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 4 2020, 4:45 AM (7 w, 2 d)

Recent Activity

Aug 11 2020

kdobros added a comment to D85323: [mlir][LLVM] Add memref dialect cast.

I think best way would be to move some type conversions, that don't depend on convention, into dialect and reuse them in both StandardToLLVM and in verifier.

I disagree. The reason why we have lib/Conversion/AToB is that conversions depend on both A and B and we did not want to have dialect B depend on A. Putting type conversions in the dialect is the exact opposite of this goal. (Well, to be honest, standard types aren't a part of the standard dialect, but that's just a quirk of standard stuff). There aren't many other cases where type casts are inserted, and the LLVM dialect has always been the trailblazer in the area, but we need to find a cleaner way. We put the "cast" op into the LLVM because we did not want to have it in standard as it could be mistaken for some magic "cast any type to any other type" op with no verification. Maybe we should consider a separate "cast" dialect with pluggable rules, or a separate dialect that can depend on both LLVM and Standard, but not making LLVM depend on Standard (or any other dialect that may lower into LLVM, e.g. Linalg also has type conversions).

Aug 11 2020, 6:58 PM · Restricted Project
kdobros updated the diff for D85323: [mlir][LLVM] Add memref dialect cast.

Split emitted error into error and note

Aug 11 2020, 6:24 PM · Restricted Project

Aug 7 2020

kdobros added a comment to D85323: [mlir][LLVM] Add memref dialect cast.

Thanks for the review.

Aug 7 2020, 7:28 AM · Restricted Project
kdobros updated the diff for D85323: [mlir][LLVM] Add memref dialect cast.

Addressed review comments.
Rewrote tests to check only the verifier - roundtrip.mlir and invalid.mlir.
Reduced number of error messages.

Aug 7 2020, 7:11 AM · Restricted Project

Aug 6 2020

kdobros added a comment to D85196: [mlir][spirv] Add correct handling of Kernel and Addresses capabilities.

@mravishankar @antiagainst
I think I addressed all the comments.
If you think the change is still good as is, could you commit on my behalf? I don't have commit access.
Name and email: Konrad Dobros <konrad.dobros@intel.com>

Aug 6 2020, 4:46 PM · Restricted Project
kdobros updated the diff for D85196: [mlir][spirv] Add correct handling of Kernel and Addresses capabilities.

Addressed comments.
Refactored not needed CHECKs in .mlir tests.
Brought back FailureOr, as per comment it is preferred over Optional.

Aug 6 2020, 4:38 PM · Restricted Project
kdobros updated the diff for D85196: [mlir][spirv] Add correct handling of Kernel and Addresses capabilities.

Addressed review comments.
Minor style fixes - added explicit types if they don't hurt readability.

Aug 6 2020, 11:19 AM · Restricted Project
kdobros added inline comments to D85196: [mlir][spirv] Add correct handling of Kernel and Addresses capabilities.
Aug 6 2020, 7:55 AM · Restricted Project

Aug 5 2020

kdobros requested review of D85323: [mlir][LLVM] Add memref dialect cast.
Aug 5 2020, 9:29 AM · Restricted Project
kdobros added a comment to D85196: [mlir][spirv] Add correct handling of Kernel and Addresses capabilities.

@mravishankar @antiagainst
Thank you for the reviews, I have made some changes addressing your comments.

Aug 5 2020, 5:17 AM · Restricted Project
kdobros updated the diff for D85196: [mlir][spirv] Add correct handling of Kernel and Addresses capabilities.

Separated functions to query execution environment related information
into ExecutionEnvironment{.h,.cpp}.
Removed generation of spv.interface_var_abi if it's not needed.
Fixed missing newline.

Aug 5 2020, 4:54 AM · Restricted Project

Aug 4 2020

kdobros requested review of D85196: [mlir][spirv] Add correct handling of Kernel and Addresses capabilities.
Aug 4 2020, 5:25 AM · Restricted Project