fpetrogalli added a comment. In D137516#3912842 <https://reviews.llvm.org/D137516#3912842>, @lenary wrote:
> 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. +1 >> 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. Agree. >> 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. I'd be very happy to use the name `TargetParser`. I initially used that because I knew that moving the RSCV bits of it meant we needed to move the other parsers too, together with `Support/Host`. Therefore I was trying to create a name that would have covered for both the target parser any other pieces that would have been moved in it. However... > Edited to add: Once I've split out Support/Host, I'm likely to give it a less > generic name, something like "HostDetection", which is more clearly what it > is doing. ... you seem to imply that you want to create a second component that is independent of `[TargetParser|TargetSUpport]`. If that's the case, I am even more convinced that `TargetParser` is the best name for the component introduced in this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137516/new/ https://reviews.llvm.org/D137516 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
