lenary added a comment. In D137516#3912900 <https://reviews.llvm.org/D137516#3912900>, @fpetrogalli wrote:
> 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. The patch that outlines my proposal for TargetParser is here: https://reviews.llvm.org/D137838 (it has dependent patches). I also added a comment to your RFC thread explaining the dependent patches. 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
