compiler: prevent compile time functions leaking into run time

[davecheney]

Interpreter fallback can leave expensive initialization in firmware without a build error. Add a //go:noruntime directive so retained runtime uses fail the build.

The name matches the other //go:no... pragmas such as //go:noinline and //go:noheap. The directive does not make the interpreter evaluate a function. It only checks that no runtime use of the function remains.

[davecheney]

@deadprogram @dgryski this is a companion to #5837 . Depending on the order in which they are merged both can be simplified, if this PR lands before #5837 then its test can be replaced with //go:noruntime, if #5837 lands first then this PR will rebase itself on that and remove the test then, as well as remove the duplicate in its tests.

[davecheney]

@deadprogram @dgryski PTAL, I've rebased the branch and removed the ornate testing to spot the PLL calcs leaking into the runtime and replaced them with //go:nolinktime.

I have a follow up branch which hardens some of the other pragma implementations in the compiler.

[deadprogram]

Thanks for another one @davecheney! Replacing the IR scan test with a build error is a nice improvement. I ran the tests on my local machine and was perusing the code. The following is edited from an automated review:

1. When interp falls back, the error points at each marked function definition but not at the use that kept it alive. Forcing the RP2 PLL init to fall back gives four errors in machine_rp2_pll.go and nothing pointing at the CalcDivs call in machine_rp2_clocks.go. Perhaps also report the first remaining use of the marked function? That would make it much easier to see why the function remained.

2. The README calls the directive a prototype, but any package can use it. Should it be limited to GOROOT packages for now, the same way some other pragmas are restricted? Or if it is meant for users, perhaps it also needs a page on tinygo.org?

3. Since a live method table can retain a marked method, pllSearch.CalcDivs would break the build if a pllSearch value ever ended up in an interface. Should it be a plain function instead? Not a big deal, just a possible sharp edge.

Your thoughts?

[davecheney]

@deadprogram PTAL

Thanks for another one @davecheney! Replacing the IR scan test with a build error is a nice improvement. I ran the tests on my local machine and was perusing the code. The following is edited from an automated review:

1. When interp falls back, the error points at each marked function definition but not at the use that kept it alive. Forcing the RP2 PLL init to fall back gives four errors in machine_rp2_pll.go and nothing pointing at the CalcDivs call in machine_rp2_clocks.go. Perhaps also report the first remaining use of the marked function? That would make it much easier to see why the function remained.

I've churched up the error messages

src/machine/machine_rp2_clocks.go:151:37: //go:noruntime function machine.calcPLLDivs is used at runtime by machine.init#2
src/machine/machine_rp2_pll.go:121: //go:noruntime function machine.calcPLLDivs remains reachable at runtime ... (pllFreqOutPostdiv, genTable, genTableEntry)

2. The README calls the directive a prototype, but any package can use it. Should it be limited to GOROOT packages for now, the same way some other pragmas are restricted? Or if it is meant for users, perhaps it also needs a page on tinygo.org?

In the spirit of your machine.AssignCore experiment, I have relabelled this as an experiment. Restricting this to only GOROOT makes testing awkward so hopefully the experimental moniker will be sufficient. Ideally I don't see a reason user code cannot use this in their own init() functions.

3. Since a live method table can retain a marked method, pllSearch.CalcDivs would break the build if a pllSearch value ever ended up in an interface. Should it be a plain function instead? Not a big deal, just a possible sharp edge.

Agreed, converted to a function. There was no reason for it to be a method on a config struct, it was just like that because of the way the previous change split the method into two.

[davecheney]

@deadprogram wrt your last point about a page on tinygo org. Yes, absolutely, but first I want to follow @dgryski 's suggestion and take this for a test drive across the tinygo codebase. If it survives contact with the real world I'll get you a page for the docs site for 0.44.

Please note there is also https://github.com/davecheney/tinygo/tree/davecheney-harden-pragmas which I'll PR once this lands which adds a bunch of checks for correct use of the fleet of pragmas that tinygo supports.

[deadprogram]

Thanks for the quick turnaround @davecheney! I ran the tests again on my local machine with the updated branch.

1. The new error now points right at the calcPLLDivs call in machine_rp2_clocks.go, which makes it easy to see why the function stayed around.

2. The experiment label makes sense, and sounds like a good plan to try it out across the codebase first before the docs page.

3. Thanks for switching calcPLLDivs to a plain function.

Looking forward to the pragma hardening follow-up.