Repository navigation
compiler: always inline runtime/volatile functions - #6064
davecheney wants to merge 1 commit into
Conversation
db46182 to
b9a4cfa
Compare
At -opt=z every function has minsize, and LLVM ignores inlinehint in a minsize caller. A register write through a pointer, such as a Register32 held in a driver struct, then becomes a call to Set. The inlined store is also smaller than the call. Measured with the ili9325 Lorenz example from tinygo-org/drivers#926 on an Arduino M0 (SAMD21, 48 MHz). The flash sizes include timing code. The speeds are the mean of three readings. | opt | flash before | flash after | FillScreen before | FillScreen after | Lorenz steps/s before | Lorenz steps/s after | |---|---|---|---|---|---|---| | z | 24104 | 24024 | 137.9 ms | 34.2 ms | 3125 | 3667 | | s | 24196 | 24148 | 29.2 ms | 29.2 ms | 3972 | 3943 | | 1 | 33620 | 33620 | 29.3 ms | 29.3 ms | 4117 | 4134 | | 2 | 33092 | 33092 | 29.2 ms | 29.5 ms | 4156 | 4116 | The z before values are from the same dev commit. The s, 1 and 2 before speeds are from an earlier dev commit. At -opt=z, FillScreen is still 5 ms slower than at -opt=s, with no calls to runtime/volatile left. That difference comes from minsize in other code and is not part of this change. Fixes tinygo-org#6063
b9a4cfa to
6f797f4
Compare
dgryski
left a comment
dgryski
left a comment
There was a problem hiding this comment.
LGTM.. Question for a follow-up PR (just while we're here looking at runtime and inlining...)
| switch b.info.inline { | ||
| case inlineHint: | ||
| // Add LLVM inline hint to functions with //go:inline pragma. | ||
| inline := b.ctx.CreateEnumAttribute(llvm.AttributeKindID("inlinehint"), 0) |
There was a problem hiding this comment.
I wonder if we should always inline runtime functions that have been tagged with go:inline
diff --git a/compiler/compiler.go b/compiler/compiler.go
index 421a9b76..9504a7b4 100644
--- a/compiler/compiler.go
+++ b/compiler/compiler.go
@@ -1245,7 +1245,11 @@ func (b *builder) createFunctionStart(intrinsic bool) {
switch b.info.inline {
case inlineHint:
// Add LLVM inline hint to functions with //go:inline pragma.
- inline := b.ctx.CreateEnumAttribute(llvm.AttributeKindID("inlinehint"), 0)
+ kind := "inlinehint"
+ if b.fn.Pkg != nil && b.fn.Pkg.Pkg.Path() == "runtime" {
+ kind = "alwaysinline"
+ }
+ inline := b.ctx.CreateEnumAttribute(llvm.AttributeKindID(kind), 0)
b.llvmFn.AddFunctionAttr(inline)
case inlineNone:
// Add LLVM attribute to always avoid inlining this function.
There was a problem hiding this comment.
Benchmarked map operations with this patch on native, wasip1, arduino r4, and a pico 2. Consistently faster for on -opt=z but slower for -opt=2. So maybe if we could guard this on optimization level?
There was a problem hiding this comment.
i'll look into that. In my testing opt=2 is never smaller or faster than opt=1, I know we use opt=2 for size tests, and I'm kind of wondering if that is the right choice, kinda of the way that -O2 was the default choice in gcc back in the day not -O3. See also #6083 as a proposal.
Once I'm ready to move this out of draft I can make this opt level specific as it only really applies to opt=z and the magic minsize attribute. But I also want to think a little bit more about encoding this magic into the compiler vs another of my new found passions, pragams
At -opt=z every function has minsize, and LLVM ignores inlinehint in a minsize caller. A register write through a pointer, such as a Register32 held in a driver struct, then becomes a call to Set. The inlined store is also smaller than the call.
Measured with the ili9325 Lorenz example from tinygo-org/drivers#926 on an Arduino M0 (SAMD21, 48 MHz). The flash sizes include timing code. The speeds are the mean of three readings.
The z before values are from the same dev commit. The s, 1 and 2 before speeds are from an earlier dev commit.
At -opt=z, FillScreen is still 5 ms slower than at -opt=s, with no calls to runtime/volatile left. That difference comes from minsize in other code and is not part of this change.
Fixes #6063