[PATCH] D137516: [TargetSupport] Move TargetParser API in a separate LLVM component.

Mon Nov 7 09:58:18 PST 2022

lenary added a comment.

In D137516#3912751 <https://reviews.llvm.org/D137516#3912751>, @fpetrogalli wrote:

> @arsenm @frasercrmck @lenary - thank you for the feedback
>
> 1. The library could host anything that needs to be auto-generated with tablegen. If we add `MVT`s, the name `TargetSupport` becomes obsolete.

I am in favour of splitting parts of Support that could be table-gen'd out of Support, and into supplemental libraries.

> 2. Therefore we could use a name like `AutoGenSupport` (or similar, I am open to suggestions).

I actually think this is an awful name, even worse than "Support". One of the big issues with "Support" is that it's a junk-drawer that anyone throws stuff in when they need it anywhere in LLVM, when their layering questions become more difficult. This then becomes very hard to undo, because everything at this layer depends on everything else. "AutoGenSupport" to me says "we now have two bits, the junk drawer, and the junk drawer that's auto generated" and so I feel this is is worse, as you cannot differentiate which you need based on what you're doing.

I think we need to name things after what they're for, not how they're made.

> 3. @lenary is saying that components like `Support/Host` needs to be moved together with the AArch64/ARM bits of the target parser. Do people see an issue if we add files that do not need auto-generated content in a library called `AutoGenSupport`?

The reason I'm in favour of "TargetParser" is because we already call the classes and related files "target parsers", and most of them contain target-related parsers. I don't know why this is a bad name.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137516/new/

https://reviews.llvm.org/D137516