[CIR][ABI][NFC] Make createLowerModule public
#734
Conversation
seven-mile
requested review from
sitio-couto
and removed request for
bcardosolopes and
lanza
| @@ -91,6 +91,8 @@ class LowerModule { | |||
| LogicalResult rewriteFunctionCall(CallOp callOp, FuncOp funcOp); | |||
| }; | |||
|
|
|||
| LowerModule createLowerModule(ModuleOp module, PatternRewriter &rewriter); | |||
There was a problem hiding this comment.
I added it to this internal header. If we want to use it in, let's say, LowerToLLVM.cpp, we still have to write:
#include "../../Dialect/Transforms/TargetLowering/LowerModule.h"But I'm fine with leaving the interim state somehow dirty. It also encourage the eager refactor.
There was a problem hiding this comment.
That's the problem with technical debt, we have a FIXME on the other dirty inclusion and now it seems encouraging to use that hack elsewhere :)
I'm fine with this NFC change but be aware that I'm not gonna approve using #include "../../Dialect/Transforms/TargetLowering/LowerModule.h" over LowerToLLVM.cpp, so we might need other PRs to look into putting those things in a proper location - note that they don't necessarily need to be public, but we could use cmake to pass in proper include dirs, etc.
| @@ -91,6 +91,8 @@ class LowerModule { | |||
| LogicalResult rewriteFunctionCall(CallOp callOp, FuncOp funcOp); | |||
| }; | |||
|
|
|||
| LowerModule createLowerModule(ModuleOp module, PatternRewriter &rewriter); | |||
There was a problem hiding this comment.
That's the problem with technical debt, we have a FIXME on the other dirty inclusion and now it seems encouraging to use that hack elsewhere :)
I'm fine with this NFC change but be aware that I'm not gonna approve using #include "../../Dialect/Transforms/TargetLowering/LowerModule.h" over LowerToLLVM.cpp, so we might need other PRs to look into putting those things in a proper location - note that they don't necessarily need to be public, but we could use cmake to pass in proper include dirs, etc.
Although currently LowerModule is not ready for formal usage, we need it for target-specific lowering to LLVM. This PR temporarily public the symbol `createLowerModule` to reuse the logic of preparing a `LowerModule`, making it easier for future refactor (making `TargetLoweringInfo` available for most stages in CIR Lowering).
Although currently LowerModule is not ready for formal usage, we need it for target-specific lowering to LLVM. This PR temporarily public the symbol `createLowerModule` to reuse the logic of preparing a `LowerModule`, making it easier for future refactor (making `TargetLoweringInfo` available for most stages in CIR Lowering).
Although currently LowerModule is not ready for formal usage, we need it for target-specific lowering to LLVM. This PR temporarily public the symbol `createLowerModule` to reuse the logic of preparing a `LowerModule`, making it easier for future refactor (making `TargetLoweringInfo` available for most stages in CIR Lowering).
Although currently LowerModule is not ready for formal usage, we need it for target-specific lowering to LLVM. This PR temporarily public the symbol `createLowerModule` to reuse the logic of preparing a `LowerModule`, making it easier for future refactor (making `TargetLoweringInfo` available for most stages in CIR Lowering).
Although currently LowerModule is not ready for formal usage, we need it for target-specific lowering to LLVM. This PR temporarily public the symbol `createLowerModule` to reuse the logic of preparing a `LowerModule`, making it easier for future refactor (making `TargetLoweringInfo` available for most stages in CIR Lowering).
Although currently LowerModule is not ready for formal usage, we need it for target-specific lowering to LLVM. This PR temporarily public the symbol
createLowerModuleto reuse the logic of preparing aLowerModule, making it easier for future refactor (makingTargetLoweringInfoavailable for most stages in CIR Lowering).