Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CIR] Merge the mlir::cir namespace into cir #1084

Merged
merged 6 commits into from
Nov 8, 2024

Conversation

smeenai
Copy link
Collaborator

@smeenai smeenai commented Nov 8, 2024

#1025 explains why we want to move
the CIR dialect from the mlir::cir to the cir namespace.

This is a large PR, and I've split it out into four commits (that'll be
squashed when landing). The first commit changes mlir::cir to cir
everywhere. This was originally done mechanically with:

find clang \( -name '*.h' -o -name '*.cpp' -o -name '*.td' \) -print0 | xargs -0 perl -pi -e 's/mlir::cir/cir/g'
find clang \( -name '*.h' -o -name '*.cpp' \) -print0 | xargs -0 perl -pi -e 's/::cir/cir/g'
find clang \( -name '*.h' -o -name '*.cpp' \) -print0 | xargs -0 perl -0777 -pi -e 's/namespace mlir \{\nnamespace cir \{/namespace cir {/g'
find clang \( -name '*.h' -o -name '*.cpp' \) -print0 | xargs -0 perl -0777 -pi -e 's!\} // namespace cir\n\} // namespace mlir!} // namespace cir!g'

It then required some manual fixups to address edge cases.

Code that lived under mlir::cir could refer to the mlir namespace
without qualification, but after the namespace change, we need to
explicitly qualify all our usages. This is done in the second commit via
https://gist.github.com/smeenai/996200fd45ad123bbf22b412d59479b6, which
is an idempotent script to add all qualifications. I added cases to the
script one at a time and reviewed each change afterwards to ensure we
were only making the intended modifications, so I feel pretty confident
in the end result. I also removed using namespace llvm from some
headers to avoid conflicts, which in turn required adding some llvm::
qualifiers as well.

The third commit fixes a test, since an error message now contains the
mlir namespace. Similar tests in flang also have the namespace in their
error messages, so this is an expected change.

The fourth change runs git clang-format. Unfortunately, that doesn't
work for TableGen files, so we'll have a few instances of undesirable
formatting left there. I'll look into fixing that as a follow-up.

I validated the end result by examining the symbols in the built Clang
binary. There's nothing in the mlir::cir namespace anymore.
https://gist.github.com/smeenai/8438fd01588109fcdbde5c8652781dc0 had the
symbols which lived in cir and should have moved to clang::CIRGen,
and I validated that all the symbols were moved, with the exceptions
noted in #1082 and the duplicated
symbols noted in #1025.

@smeenai
Copy link
Collaborator Author

smeenai commented Nov 8, 2024

I'll wait for the upstream changes to get approved and then land this.

@smeenai smeenai force-pushed the users/smeenai/cirgen-namespace branch from c9c46ca to f2ec5f2 Compare November 8, 2024 18:45
Base automatically changed from users/smeenai/cirgen-namespace to main November 8, 2024 18:46
@smeenai smeenai force-pushed the users/smeenai/mlir-cir-namespace-merge branch from 0d8a2c3 to 0a2410b Compare November 8, 2024 18:53
    find clang \( -name '*.h' -o -name '*.cpp' -o -name '*.td' \) -print0 | xargs -0 perl -pi -e 's/mlir::cir/cir/g'
    find clang \( -name '*.h' -o -name '*.cpp' \) -print0 | xargs -0 perl -pi -e 's/::cir/cir/g'
    find clang \( -name '*.h' -o -name '*.cpp' \) -print0 | xargs -0 perl -0777 -pi -e 's/namespace mlir \{\nnamespace cir \{/namespace cir {/g'
    find clang \( -name '*.h' -o -name '*.cpp' \) -print0 | xargs -0 perl -0777 -pi -e 's!\} // namespace cir\n\} // namespace mlir!} // namespace cir!g'
@smeenai smeenai force-pushed the users/smeenai/mlir-cir-namespace-merge branch from 0a2410b to 9d758ef Compare November 8, 2024 18:55
@smeenai smeenai merged commit 0c77b27 into main Nov 8, 2024
6 checks passed
@smeenai smeenai deleted the users/smeenai/mlir-cir-namespace-merge branch November 8, 2024 18:56
keryell added a commit to keryell/mlir-aie that referenced this pull request Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants