[CIR][ABI][NFC] Add CC lowering for void CallOps #668
Conversation
This patch implements the lowering of function calls that receive and return void. In practice, nothing has to be done (at least for the x86 ABI), so this case is used as a primer for the target lowering library since it helps populate the base logic for handling calling convention lowering of function calls without adding too much complexity.
There was a problem hiding this comment.
Some nits but overall good, thanks!
Please try not to duplicate things if they already exist in CIR, take the opportunity to unify/refactor them, it will make landing your PRs faster - most of the comments are very similar to the ones I added in the previous PR.
Btw, I thought we had an agreement about the next PR being about merging the existing ABI lowering logic to the stuff you're adding, can you please clarify the plans here?
clang/lib/CIR/Dialect/Transforms/TargetLowering/Targets/X86.cpp
Outdated
Show resolved
Hide resolved
|
@bcardosolopes applied and answered.
I understood that we would just move all ABI stuff to the same for now and unify them later. Regardless, it would be better to prime the library first and unify the ABI logic later to simplify the process.
My plan was to leave it as is until I have reached structs/pointers in the calling convention lowering pass. After that, I would start unifying the ABI logic updating stuff like VAArg lowering to follow codegen parity. |
My problem with that is if another PR shows up that needs to touch that part, we're gonna be creating more techinical debt. In that case we're gonna have to block those on top of your work, and I'd appreciate then if you are the one during the review and making sure the skeleton is appropriate, does that work for you? |
|
@bcardosolopes applied. |
This reverts commit 901f532. Revert as a prereq to revert #668 This broke the build: ``` /usr/bin/ld: lib/libMLIRCIRTransforms.a(LoweringPrepare.cpp.o): in function `(anonymous namespace)::LoweringPreparePass::lowerVAArgOp(mlir::cir::VAArgOp)': /local/home/dolsen/clangir/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp:340: undefined reference to `cir::CIRDataLayout::CIRDataLayout(mlir::ModuleOp)' /usr/bin/ld: lib/libTargetLowering.a(LowerTypes.cpp.o): in function `cir::CIRDataLayout::CIRDataLayout(llvm::StringRef, mlir::ModuleOp)': /local/home/dolsen/clangir/clang/include/clang/CIR/Dialect/IR/CIRDataLayout.h:31: undefined reference to `cir::CIRDataLayout::reset(llvm::StringRef)'`' ``` Steps to reproduce: ``` $ cmake -GNinja -DCMAKE_INSTALL_PREFIX=/<some-path>/clangir-install "-DLLVM_ENABLE_PROJECTS=clang;mlir" -DCLANG_ENABLE_CIR=ON -DLLVM_TARGETS_TO_BUILD=host -DCMAKE_BUILD_TYPE=Debug ../clangir/llvm $ ninja install ```
This reverts commit 66bb15b. This broke the build: ``` /usr/bin/ld: lib/libMLIRCIRTransforms.a(LoweringPrepare.cpp.o): in function `(anonymous namespace)::LoweringPreparePass::lowerVAArgOp(mlir::cir::VAArgOp)': /local/home/dolsen/clangir/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp:340: undefined reference to `cir::CIRDataLayout::CIRDataLayout(mlir::ModuleOp)' /usr/bin/ld: lib/libTargetLowering.a(LowerTypes.cpp.o): in function `cir::CIRDataLayout::CIRDataLayout(llvm::StringRef, mlir::ModuleOp)': /local/home/dolsen/clangir/clang/include/clang/CIR/Dialect/IR/CIRDataLayout.h:31: undefined reference to `cir::CIRDataLayout::reset(llvm::StringRef)'`' ``` Steps to reproduce: ``` $ cmake -GNinja -DCMAKE_INSTALL_PREFIX=/<some-path>/clangir-install "-DLLVM_ENABLE_PROJECTS=clang;mlir" -DCLANG_ENABLE_CIR=ON -DLLVM_TARGETS_TO_BUILD=host -DCMAKE_BUILD_TYPE=Debug ../clangir/llvm $ ninja install ```
This patch implements the lowering of function calls that receive and return void. In practice, nothing has to be done (at least for the x86 ABI), so this case is used as a primer for the target lowering library since it helps populate the base logic for handling calling convention lowering of function calls.
This patch implements the lowering of function calls that receive and return void. In practice, nothing has to be done (at least for the x86 ABI), so this case is used as a primer for the target lowering library since it helps populate the base logic for handling calling convention lowering of function calls.
This patch implements the lowering of function calls that receive and return void. In practice, nothing has to be done (at least for the x86 ABI), so this case is used as a primer for the target lowering library since it helps populate the base logic for handling calling convention lowering of function calls.