-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support LLVM backend #171
Support LLVM backend #171
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, this is a great PR that makes significant progress, and I enjoyed reviewing it. As I am not familiar with the LLVM APIs, there's not much I can support. However, as all test cases have passed, I believe there are no major issues. Small ones can be found and addressed as we evolve in the future. 🔥
I have submitted some minor revisions that I hope will make the PR even better. 😄
include/llvm_ir_generator.hpp
Outdated
/// @brief A LLVM object that includes core LLVM infrastructure. | ||
std::unique_ptr<llvm::LLVMContext> context_; | ||
/// @brief Provides LLVM Builder API for constructing IR. By default, Constant | ||
/// folding is enabled and we have more flexibility for inserting | ||
/// instructions. | ||
std::unique_ptr<llvm::IRBuilder<>> builder_; | ||
/// @brief Stores global variables, function lists, and the constructed IR. | ||
std::unique_ptr<llvm::Module> module_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They seem to be passed by reference when used, meaning they are not going to be copied. Furthermore, I believe these classes are not copyable, thus already unique within the code generator.
src/llvm_ir_generator.cpp
Outdated
auto var_type = llvm_util_.GetLLVMType(*(decl.type)); | ||
// For function pointer, we need to change from FunctionType to PointerType | ||
var_type = var_type->isFunctionTy() ? var_type->getPointerTo() : var_type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. I didn't realize the type is already modified by the conversion.
src/llvm_ir_generator.cpp
Outdated
} | ||
} else if (val->getType()->isPointerTy()) { | ||
// function pointer | ||
auto type = llvm_util_.GetLLVMType(*(call_expr.func_expr->type)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When will LLVM require a function pointer to have a pointer type? I'm considering having GetLLVMType
return the pointer type for function pointers directly. Would that be more straightforward?
src/llvm_ir_generator.cpp
Outdated
if (id_expr.type->IsPtr() || id_expr.type->IsFunc()) { | ||
auto res = builder_->CreateLoad(llvm_util_.IntPtrType(), id_val); | ||
val_recorder.Record(res); | ||
val_to_id_addr[res] = id_val; | ||
} else { | ||
auto res = | ||
builder_->CreateLoad(llvm_util_.GetLLVMType(*(id_expr.type)), id_val); | ||
val_recorder.Record(res); | ||
val_to_id_addr[res] = id_val; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see. This is because GetLLVMType
returns FunctionType
on function pointers. 😅
src/llvm_ir_generator.cpp
Outdated
auto res_addr = builder_->CreateConstInBoundsGEP2_32( | ||
arr_type, base_addr, 0, (unsigned int)index->val); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, there's a dedicated page for GEP. 😮 It's a complex topic!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
Changes of the old infrastructure
lli
for testing generated LLVM IR code. I assumed thatlli
can linkprintf
tolibc
, which is it's own c library, and it actually worked! In order to make this work, I did a little bit of hacking inLLVMIRGenerator::FuncCallExprNode
. Whenever, there is a__builtin_print
function being called, it will replaced it withprintf
, so that we don't need to modify our test files.Makefile
: I added a few flags withllvm-config
.main.cpp
: separated QBE and LLVM code generation process with two helper functions. Added LLVM as target option, maybe we can consider changing LLVM as default target. 🐉MemberIndex
for record type for getting the index number of members. Union will always return index0
if member is found. Lastly, changedreturn_type()
ofFuncType
to supportLLVMIRUtil::GetLLVMType()
, which may also need additional help.Newly introduced
include/llvm/util.hpp, src/llvm/util.cpp
: Some utility functions for creating LLVM IR. Also, some suggestions for storingbuilder_
are appreciated! 🙏GetLLVMType
function translate ourType
into correspondingllvm::Type*
, which can be directly used forCreateCall/Load/Store
etc.include/llvm_ir_generator.hpp
: Initialize some LLVM IR builder objects. I saw people say thatLLVMContext
is for LLVM developers to know the details. Right now, we only need to knowLLVMContext
includes LLVM core infrastructures.Module
is kind of like translation unit in our parser, it knows functions, global variables and symbol table, which I didn't use (Maybe it has some handy functions we can use).IRBuilder
is the key object for using LLVM API to construct LLVM IR.src/llvm_ir_generator.cpp
: The main LLVM IR code generator file. Some differences worth noticing:llvm::CmpInst
, so the implementation is a bit different than QBE. I think they separated them because comparison may have different low-level computer architecture optimization techniques than other binary operations.num_recorder
, it isval_recorder
in LLVM IR generator instead. Sincellvm::Value
is a super important class, and any class can literally bellvm::Value
class, except forllvm::Type
, I decided to storellvm::Value*
in the val_recorder. Upper level node can directly use the value stored in it.builder_->Create...
will create a dest number that we can use.builder_->CreateAlloc/CreateCall/CreateStore/CreateLoad
requires the callee's type. It can be tricky for function pointers because we cannot get the element type of pointer. The reason is to support Opaque Pointers. The solution for function pointer's type is to returnllvm::FunctionType
instead ofllvm::PointerType
forGetLLVMType
.br
,ret
.llvm::BasicBlock*
can also represent aslabel
. If we want to jump to label xx, we would jump to basic block xx.Help wanted!
No help needed! I refactored
GetLLVMType
and remove redundant code! 😄As I mentioned that calling a function requiresllvm::FunctionType*
( return type + parameter types), function pointers can be tricky to handle. The ideal way is to get LLVM type from our current type system, which is the job ofLLVMIRUtil::GetLLVMType
, it can return the correct LLVM type for a given Type, similar with yourResolvedType
. Right now, I have aval_to_type
map for mappingllvm::Value*
tollvm::Type*
. This way, I can get the correct function type of the correspondingllvm::Value*
to make a function call. My plan is to get rid ofval_to_type
and fully depend on our type system to get the correct LLVM type.The current bottleneck is to changePtrType
'sbase_type()
fromconst Type& base_type()
toconst std::unique_ptr<Type>& base_type()
, so that I can pass the base type ofPtrType
toLLVMIRUtil::GetLLVMType
. Changing this also require to changeIsEqual
,IsConvertible
etc.