You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The lib/CIR/CodeGen/TargetInfo.cpp collectively includes the implementations of TargetInfo and ABIInfo for the both targets we currently support: x86_64 and AArch64. Soon I may try merging the SPIR-V target support for CIRGen, so a refactor of this source file seems reasonable.
This also makes the structure matches the Clang CodeGen. Proposed structure:
lib/CIR/CodeGen/TargetInfo.cpp only includes TargetCIRGenInfo and a dispatcher CIRGenModule::getTargetCIRGenInfo
lib/CIR/CodeGen/ABIInfo.cpp only includes cir::ABIInfo
lib/CIR/CodeGen/Targets/XXX.cpp implements several target factories with the following signature:
Discussion: do we need an implementation of DefaultTargetCIRGenInfo and DefaultABIInfo like clang did? The ABIInfo of SPIR-V indeed has a dependency on DefaultABIInfo. We shall make it another patch anyway.
The text was updated successfully, but these errors were encountered:
seven-mile
changed the title
Make TargetCIRGenInfo extensive to support more targets
Enhance the extensibility of TargetCIRGenInfo to support more targets
May 13, 2024
so a refactor of this source file seems reasonable.
+1
Discussion: do we need an implementation of DefaultTargetCIRGenInfo and DefaultABIInfo?
Sounds reasonable to me, we should have the skeleton as close as possible as to allow folks to make easier progress whenever adding things for new targets.
I think you should go ahead and make this refactoring, so you are not blocked. However, let me give you some remarks about some upcoming changes needed - in case that helps you somehow.
Part of the ABI information needs also to be available for LoweringPrepare and potentially other passes in the future and right now LoweringPrepare (lib/CIR/Transforms) doesn't have access to that information because it's in a sibling lib (lib/CIR/CodeGen) and doesn't really depend on it (see PR for vaargs and dynamic cast)
I'm a bit afraid that after you do this refactoring, we'd need to redesign some of it again; it's probably unfortunate to have to do this work twice. My rough sketch of the options are:
Move this information to a new place so it's reusable by both lib/CIR/Transforms and lib/CIR/CodeGen. The challenges here are that some of this ABI bits are currently coupled with things like CGM or CGF, and we'd have to break them.
Move lib/CIR/Transforms under lib/CIR/CodeGen/Transforms, this would still probably require breaking dep on CGM or CGF, but will likely make usual CIRGen stuff more accessible to transformations as well.
@sitio-couto is also gonna touch a lot of this soon. Can both of you chat/discuss/sync about an strategy that might lead to both of you taking advantage of each other's work?
The
lib/CIR/CodeGen/TargetInfo.cpp
collectively includes the implementations ofTargetInfo
andABIInfo
for the both targets we currently support:x86_64
andAArch64
. Soon I may try merging the SPIR-V target support for CIRGen, so a refactor of this source file seems reasonable.This also makes the structure matches the Clang CodeGen. Proposed structure:
lib/CIR/CodeGen/TargetInfo.cpp
only includesTargetCIRGenInfo
and a dispatcherCIRGenModule::getTargetCIRGenInfo
lib/CIR/CodeGen/ABIInfo.cpp
only includescir::ABIInfo
lib/CIR/CodeGen/Targets/XXX.cpp
implements several target factories with the following signature:std::unique_ptr<TargetCIRGenInfo> createXXXTargetCIRGenInfo(CIRGenModule &CGM);
Discussion: do we need an implementation of
DefaultTargetCIRGenInfo
andDefaultABIInfo
like clang did? TheABIInfo
of SPIR-V indeed has a dependency onDefaultABIInfo
. We shall make it another patch anyway.The text was updated successfully, but these errors were encountered: