avr-llvm / llvm

[MERGED UPSTREAM] AVR backend for the LLVM compiler library
220 stars 21 forks source link

Implement the 16-bit STS instruction #138

Open dylanmckay opened 9 years ago

dylanmckay commented 9 years ago

There are two instructions which perform direct stores to data space.

They have the same mnemonic, but one is a 32-bit wide instruction taking a GPR pair as an operand, and one is a 16-bit wide instruction taking a single GPR(although it only uses the upper 16 registers (r16-r31).

It seems as if the 16-bit version is unsupported - it isn't enabled when compiling on the higher end XMEGAs, or the low end avr2s. I discovered it by accident when using avr-as -mall-opcodes on our test suite. It seems as if the only way it is enabled is using this command, and it is probably a bug in that regard because it "overrides" the 32-bit version so it cannot be used with the flag.

The instruction set manual section on the 16-bit variant.

Stores one byte from a Register to the data space. For parts with SRAM, the data space consists of the Register
File, I/O memory and internal SRAM (and external SRAM if applicable). For parts without SRAM, the data space
consists of the Register File only. In some parts the Flash memory has been mapped to the data space and can
be written using this command. The EEPROM has a separate address space.
A 7-bit address must be supplied. The address given in the instruction is coded to a data space address as
follows:

ADDR[7:0] = (NOT(INST[8]), INST[8], INST[10], INST[9], INST[3], INST[2], INST[1], INST[0])
Memory access is limited to the address range 0x40...0xbf of the data segment.
This instruction is not available in all devices. Refer to the device specific instruction set summary.

...

Note:
Registers r0...r15 are remaped to r16...r31

Of course, the instruction is still a part of the ISA, so the question then follows - do we support it (very difficult and time consuming, probably a terrible idea), or pretend it doesn't exist, and only support the 32-bit variant?

dylanmckay commented 9 years ago

cc @agnat

agnat commented 9 years ago

[...] 32-bit wide instruction taking a GPR pair as an operand [...]

If I'm not mistaken both versions take a single register. Anyway...

I think it is pretty low priority. It seems like a nice task for a boring winter evening, though. Let's keep the ticket and ignore it for now. Maybe change the title to something more... actionable.

agnat commented 9 years ago

Hm, I see... It only takes one cycle. Anyway, I still think it's low priority.

dylanmckay commented 8 years ago

The reason I originally said this would be difficult to implement was because I assumed we'd have to be smart about selecting the correct instruction based on the registers.

It should be fairly straightforward to implement without this behaviour, which is what GCC does too I think.

dylanmckay commented 8 years ago

Removing low priority tag because this blocks AVRTiny, which doesn't have the 32-bit variant.