rjmccall added a comment. In D96802#2569810 <https://reviews.llvm.org/D96802#2569810>, @varungandhi-apple wrote:
>> The TargetInfo classes are already target-architecture-specific, so it's >> somewhat strange for them all to funnel to a single function that then >> immediately switches on the target architecture. > > IMO ideally, the whole thing would be stored as a table of Target x > CallingConvention. Each entry of the table would have a > CallingConvCheckResult. That way, you can quickly figure out both: > > - which targets is a calling convention supported for > - what calling conventions are supported for a particular target > > Certainly, that's not the state of the code right now. So I do agree with > your point that it looks a bit strange. However, given that things are > somewhat in flux for `CC_SwiftAsync`, I think it is valuable to centralize > the information for it in one place instead of spreading it out over multiple > files. I don't see why. I can't really imagine adding a second dimension to the decision here — there's no reason to ever make this OS-dependent, for example — and so the only likely evolution is adding support for a new architecture, which means adding another case to your centralized function instead of just changing how the CC is handled in the target-specific function. And for this patch, just implementing the new case as `return CCCR_Error` seems much more obvious than calling a centralized function which just switches out again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96802/new/ https://reviews.llvm.org/D96802 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
