[PATCH] D53407: [llvm-mca] Move namespace mca inside llvm::

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In https://reviews.llvm.org/D53407#1278719, @andreadb wrote:

> In https://reviews.llvm.org/D53407#1274525, @MaskRay wrote:
>
> > > I am okay with this change provided that you only change files in llvm-mca/include or in llvm-mca/lib.
> >
> > Why is the difference between `llvm-mca/include/*` and `llvm-mca/Views/*`?
> >
> > If `llvm-mca/Views/*` names want to stay in `namespace mca`, as they reference `llvm-mca/include/*`, they need to be written as:
> >
> >   namespace mca {
> >   using namespace llvm::mca;
> >
> >
> > or use qualified `llvm::mca::*` (which is clumsy)
> >
> > Having both `namespace mca` and `namespace llvm::mca` will make it easy to cause name conflict.
>
>
> I am okay with having the core abstractions defined inside namespace llvm::mca.
>  I am not a big fan of having all the driver components (like views) defined in namespace llvm::mca too. If the ultimate goal is just to remove a bunch of "using namespace" from a few files, then this patch looks like a too big change for very little gain.
>
> That being said. I am not strongly against this change. So, I spoke with other people about your patch. We definitely want to move the core abstractions of mca inside namespace llvm; that would simplify the transition from the tools/mca directory to llvm/lib. We all agree that (at least conceptually) it is much nicer if driver abstractions don't live inside of namespace llvm. However, nobody felt strongly about it either.
>
> In conclusion. It is fine if you want to move everything under llvm::mca. However, we may revisit this decision in future.
>
> Please rebase your original patch, and remove all the redundant llvm:: prefixes from header files when possible.
>
> Thanks,
> Andrea


Thanks for the feedback.

Rebased. I decide to go with the easy route: moving the whole namespace mca inside llvm:: , the justification is

- Having both llvm::mca mca cause namespace lookup ambiguity for `mca::`
- In driver components' header files, I would otherwise use `llvm::mca::*` (core abstractions) here and there

When we want to revisit the decision (everything in `llvm::mca::*`) in the future, we can move things to a nested namespace of `llvm::mca::`, to conceptually make them separate from the rest of `llvm::mca::*`


Repository:
  rL LLVM

https://reviews.llvm.org/D53407