User Details
- User Since
- Jan 13 2020, 1:53 AM (135 w, 12 h)
Fri, Aug 12
Thu, Aug 11
Wed, Aug 10
Mon, Aug 1
Thu, Jul 28
May 5 2022
I am currently out-of-office and don‘t have any access to my build machines. Therefore, I cannot test and confirm that the patch doesn‘t have unintended side effects. However, there is definitely a need to add a dependency to the tablgen generated header files. Sorry for missing this as part of https://reviews.llvm.org/D124851. I would suggest to take a look how this is done for MLIR dialects, where explicits deps to the generated targets are passed to add_mlir_dialect_library like in https://github.com/llvm/llvm-project/blob/04b419048955fc33718ba97e79f3558b6a27830e/mlir/lib/Dialect/EmitC/IR/CMakeLists.txt#L7-L9. It might not be necessary to touch AddLLVM and I assume the deps should rather be on the *IncGen target,
May 3 2022
The missing dep on MLIRIR actually showed up as a race condition in the build process on a GH runner.
Apr 28 2022
Apr 25 2022
Thanks for the review @jpienaar! I will address your comments before landing.
Updating D124002: [mlir][emitc] Disallow !emitc.opaque pointers
Apr 19 2022
Apr 13 2022
Apr 12 2022
Apr 11 2022
Mar 28 2022
Mar 23 2022
Mar 14 2022
Mar 11 2022
Please excuse if my previous reply was confusing. I checked more carefully and think these changes are good to go.
Mar 2 2022
Thanks for working on this Stella! Untangling the functions/marcos is a bit hard. Thus, I am mainly commenting to make sure I fully understand the patch :)
Feb 24 2022
Feb 23 2022
Address review comments
Feb 17 2022
As an example, the canonicalization pass would transform
func @myfunc() { %0 = "emitc.constant"() {value = 0 : i32} : () -> i32 %1 = "emitc.constant"() {value = 0 : i32} : () -> i32 %2 = emitc.apply "&"(%0) : (i32) -> !emitc.ptr<i32> %3 = emitc.apply "&"(%1) : (i32) -> !emitc.ptr<i32> emitc.call "write"(%2, %3) : (!emitc.ptr<i32>, !emitc.ptr<i32>) -> () return }
into
func @myfunc() { %0 = "emitc.constant"() {value = 0 : i32} : () -> i32 %1 = emitc.apply "&"(%0) : (i32) -> !emitc.ptr<i32> %2 = emitc.apply "&"(%0) : (i32) -> !emitc.ptr<i32> emitc.call "write"(%1, %2) : (!emitc.ptr<i32>, !emitc.ptr<i32>) -> () return }
resulting in pointer aliasing. Here, the variable op would be helpful.
Feb 14 2022
Feb 10 2022
Updating D119337: [mlir][emitc] Add a pointer type
@jpienaar I adressed your comments with the latest revision.
Updating D119337: [mlir][emitc] Add a pointer type
Feb 9 2022
Jan 26 2022
Updating D118154: [mlir] Don't emit unused labels
Jan 25 2022
Nov 8 2021
Oct 28 2021
I recently noticed that arith::ConstantOp support was added to the Cpp emitter with this commit. @jpienaar and @mehdi_amini, do we really want to add support for such ops to the printer? The alternative would be to add conversions, like the arith.constant to emitc.constant one which I have implemented in our mlir-emitc repo: https://github.com/iml130/mlir-emitc/blob/c3b7bf093417ecd50e4b246ffd85079dbd75e359/lib/Dialect/EmitC/Conversion/ArithToEmitC.cpp#L27-L38.
Sep 15 2021
LGTM. Thanks for improving the EmitC!
Sep 13 2021
Thanks for taking over!
Sep 10 2021
Sep 9 2021
Sep 5 2021
Sep 3 2021
Sep 2 2021
Aug 30 2021
Aug 26 2021
Refactor translation cmdline arguments
Aug 25 2021
Thanks for the review @jpienaar! I have updated our patch accordingly.
Address review comments
Aug 20 2021
With https://github.com/llvm/mlir-www/pull/83 merged (allowing to include custom docs in auto-generated ones) and me having finally had some time to write a first markdown documentation (we can still iterate and improve that one!) I would kindly ask @rriddle, @mehdi_amini and @jpienaar to re-review the patch :)
Add markdown documentation and dialect description
Jul 22 2021
Jul 10 2021
Jul 9 2021
Updating D104632: [mlir] Add Cpp emitter
Thanks for all review comments. Most comments should be addressed with the next revision. As discussed, the emitter's API is now significantly closed (only translateToCpp is accessible). The revision also drops the emitter's restriction to only generate C code. This is of course still possible and depends (as before) on the implemented conversions, but isn't checked in the emitter itself anymore. What the revision lacks is a dedicated markdown documentation. Suggestion to where to place such docs in the docs/ folder are welcome. So far the other targets don't have separate docs or should those live side-by-side to the EmitC dialect?