[CIR][CIRGen] Move CIRGen types into clang::CIRGen #1082
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I scanned it. Makes sense to me tho we might miss something. If it passes tests I think it's reasonable. |
lanza
approved these changes
Nov 7, 2024
|
I'll wait for the upstream changes to get approved and then land this. |
smeenai
force-pushed
the
users/smeenai/cirgen-namespace
branch
from
c9c46ca to
f2ec5f2
Compare
smeenai
added a commit
that referenced
this pull request
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.
lanza
pushed a commit
that referenced
this pull request
Mar 18, 2025
#1025 explains why we want to move the CIR dialect from the `mlir::cir` to the `cir` namespace. To avoid overloading the `cir` namespace too much afterwards, move all symbols whose equivalents live inside the `clang::CodeGen` namespace to a new `clang::CIRGen` namespace, so that we match the original CodeGen's structure more closely. There's some symbols that live under `clang/include/clang/CIR` whose equivalents live in `clang/lib/CodeGen` and are in the `clang::CodeGen` namespace. We have these symbols in a common location since they're also used by lowering, so I've also left them in the `cir` namespace. Those symbols are: - AArch64ABIKind - ABIArgInfo - FnInfoOpts - TypeEvaluationKind - X86AVXABILevel This is a pretty large PR out of necessity. To make it slightly more reviewable, I've split it out into three commits (which will be squashed together when the PR lands): - The first commit manually switches places to the `clang::CIRGen` namespace. This has to be manual because we only want to move things selectively. - The second commit adjusts namespace prefixes to make builds work. I ran https://gist.github.com/smeenai/f4dd441fb61c53e835c4e6057f8c322f to make this change. The script is idempotent, and I added substitutions one at a time and reviewed each one afterwards (using `git diff --color-words=.`) to ensure only intended changes were being made. - The third commit runs `git clang-format`. Because I went one-by-one with all my substitutions and checked each one afterwards, I'm pretty confident in the validity of all the changes (despite the size of the PR).
lanza
pushed a commit
that referenced
this pull request
Mar 18, 2025
#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.
xlauko
pushed a commit
to trailofbits/instafix-llvm
that referenced
this pull request
Mar 28, 2025
llvm/clangir#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 llvm/clangir#1082 and the duplicated symbols noted in llvm/clangir#1025.
lanza
pushed a commit
to lanza/llvm-project
that referenced
this pull request
Aug 9, 2025
llvm/clangir#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 llvm/clangir#1082 and the duplicated symbols noted in llvm/clangir#1025.
lanza
pushed a commit
to lanza/llvm-project
that referenced
this pull request
Aug 11, 2025
llvm/clangir#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 llvm/clangir#1082 and the duplicated symbols noted in llvm/clangir#1025.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#1025 explains why we want to move
the CIR dialect from the
mlir::cirto thecirnamespace. To avoidoverloading the
cirnamespace too much afterwards, move all symbolswhose equivalents live inside the
clang::CodeGennamespace to a newclang::CIRGennamespace, so that we match the original CodeGen'sstructure more closely.
There's some symbols that live under
clang/include/clang/CIRwhoseequivalents live in
clang/lib/CodeGenand are in theclang::CodeGennamespace. We have these symbols in a common location since they're also
used by lowering, so I've also left them in the
cirnamespace. Thosesymbols are:
This is a pretty large PR out of necessity. To make it slightly more
reviewable, I've split it out into three commits (which will be squashed
together when the PR lands):
clang::CIRGennamespace. This has to be manual because we only want to move things
selectively.
ran https://gist.github.com/smeenai/f4dd441fb61c53e835c4e6057f8c322f
to make this change. The script is idempotent, and I added
substitutions one at a time and reviewed each one afterwards (using
git diff --color-words=.) to ensure only intended changes were beingmade.
git clang-format.Because I went one-by-one with all my substitutions and checked each one
afterwards, I'm pretty confident in the validity of all the changes
(despite the size of the PR).