Skip to content
This repository has been archived by the owner on Jun 1, 2020. It is now read-only.

use llir/llvm@v0.3.0-pre2 for code generation #115

Merged
merged 4 commits into from
Nov 29, 2018
Merged

Conversation

mewmew
Copy link

@mewmew mewmew commented Nov 28, 2018

As an experiment, use the latest version of llir/llvm for code generation.

codegen test case output:

u@x1 ~/D/l/e/codegen> go test -v
=== RUN   TestBinaryFunction
define i32 @main() {
; <label>:0
	%1 = call i32 @add(i32 10, i32 10)
	ret i32 %1
}

define i32 @add(i32 %x, i32 %y) {
; <label>:0
	%1 = add i32 10, 10
	ret i32 %1
}
--- PASS: TestBinaryFunction (0.00s)
PASS
ok  	github.com/elz-lang/elz/codegen	0.001s

Updates #5.

P.S. the test case was also updated to include a ret instruction for the @main function.

@codecov-io
Copy link

codecov-io commented Nov 28, 2018

Codecov Report

Merging #115 into llir will decrease coverage by 1.02%.
The diff coverage is 52.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##             llir     #115      +/-   ##
==========================================
- Coverage   73.46%   72.44%   -1.03%     
==========================================
  Files           5        5              
  Lines         196      196              
==========================================
- Hits          144      142       -2     
- Misses         52       53       +1     
- Partials        0        1       +1
Impacted Files Coverage Δ
codegen/codegen.go 60.86% <52.63%> (-2.18%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2899fbb...6d3a3e1. Read the comment docs.

@@ -36,5 +36,6 @@ func TestBinaryFunction(t *testing.T) {
},
},
)
builder.NewRet(v)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, this ret instruction was added in the PR.

codegen/codegen_test.go Outdated Show resolved Hide resolved
newFunctionBuilder.SetInsertPointAtEnd(block)
newFn := c.mod.NewFunc(binding.Name, retT, params...)
block := newFn.NewBlock("")
newFunctionBuilder := block
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newFnBuilder := newFn.NewBlock("")

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newFnBuilder := newFn.NewBlock("")

Done.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the update, but after thinking about it further, there is a benefit with having the basic block created explicitly (as was done in the original code). The reason is that you may want to update how the builder works, right now, it always uses SetInsertPointAtEnd by default.

In particular, I would imagine that the type Builder interface in builder.go would be replaced with a concrete builder implementation that allows for more flexible control over insertion points for instructions.

This is just a suggestion however, and you can update the code at that time, should the need arise.

Cheers,
Robin

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation, I'm also thinking why not use concrete type XD.

@dannypsnl dannypsnl merged commit f25fb3b into dannypsnl:llir Nov 29, 2018
dannypsnl added a commit that referenced this pull request Nov 29, 2018
*  use llir/llvm@v0.3.0-pre2 for code generation (#115)

* use llir/llvm@v0.3.0-pre2 for code generation

* use NewFunc instead of NewFunction

* update go.mod to latest version of llir/llvm

* codegen: simplify use of builder based on feedback from @dannypsnl

* test: remove c-binding go-llvm

* fix: count function call type by expression

At least not hard code
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants