Skip to content

compiler: include the scope in the type code name#5183

Open
dgryski wants to merge 11 commits intotinygo-org:devfrom
dgryski:dgryski/named-type-scope
Open

compiler: include the scope in the type code name#5183
dgryski wants to merge 11 commits intotinygo-org:devfrom
dgryski:dgryski/named-type-scope

Conversation

@dgryski
Copy link
Copy Markdown
Member

@dgryski dgryski commented Jan 16, 2026

Fixes #5180

jespino and others added 5 commits January 12, 2026 13:38
* machine/attiny85: add PWM support for Timer0 and Timer1

Add complete PWM implementation for ATtiny85, supporting both Timer0
and Timer1 with their respective output channels:
- Timer0: 8-bit timer for pins PB0 (OC0A) and PB1 (OC0B)
- Timer1: 8-bit high-speed timer for pins PB1 (OC1A) and PB4 (OC1B)

Timer1 provides more flexible period control with configurable top value
(OCR1C) and extended prescaler options (1-16384), making it well-suited
for LED PWM control and other applications requiring variable frequencies.

Implements full PWM interface including Configure, SetPeriod, Channel,
Set, SetInverting, Top, Counter, and Period methods.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* machine/digispark: document PWM support on pins

Add documentation to the Digispark board file indicating which pins
support PWM output:
- P0 (PB0): Timer0 channel A
- P1 (PB1): Timer0 channel B or Timer1 channel A
- P4 (PB4): Timer1 channel B

Includes package comment explaining Timer0 vs Timer1 capabilities,
with Timer1 recommended for more flexible frequency control.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* machine/attiny85: optimize PWM prescaler lookups

Replace verbose switch statements with more efficient implementations:

- SetPeriod: Use bit shift (top >>= prescaler-1) instead of 15-case
  switch for dividing uint64 by power-of-2 prescaler values

- Period: Replace switch statements with compact uint16 lookup tables
  for both Timer0 and Timer1, casting to uint64 only when needed

This addresses review feedback about inefficient switch-based lookups.
On AVR, this approach is significantly smaller:
- Bit shifts for uint64 division: ~34 bytes vs ~140 bytes
- uint16 tables: 22 bytes code + 32/16 bytes data vs ~140 bytes
- Total savings: ~190 bytes (68% reduction)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* examples/pwm: add digispark support and smoketest

Add digispark.go configuration for PWM example using Timer1 with pins P1 (LED) and P4. Also add digispark PWM example to GNUmakefile smoketests.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
…ons for all peripheral reset/unreset operations

Signed-off-by: deadprogram <ron@hybridgroup.com>
…em resources/power usage

Signed-off-by: deadprogram <ron@hybridgroup.com>
This simplifies the process of constructing and encoding layout bitmaps.
Instead of creating big integers and merging them, we can create a pre-sized bitmap and set positions within it.

This also changes the encoding logic to allow larger layouts to be encoded inline.
We would previously not encode a layout inline unless the size was less than the width of the data field.
This is overly conservative.
A layout can be encoded inline as long as:
1. The size fits within the size field.
2. All set bits in the bitmap fit into the data field.
Signed-off-by: deadprogram <ron@hybridgroup.com>
@dgryski dgryski requested review from aykevl and niaow January 16, 2026 21:30
@dgryski dgryski force-pushed the dgryski/named-type-scope branch from cec236b to 4fd15a3 Compare January 16, 2026 21:40
@niaow
Copy link
Copy Markdown
Member

niaow commented Jan 16, 2026

Maybe add this to the compiler frontend tests (compiler/testdata/interface.*)

@dgryski dgryski force-pushed the dgryski/named-type-scope branch from 4fd15a3 to 91c1d8a Compare January 16, 2026 22:46
@dgryski
Copy link
Copy Markdown
Member Author

dgryski commented Jan 16, 2026

I don't think I can put it in the golden compiler/testdata/*.ll files since the pointer address will change between runs. It does have a test in the main testdata/ directory though.

@niaow
Copy link
Copy Markdown
Member

niaow commented Jan 16, 2026

Oh, we should not be using the pointer address for this.

@dgryski
Copy link
Copy Markdown
Member Author

dgryski commented Jan 16, 2026

There doesn't appear to be anything else aside from pointer addresses to allow us to differentiate scopes. Even the String() representation of a scope includes the pointer address. I'd love a more stable suggestion. I asked in #tools on Slack for advice.

@dgryski
Copy link
Copy Markdown
Member Author

dgryski commented Jan 16, 2026

Note also this the pointer address of the scope in the compiler, not the pointer at runtime on the device.

@niaow
Copy link
Copy Markdown
Member

niaow commented Jan 16, 2026

If that is the case then we could loop through the SSA and assign numbers to them? Non-deterministic compilation is not acceptable.

@dgryski
Copy link
Copy Markdown
Member Author

dgryski commented Jan 16, 2026

Ok. I'll see what I can come up with.

@dgryski
Copy link
Copy Markdown
Member Author

dgryski commented Jan 17, 2026

PTAL

dgryski and others added 2 commits January 17, 2026 17:59
* testdata: more corpus entries

* testdata: remove skipwasi for dchest/siphash build issues
* machine/attiny85: add USI-based SPI support

Implement SPI communication for ATTiny85 using the USI (Universal Serial
Interface) hardware in three-wire mode. The ATTiny85 lacks dedicated SPI
hardware but can emulate SPI using the USI module with software clock
strobing.

Implementation details:
- Configure USI in three-wire mode for SPI operation
- Use clock strobing technique to shift data in/out
- Pin mapping: PB2 (SCK), PB1 (MOSI/DO), PB0 (MISO/DI)
- Support both Transfer() and Tx() methods

The implementation uses the USI control register (USICR) to toggle the
clock pin, which triggers automatic bit shifting in hardware. This is
more efficient than pure software bit-banging.

Current limitations:
- Frequency configuration not yet implemented (runs at max software speed)
- Only SPI Mode 0 (CPOL=0, CPHA=0) supported
- Only MSB-first bit order supported

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Co-authored-by: Ona <no-reply@ona.com>

* machine/attiny85: add SPI frequency configuration support

Add software-based frequency control for USI SPI. The ATtiny85 USI lacks
hardware prescalers, so frequency is controlled via delay loops between
clock toggles.

- Calculate delay cycles based on requested frequency and CPU clock
- Fast path (no delay) when frequency is 0 or max speed requested
- Delay loop uses nop instructions for timing control

Co-authored-by: Ona <no-reply@ona.com>

* machine/attiny85: add SPI mode configuration support

Add support for all 4 SPI modes (Mode 0-3) using USI hardware:
- Mode 0 (CPOL=0, CPHA=0): Clock idle low, sample on rising edge
- Mode 1 (CPOL=0, CPHA=1): Clock idle low, sample on falling edge
- Mode 2 (CPOL=1, CPHA=0): Clock idle high, sample on falling edge
- Mode 3 (CPOL=1, CPHA=1): Clock idle high, sample on rising edge

CPOL is controlled by setting the clock pin idle state.
CPHA is controlled via the USICS0 bit in USICR.

Co-authored-by: Ona <no-reply@ona.com>

* machine/attiny85: add LSB-first bit order support

Add software-based LSB-first support for USI SPI. The USI hardware only
supports MSB-first, so bit reversal is done in software before sending
and after receiving.

Uses an efficient parallel bit swap algorithm (3 operations) to reverse
the byte.

Co-authored-by: Ona <no-reply@ona.com>

* GNUmakefile: add mcp3008 SPI example to digispark smoketest

Test the USI-based SPI implementation for ATtiny85/digispark.

Co-authored-by: Ona <no-reply@ona.com>

* machine/attiny85: minimize SPI RAM footprint

Reduce SPI struct from ~14 bytes to 1 byte to fit in ATtiny85's limited
512 bytes of RAM.

Changes:
- Remove register pointers (use avr.USIDR/USISR/USICR directly)
- Remove pin fields (USI pins are fixed: PB0/PB1/PB2)
- Remove CS pin management (user must handle CS)
- Remove frequency control (runs at max speed)
- Remove LSBFirst support

The SPI struct now only stores the USICR configuration byte.

Co-authored-by: Ona <no-reply@ona.com>

* Revert "machine/attiny85: minimize SPI RAM footprint"

This reverts commit 387ccad.

Co-authored-by: Ona <no-reply@ona.com>

* machine/attiny85: reduce SPI RAM usage by 10 bytes

Remove unnecessary fields from SPI struct while keeping all functionality:
- Remove register pointers (use avr.USIDR/USISR/USICR directly)
- Remove pin fields (USI pins are fixed: PB0/PB1/PB2)
- Remove CS pin (user must manage it, standard practice)

Kept functional fields:
- delayCycles for frequency control
- usicrValue for SPI mode support
- lsbFirst for bit order support

SPI struct reduced from 14 bytes to 4 bytes.

Co-authored-by: Ona <no-reply@ona.com>

---------

Co-authored-by: Ona <no-reply@ona.com>
@dgryski dgryski force-pushed the dgryski/named-type-scope branch from 91c1d8a to b6aa203 Compare January 19, 2026 17:26
@dgryski
Copy link
Copy Markdown
Member Author

dgryski commented Jan 19, 2026

(Oops, hadn't actually pushed my changes.... 🤦 )

@dgryski dgryski force-pushed the dgryski/named-type-scope branch 2 times, most recently from f6cd265 to f087248 Compare January 19, 2026 18:30
@dgryski dgryski force-pushed the dgryski/named-type-scope branch from f087248 to 7b86f95 Compare January 19, 2026 18:42
for scope != pkg {
parent := scope.Parent()
for i := range parent.NumChildren() {
if parent.Child(i) == scope {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This can result in O(n^2) overhead. Maybe build a map with everything in the package once and then look up from it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This code only runs for types declared in a local scope. I'll add a scope -> name cache for this function but walking the entire list of scopes looking for named types before we need them seems like overkill.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You still have not fixed this issue. You are still iterating over every child of every parent for every child. This index is what you should be caching.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Do you want the id cached for only the matching child (so I don't need to iterate next time), or for every child? (I'm concerned the "every child" case will just bloat the cache with entries that will never be queried. This is already only walking up the scopes from the scope the type is defined in up to the parent scope, so we're only looking at the children of parent nodes on the path to the root.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Commit e56e3e5 caches the index of each scope that we encounter on the path to the root. We might have to double scan a particular parent if there are multiple scopes with types defined in them. (I'm going to run this over the test corpus and see if I can quantify the loops -- the numbers we're dealing with are pretty small..)

Copy link
Copy Markdown
Member

@niaow niaow Jan 20, 2026

Choose a reason for hiding this comment

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

or for every child?

Yes. The cache does not actually solve anything unless you do this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added some logging to getScopeID and this is the output for running the compiler over the entire test corpus (180 packages, ~1.5 million lines of Go code):

~/go/src/github.com/dgryski/tinygo-test-corpus $ rg === corpus.out |sed 's/.*===//'
scopeIdx: id=46:10: loops=58
scopeIdx: id=46:10: loops=0
scopeIdx: id=46:10: loops=0
scopeIdx: id=25:4: loops=31
scopeIdx: id=25:4: loops=0
scopeIdx: id=25:4: loops=0
scopeIdx: id=25:4: loops=0
scopeIdx: id=25:4: loops=0
scopeIdx: id=25:4: loops=0
scopeIdx: id=4:1: loops=7
scopeIdx: id=4:1: loops=0
scopeIdx: id=4:1: loops=0
scopeIdx: id=4:1: loops=0
scopeIdx: id=0:3: loops=5
scopeIdx: id=0:3: loops=0
scopeIdx: id=0:3: loops=0
scopeIdx: id=0:3: loops=0
scopeIdx: id=0:3: loops=0
scopeIdx: id=0:3: loops=0
scopeIdx: id=0:3: loops=0
scopeIdx: id=0:3: loops=5
scopeIdx: id=0:3: loops=0
scopeIdx: id=0:3: loops=0
scopeIdx: id=0:3: loops=0
scopeIdx: id=0:3: loops=0
scopeIdx: id=0:3: loops=0
scopeIdx: id=0:3: loops=0
scopeIdx: id=8:0: loops=10
scopeIdx: id=8:0: loops=0
scopeIdx: id=8:0: loops=0
scopeIdx: id=10:0: loops=11
scopeIdx: id=10:0: loops=0
scopeIdx: id=10:0: loops=0
scopeIdx: id=0:9:0:5:1:0: loops=21
scopeIdx: id=0:9:0:5:1:0: loops=0
scopeIdx: id=0:9:0:5:1:0: loops=0

Even only adding in the matching child, the number of iterations is very small, and this code just isn't called that frequently. Adding in all the children will just increase the memory usage for no benefit, since none of them are likely to have named types in them that we're going to query.

@niaow
Copy link
Copy Markdown
Member

niaow commented Jan 19, 2026

This is better

Copy link
Copy Markdown
Member

@niaow niaow left a comment

Choose a reason for hiding this comment

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

And again, this needs to be in the compiler testdata.

scopeid map[*types.Scope]string
}{
sync.Mutex{},
make(map[*types.Scope]string),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Move this into the compiler context to get rid of the lock and the memory leak.

for scope != pkg {
parent := scope.Parent()
for i := range parent.NumChildren() {
if parent.Child(i) == scope {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You still have not fixed this issue. You are still iterating over every child of every parent for every child. This index is what you should be caching.

@niaow
Copy link
Copy Markdown
Member

niaow commented Jan 20, 2026

Actually I just thought of a much simpler solution to this that I can try in a bit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants