Page MenuHomePhabricator

mscuttari (Michele Scuttari)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 13 2022, 8:53 AM (37 w, 6 d)

Recent Activity

Wed, Sep 28

mscuttari accepted D134814: [mlir] Use 'GEN_PASS_DECL' for pass declarations.
Wed, Sep 28, 10:52 AM · Restricted Project, Restricted Project
mscuttari added a comment to D134766: [mlir] Add macro for enabling all generated pass declarations.

Might be worth adding for consistency, but I don't have a use case for that at the moment. Typically I see the pass definitions for a group being spread across multiple files.

Wed, Sep 28, 8:42 AM · Restricted Project, Restricted Project
mscuttari accepted D134766: [mlir] Add macro for enabling all generated pass declarations.

LGTM. What about doing this also for the definitions?

Wed, Sep 28, 12:27 AM · Restricted Project, Restricted Project

Tue, Sep 27

mscuttari committed rGb1ce63bb5fdc: [MLIR] Migrate Arithmetic -> LLVM conversion pass to the auto-generated… (authored by mscuttari).
[MLIR] Migrate Arithmetic -> LLVM conversion pass to the auto-generated…
Tue, Sep 27, 1:08 PM · Restricted Project, Restricted Project
mscuttari closed D134752: [MLIR] Migrate Arithmetic -> LLVM conversion pass to the auto-generated constructor.
Tue, Sep 27, 1:08 PM · Restricted Project, Restricted Project
mscuttari updated the diff for D134752: [MLIR] Migrate Arithmetic -> LLVM conversion pass to the auto-generated constructor.

Easier constructor inheritance

Tue, Sep 27, 12:44 PM · Restricted Project, Restricted Project
mscuttari requested review of D134752: [MLIR] Migrate Arithmetic -> LLVM conversion pass to the auto-generated constructor.
Tue, Sep 27, 10:41 AM · Restricted Project, Restricted Project

Mon, Sep 26

mscuttari committed rG939c5cf6a7d9: [MLIR] Migrate MemRef -> LLVM conversion pass to the auto-generated constructor (authored by mscuttari).
[MLIR] Migrate MemRef -> LLVM conversion pass to the auto-generated constructor
Mon, Sep 26, 12:56 AM · Restricted Project, Restricted Project
mscuttari closed D134607: [MLIR] Migrate MemRef -> LLVM conversion pass to the auto-generated constructor.
Mon, Sep 26, 12:56 AM · Restricted Project, Restricted Project

Sun, Sep 25

mscuttari requested review of D134607: [MLIR] Migrate MemRef -> LLVM conversion pass to the auto-generated constructor.
Sun, Sep 25, 12:53 PM · Restricted Project, Restricted Project

Wed, Sep 7

mscuttari added a comment to D133384: [MLIR] Improve interaction of TypedValue with BlockAndValueMapping.

Thanks for addressing the issue

Wed, Sep 7, 12:12 AM · Restricted Project, Restricted Project

Tue, Sep 6

mscuttari added inline comments to D132010: [MLIR] Add mlir::TypedValue.
Tue, Sep 6, 1:19 PM · Restricted Project, Restricted Project
mscuttari added a comment to D132838: [MLIR] Update pass declarations to new autogenerated files.

Just go ahead and land this

Tue, Sep 6, 8:34 AM · Restricted Project, Restricted Project, Restricted Project

Sep 2 2022

mscuttari accepted D133215: Migrate "CheckUses" pass to the auto-generated constructor (NFC).
Sep 2 2022, 9:51 AM · Restricted Project, Restricted Project
mscuttari added inline comments to D133215: Migrate "CheckUses" pass to the auto-generated constructor (NFC).
Sep 2 2022, 9:36 AM · Restricted Project, Restricted Project
mscuttari added inline comments to D132010: [MLIR] Add mlir::TypedValue.
Sep 2 2022, 1:56 AM · Restricted Project, Restricted Project

Sep 1 2022

mscuttari added a comment to D132838: [MLIR] Update pass declarations to new autogenerated files.

@jpienaar when you are back, please accept the review so that I can then hopefully close it (no hurry, take your time :))
Part of the non-auto-closing issue is that I wrongly put by hand "Differential Review" instead of "Differential Revision", but still I have no option to close it manually

Sep 1 2022, 3:45 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari added a comment to D133027: [MLIR] Keep but deprecate old autogenerated pass base classes.

Thanks! Can you start a thread (tagged [PSA]) on discord announcing the deprecation and including the instructions for downstream to upgrade within X weeks? (anything between 2 to 6 weeks is likely fine)

Sep 1 2022, 3:42 AM · Restricted Project, Restricted Project

Aug 31 2022

mscuttari committed rGa22af3e1eb9e: [MLIR] Keep but deprecate old autogenerated pass base classes (authored by mscuttari).
[MLIR] Keep but deprecate old autogenerated pass base classes
Aug 31 2022, 2:39 PM · Restricted Project, Restricted Project
mscuttari closed D133027: [MLIR] Keep but deprecate old autogenerated pass base classes.
Aug 31 2022, 2:39 PM · Restricted Project, Restricted Project
mscuttari added inline comments to D132838: [MLIR] Update pass declarations to new autogenerated files.
Aug 31 2022, 8:19 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari requested review of D133027: [MLIR] Keep but deprecate old autogenerated pass base classes.
Aug 31 2022, 8:13 AM · Restricted Project, Restricted Project
mscuttari added inline comments to D132838: [MLIR] Update pass declarations to new autogenerated files.
Aug 31 2022, 8:00 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari added inline comments to D132838: [MLIR] Update pass declarations to new autogenerated files.
Aug 31 2022, 7:41 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari added a comment to D132838: [MLIR] Update pass declarations to new autogenerated files.

What should I do for the remaining ones (basically, all)? @rriddle was mentioning to open bugs and delegate them, but how should I proceed? One bug for each pass seems a lot to be done manually and would clutter the github issue page.

In general we operate per areas: that is it could be "per dialect". I would file a single bug for the entire project and then tag people for each as we go.
If you file the bug with the instructions (before/after and pointer to the doc) that would already be great!

Aug 31 2022, 5:47 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari added a comment to D132838: [MLIR] Update pass declarations to new autogenerated files.

Patch landed, all builds seem fine.
As I have interest in some specific passes for my projects, I will upload additional revisions updating their code to be coherent with what explained in the docs (that is, they need naming change and options usage).
What should I do for the remaining ones (basically, all)? @rriddle was mentioning to open bugs and delegate them, but how should I proceed? One bug for each pass seems a lot to be done manually and would clutter the github issue page. Maybe we should let people discover the change and wait for a volunteer to fix his area of interest? (joking, but not too much :D)

Aug 31 2022, 4:36 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari committed rG67d0d7ac0acb: [MLIR] Update pass declarations to new autogenerated files (authored by mscuttari).
[MLIR] Update pass declarations to new autogenerated files
Aug 31 2022, 3:43 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari updated the diff for D132838: [MLIR] Update pass declarations to new autogenerated files.

Fix formatting

Aug 31 2022, 2:19 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari updated the diff for D132838: [MLIR] Update pass declarations to new autogenerated files.

More conservative patch. Individual pass improvements are going to be handled by additional
patches.

Aug 31 2022, 1:19 AM · Restricted Project, Restricted Project, Restricted Project

Aug 30 2022

mscuttari added a reverting change for rG2be8af8f0e07: [MLIR] Update pass declarations to new autogenerated files: rG039b969b32b6: Revert "[MLIR] Update pass declarations to new autogenerated files".
Aug 30 2022, 1:22 PM · Restricted Project, Restricted Project, Restricted Project
mscuttari committed rG039b969b32b6: Revert "[MLIR] Update pass declarations to new autogenerated files" (authored by mscuttari).
Revert "[MLIR] Update pass declarations to new autogenerated files"
Aug 30 2022, 1:22 PM · Restricted Project, Restricted Project, Restricted Project
mscuttari added a comment to D132838: [MLIR] Update pass declarations to new autogenerated files.

Well, build bots are complaining about some problems.
https://lab.llvm.org/buildbot/#/builders/220/builds/4537
https://lab.llvm.org/buildbot/#/builders/61/builds/31581

Aug 30 2022, 1:04 PM · Restricted Project, Restricted Project, Restricted Project
mscuttari committed rG2be8af8f0e07: [MLIR] Update pass declarations to new autogenerated files (authored by mscuttari).
[MLIR] Update pass declarations to new autogenerated files
Aug 30 2022, 12:57 PM · Restricted Project, Restricted Project, Restricted Project
mscuttari added a comment to D132838: [MLIR] Update pass declarations to new autogenerated files.

Don't feel like you are pressured to do all of the follow up cleanups, please file bugs for things and we can delegate from there.

Aug 30 2022, 12:46 PM · Restricted Project, Restricted Project, Restricted Project
mscuttari added a comment to D132838: [MLIR] Update pass declarations to new autogenerated files.

Patch seems good from the build bots perspective. I've also dropped the old declarations, as they would not be needed anymore after this patch.
A lot of passes can be further updated to take advantage of the new infra (i.e. inheriting constructors, accepting options, removing redundant option structures), but I would like to separate this into additional and more self-contained patches, and possibly delegate them to more people as they need them to be implemented. My idea behind this patch is to just drop the old autogenerated files and adopt the new ones (patching the whole codebase to reflect the best practices is too much for a single person and sometimes would require discussing names, code organization, etc. which seems out of scope now).

Aug 30 2022, 6:35 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari updated the diff for D132838: [MLIR] Update pass declarations to new autogenerated files.

Fix formatting. Still I have to understand why clang-format doesn't run on some apparently random files.

Aug 30 2022, 5:15 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari updated the diff for D132838: [MLIR] Update pass declarations to new autogenerated files.
  • Drop old declarations
  • Fix some tests expecting the old pass names
  • Convert missing flang passes
  • Rebase to current upstream code
Aug 30 2022, 3:43 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari committed rG13ed6958df40: [MLIR] Unique autogenerated file for tablegen passes (authored by mscuttari).
[MLIR] Unique autogenerated file for tablegen passes
Aug 30 2022, 12:53 AM · Restricted Project, Restricted Project
mscuttari closed D132884: [MLIR] Unique autogenerated file for tablegen passes.
Aug 30 2022, 12:53 AM · Restricted Project, Restricted Project
mscuttari updated the diff for D132884: [MLIR] Unique autogenerated file for tablegen passes.

Fix formatting

Aug 30 2022, 12:20 AM · Restricted Project, Restricted Project
mscuttari updated the diff for D132884: [MLIR] Unique autogenerated file for tablegen passes.

Fix formatting

Aug 30 2022, 12:14 AM · Restricted Project, Restricted Project

Aug 29 2022

mscuttari requested review of D132884: [MLIR] Unique autogenerated file for tablegen passes.
Aug 29 2022, 1:28 PM · Restricted Project, Restricted Project
mscuttari committed rG0815281ff286: [MLIR] Fix autogenerated pass constructors linkage (authored by mscuttari).
[MLIR] Fix autogenerated pass constructors linkage
Aug 29 2022, 11:54 AM · Restricted Project, Restricted Project
mscuttari closed D132572: [MLIR] Fix autogenerated pass constructors linkage.
Aug 29 2022, 11:54 AM · Restricted Project, Restricted Project
mscuttari added a comment to D132838: [MLIR] Update pass declarations to new autogenerated files.

Right. I will land D132572, open a new review to merge .cpp.inc into .h.inc (just for passes, fixing all the infra also for types, etc. is a bit too much for me) and then update this patch.

Aug 29 2022, 10:39 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari updated the diff for D132838: [MLIR] Update pass declarations to new autogenerated files.

Update flang passes

Aug 29 2022, 9:57 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari added a comment to D132838: [MLIR] Update pass declarations to new autogenerated files.

Nice!

I love that we remove the need for PassDetail.h.

Since we guard things behind macros, could we generate everything in Passes.h.inc instead of the need of a separate TableGen invocation for Passes.cpp.inc? Is there an advantage to use both?

Aug 29 2022, 2:12 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari added a comment to D132838: [MLIR] Update pass declarations to new autogenerated files.

This patch follows D131839 and D132572. Changes of D132572 are also included in this patch, but they will disappear once that patch will land.

Aug 29 2022, 2:08 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari added a comment to D132572: [MLIR] Fix autogenerated pass constructors linkage.

I've updated the docs. Feel free to comment on them if you feel them not to be clear enough.
In the meanwhile, I've uploaded a first draft regarding the patches to the pass declarations, so we can start discussing about them: D132838

Aug 29 2022, 2:07 AM · Restricted Project, Restricted Project
mscuttari requested review of D132838: [MLIR] Update pass declarations to new autogenerated files.
Aug 29 2022, 2:06 AM · Restricted Project, Restricted Project, Restricted Project
mscuttari updated the diff for D132572: [MLIR] Fix autogenerated pass constructors linkage.

Update docs

Aug 29 2022, 1:58 AM · Restricted Project, Restricted Project

Aug 27 2022

mscuttari added a comment to D132572: [MLIR] Fix autogenerated pass constructors linkage.

@rriddle I'm sorry for the early ping, but I'm fixing the big patch regarding all the passes and I would like to know if the current namespace separation looks good. I'd like to avoid reiterating over all the passes just for this :D

Aug 27 2022, 11:48 AM · Restricted Project, Restricted Project

Aug 26 2022

mscuttari updated the diff for D132572: [MLIR] Fix autogenerated pass constructors linkage.

impl namespace for pass base class and friend create constructors

Aug 26 2022, 2:48 AM · Restricted Project, Restricted Project
mscuttari added a comment to D132572: [MLIR] Fix autogenerated pass constructors linkage.

Nice! Do you think we should add a namespace field to the tablegen pass def now? Seems like that would be consistent with everything else (and would remove potential user foot guns as well).

Aug 26 2022, 2:26 AM · Restricted Project, Restricted Project
mscuttari added a comment to D132572: [MLIR] Fix autogenerated pass constructors linkage.

@rriddle thanks for the suggestion, working flawlessly. I have uploaded your fix and removed the changes I was doing to the passes, which I will instead upload in a separate patch in order to avoid a single commit mixing different stuff.

Aug 26 2022, 12:21 AM · Restricted Project, Restricted Project
mscuttari updated the diff for D132572: [MLIR] Fix autogenerated pass constructors linkage.

Add createPassNameImpl methods

Aug 26 2022, 12:20 AM · Restricted Project, Restricted Project
mscuttari retitled D132572: [MLIR] Fix autogenerated pass constructors linkage from [MLIR] Split GEN_PASS_DEF to [MLIR] Fix autogenerated pass constructors linkage.
Aug 26 2022, 12:19 AM · Restricted Project, Restricted Project

Aug 25 2022

mscuttari added a comment to D132572: [MLIR] Fix autogenerated pass constructors linkage.

The issue is that the friend function in the class gets an inline linkage: it only gets emitted if it used in the translation unit.
I'm not sure if it can be made "not inline", usually you have to define it outside of the class but I don't know what the syntax would be here...

Aug 25 2022, 12:58 AM · Restricted Project, Restricted Project

Aug 24 2022

mscuttari updated the diff for D132572: [MLIR] Fix autogenerated pass constructors linkage.

Wrapping the base class in mlir namespace. It is indeed the correct way to go, but still the
problem persists.

Aug 24 2022, 11:29 AM · Restricted Project, Restricted Project
mscuttari added a comment to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

Can you send a draft revision that shows the issue?

Aug 24 2022, 10:05 AM · Restricted Project, Restricted Project
mscuttari added inline comments to D132572: [MLIR] Fix autogenerated pass constructors linkage.
Aug 24 2022, 10:03 AM · Restricted Project, Restricted Project
mscuttari added a comment to D132572: [MLIR] Fix autogenerated pass constructors linkage.

Compilation log (a bit trimmed, for clearness):

Aug 24 2022, 9:59 AM · Restricted Project, Restricted Project
mscuttari requested review of D132572: [MLIR] Fix autogenerated pass constructors linkage.
Aug 24 2022, 9:57 AM · Restricted Project, Restricted Project
mscuttari added a comment to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

@rriddle @mehdi_amini I'm updating my previous modifications to the existing passes but I've realized that the friend trick is not going to work, leading to undefined symbols at link time for what regard the createPassName methods. I suspect that the only solution is to split the GEN_PASS_DEF macro into something like GEN_PASS_DEF_BASECLASS_PASSNAME and GEN_PASS_DEF_CONSTRUCTOR_PASSNAME and include the .cpp.inc autogenerated file both before and after the actual class definition.
If that's ok, I will open a new review for this.

Aug 24 2022, 9:41 AM · Restricted Project, Restricted Project
mscuttari added a comment to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

Hi there,

This change is triggering a warning (which we treat as error) in one of our builds:

/usr/bin/clang++ -DMLIR_CUDA_CONVERSIONS_ENABLED=0 -DMLIR_INCLUDE_TESTS -DMLIR_ROCM_CONVERSIONS_ENABLED=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/__w/1/b/llvm/Release/tools/mlir/unittests/TableGen -I/__w/1/llvm-project/mlir/unittests/TableGen -I/__w/1/b/llvm/Release/include -I/__w/1/llvm-project/llvm/include -I/__w/1/llvm-project/mlir/include -I/__w/1/b/llvm/Release/tools/mlir/include -I/__w/1/llvm-project/mlir/unittests/TableGen/../../test/lib/Dialect/Test -I/__w/1/b/llvm/Release/tools/mlir/unittests/TableGen/../../test/lib/Dialect/Test -I/__w/1/llvm-project/llvm/utils/unittest/googletest/include -I/__w/1/llvm-project/llvm/utils/unittest/googlemock/include -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -Werror=mismatched-tags -O3 -DNDEBUG  -Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -UNDEBUG -Wno-suggest-override -std=c++17 -MD -MT tools/mlir/unittests/TableGen/CMakeFiles/MLIRTableGenTests.dir/PassGenTest.cpp.o -MF tools/mlir/unittests/TableGen/CMakeFiles/MLIRTableGenTests.dir/PassGenTest.cpp.o.d -o tools/mlir/unittests/TableGen/CMakeFiles/MLIRTableGenTests.dir/PassGenTest.cpp.o -c /__w/1/llvm-project/mlir/unittests/TableGen/PassGenTest.cpp
In file included from /__w/1/llvm-project/mlir/unittests/TableGen/PassGenTest.cpp:12:
In file included from /__w/1/llvm-project/llvm/utils/unittest/googlemock/include/gmock/gmock.h:58:
In file included from /__w/1/llvm-project/llvm/utils/unittest/googlemock/include/gmock/gmock-actions.h:53:
In file included from /__w/1/llvm-project/llvm/utils/unittest/googlemock/include/gmock/internal/gmock-internal-utils.h:50:
/__w/1/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:1526:11: error: comparison of integers of different signs: 'const unsigned int' and 'const int' [-Werror,-Wsign-compare]
  if (lhs == rhs) {
      ~~~ ^  ~~~
/__w/1/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:1553:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<unsigned int, int>' requested here
    return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
           ^
/__w/1/llvm-project/mlir/unittests/TableGen/PassGenTest.cpp:86:3: note: in instantiation of function template specialization 'testing::internal::EqHelper::Compare<unsigned int, int, nullptr>' requested here
  EXPECT_EQ(unwrap(pass)->getTestOption(), 57);
  ^
/__w/1/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:2027:54: note: expanded from macro 'EXPECT_EQ'
  EXPECT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
Aug 24 2022, 7:36 AM · Restricted Project, Restricted Project
mscuttari committed rGd467515602fc: [MLIR] Fix cast warning in pass tablegen test (authored by mscuttari).
[MLIR] Fix cast warning in pass tablegen test
Aug 24 2022, 7:36 AM · Restricted Project, Restricted Project
mscuttari committed rG32c5578bcddf: [MLIR] Split autogenerated pass declarations & C++ controllable pass options (authored by mscuttari).
[MLIR] Split autogenerated pass declarations & C++ controllable pass options
Aug 24 2022, 1:02 AM · Restricted Project, Restricted Project
mscuttari closed D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.
Aug 24 2022, 1:01 AM · Restricted Project, Restricted Project

Aug 23 2022

mscuttari updated the diff for D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

Address comments

Aug 23 2022, 2:20 PM · Restricted Project, Restricted Project

Aug 18 2022

mscuttari closed D131359: [MLIR] DynamicMemRefType: iteration and access by indices.

Closed by commit 8a10ee7590a91c9b2e5c80b52822a3a4c3af1a15
Forgot to add the review link this time :(

Aug 18 2022, 12:36 PM · Restricted Project, Restricted Project
mscuttari committed rG8a10ee7590a9: [MLIR] DynamicMemRefType: iteration and access by indices (authored by mscuttari).
[MLIR] DynamicMemRefType: iteration and access by indices
Aug 18 2022, 12:32 PM · Restricted Project, Restricted Project
mscuttari updated the diff for D131359: [MLIR] DynamicMemRefType: iteration and access by indices.

Removed wrong and useless explicit copy constructor from DynamicMemRefType

Aug 18 2022, 10:51 AM · Restricted Project, Restricted Project
mscuttari reopened D131359: [MLIR] DynamicMemRefType: iteration and access by indices.
Aug 18 2022, 10:00 AM · Restricted Project, Restricted Project
mscuttari added inline comments to rGb8ecf32f81bb: DynamicMemRefType: iteration and access by indices.
Aug 18 2022, 7:32 AM · Restricted Project, Restricted Project
mscuttari added inline comments to rGb8ecf32f81bb: DynamicMemRefType: iteration and access by indices.
Aug 18 2022, 6:39 AM · Restricted Project, Restricted Project
mscuttari updated the summary of D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.
Aug 18 2022, 4:46 AM · Restricted Project, Restricted Project
mscuttari added a comment to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

I've updated the documentation. Please give it a read and tell if it's not clear enough or if I missed something.
Also, I realized that the list options were not handled correctly. Now they generate an ArrayRef variable within the options struct. I've also added a list option within the test pass to check for the correct behaviour.

Aug 18 2022, 4:44 AM · Restricted Project, Restricted Project
mscuttari updated the diff for D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

Handle list option & Update documentation & Remove generated 'Pass' suffix

Aug 18 2022, 4:41 AM · Restricted Project, Restricted Project

Aug 17 2022

mscuttari added inline comments to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.
Aug 17 2022, 3:27 PM · Restricted Project, Restricted Project
mscuttari added inline comments to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.
Aug 17 2022, 3:03 PM · Restricted Project, Restricted Project
mscuttari added inline comments to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.
Aug 17 2022, 2:53 PM · Restricted Project, Restricted Project
mscuttari added inline comments to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.
Aug 17 2022, 2:41 PM · Restricted Project, Restricted Project
mscuttari added inline comments to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.
Aug 17 2022, 2:03 PM · Restricted Project, Restricted Project
mscuttari added inline comments to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.
Aug 17 2022, 1:55 PM · Restricted Project, Restricted Project
mscuttari added inline comments to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.
Aug 17 2022, 1:55 PM · Restricted Project, Restricted Project
mscuttari added a comment to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

This is missing documentation updates to PassManagement.md.

Aug 17 2022, 1:47 PM · Restricted Project, Restricted Project
mscuttari retitled D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options from [MLIR] Autogenerate struct and constructor to provide custom pass options to [MLIR] Split autogenerated pass declarations & C++ controllable pass options.
Aug 17 2022, 10:47 AM · Restricted Project, Restricted Project
mscuttari added inline comments to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.
Aug 17 2022, 10:47 AM · Restricted Project, Restricted Project
mscuttari updated the diff for D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

Address comments

Aug 17 2022, 10:46 AM · Restricted Project, Restricted Project
mscuttari updated subscribers of rGb8ecf32f81bb: DynamicMemRefType: iteration and access by indices.

@tjoerg Can you please explain how to reproduce the problem? Running the tests with the following cmake configuration doesn't make them failing: cmake -DENABLE_PROJECTS="clang;mlir;compiler-rt" -DLLVM_USE_SANITIZER="Undefined"

Aug 17 2022, 9:20 AM · Restricted Project, Restricted Project
mscuttari updated the diff for D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

Formatting fix. Somehow, this time git-clang-format didn't warn me (!?).

Aug 17 2022, 3:09 AM · Restricted Project, Restricted Project
mscuttari updated the diff for D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

The old pass base class declarations are emitted in order to preserve backward compatibility.
The test reflects the usage of the patched infrastructure.

Aug 17 2022, 2:34 AM · Restricted Project, Restricted Project
mscuttari added a comment to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

Wow! You didn't have to update all the passes to adopt this new feature from the get go.

Actually I tend to prefer to land the TableGen feature with the test dialect, and adopt it widely in one or multiple subsequent commits. This also help to show when something is backward compatible: can you clarify if this change is gonna be a breakage for all users (that is you had to update the entire codebase?).

Aug 17 2022, 12:44 AM · Restricted Project, Restricted Project

Aug 16 2022

mscuttari added a comment to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

I've introduced what we have been discussing yesterday. The flang build still needs to be fixed, but I would like to first reach consensus on the design.
An update wrt one of my previous answers: the definition of struct containing a pass options can now be obtained by including the header of the pass (e.g ConvertMemRefToLLVMPassOptions can be obtained by including mlir/Conversion/MemRefToLLVM/MemRefToLLVM.h)

Aug 16 2022, 1:49 PM · Restricted Project, Restricted Project
mscuttari updated the diff for D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

Introduced both the PASS_GEN_DECL_PassName and PASS_GEN_DEF_PassName. The former allows to import the options struct and the constructors (createPassNamePass() and createPassNamePass(const PassNamePassOptions &options)); these two methods are automatically declared in case of a missing constructor inside the pass TableGen. The latter allows to import the definition of the pass base class.
The declarations can be emitted through the option -gen-pass-decls, like it was before, while the definitions can be generated with the new option -gen-pass-defs.

Aug 16 2022, 1:45 PM · Restricted Project, Restricted Project

Aug 15 2022

mscuttari added a comment to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

So, just to confirm, do we want a GEN_PASS_PassName define that enables both the options struct and the pass base class? Shouldn't the base class be available only in the PassDetails.h header?

There are two GEN_PASS_PassName I mentioned: one for the public header and the other for the private implementation. The Base class goes in the private part.

Aug 15 2022, 1:58 PM · Restricted Project, Restricted Project
mscuttari added a comment to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

The options structs are declared inside the mlir/Conversion/Passes.h header. PassDetails.h includes only the pass base classes.
So, just to confirm, do we want a GEN_PASS_PassName define that enables both the options struct and the pass base class? Shouldn't the base class be available only in the PassDetails.h header? With a single define we either miss the public visibility of options (which makes all this useless) or have to accept the base classes having public visibility.

Aug 15 2022, 1:06 PM · Restricted Project, Restricted Project
mscuttari added a comment to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.
Aug 15 2022, 1:04 PM · Restricted Project, Restricted Project
mscuttari added a comment to D131839: [MLIR] Split autogenerated pass declarations & C++ controllable pass options.

I also see that these changes break flang. Maybe we should temporarily add an option inside the pass tablegen, in order to enable the structs generation, and just tell people to start adapting to this?

In general I rather have the "right" default and provide an opt-out flag: that let people adapt to it but when they opt out they at least do so conscientiously and can add a "todo" and/or a tracking bug for upgrading.

Aug 15 2022, 9:07 AM · Restricted Project, Restricted Project
mscuttari committed rGb8ecf32f81bb: DynamicMemRefType: iteration and access by indices (authored by mscuttari).
DynamicMemRefType: iteration and access by indices
Aug 15 2022, 8:39 AM · Restricted Project, Restricted Project