search

avr-llvm / llvm

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

tinyavr (ATtiny10) code generator incorrectly generates MOVW #200

Closed cpldcpu closed 8 years ago

cpldcpu commented 8 years ago

I just noticed, that the code generator generates a word reg-reg move instruction (MOVW) also for the tinyavr core (ATtiny10). This is not support according to the datasheet.

See attached example.

avrtest.zip

dylanmckay commented 8 years ago

I wouldn't be surprised to be honest, I haven't worked on AVR Tiny support very much.

Good thing is though that this is an easy fix :)

@cpldcpu: Is MOVW unsupported by all Tiny cores or just the 10?

dylanmckay commented 8 years ago

Looking at the datasheet to the ATtiny25, it looks like it's not all of the Tiny cores.

dylanmckay commented 8 years ago

Pulling up the datasheet for the ATtiny10 family, it looks like these are the devices that don't support the instruction.

dylanmckay commented 8 years ago

It looks like in AVRInstrInfo::copyPhysReg, we were unconditionally creating MOVW instructions, ignoring the subtarget settings.

Am fixing it up now to use two 8-bit MOV instructions if MOVW is unsupported.

Would you be able to test that the fix works @cpldcpu once I merge it?

dylanmckay commented 8 years ago

Should be fixed as of f6524cd0. @cpldcpu can you confirm this?

cpldcpu commented 8 years ago

All devices with the "avrtiny" architecture do not support MOVW: ATtiny 4/5/9/10/20/40/102/104

This is just a small subset of the "ATtiny" series. The ATtiny 25 is based on the AVR25 architecture.

Slightly confusing...

I will test later.

Btw - I came across a huge load of issues in connection with assembler code. Is this currently a focus?

dylanmckay commented 8 years ago

So, to be clear, we should disable MOVW for all tiny cores except the 25, 45, and 85?

Btw - I came across a huge load of issues in connection with assembler code. Is this currently a focus?

What kind of issues?

cpldcpu commented 8 years ago

Nono, only for the "tinyavr" / "reduced core" architecture. See Avr-gcc mapping:

http://www.nongnu.org/avr-libc/user-manual/using_tools.html

Regarding assembler: Can provide more bug reports later. I have been trying to compile V-USB.

dylanmckay commented 8 years ago

The table of subtarget features we have is pretty much taken from that data, so after f6524cd this should now work correctly for all Tiny devices.

dylanmckay commented 8 years ago

@cpldcpu if you want to open an issue, go ahead :)

cpldcpu commented 8 years ago

Might as well add the latest two:

def : Device<"attiny102", FamilyAVRTiny, ELFArchAVRTiny>; def : Device<"attiny104", FamilyAVRTiny, ELFArchAVRTiny>;

Will open an issue later.

dylanmckay commented 8 years ago

Have added those devices as of 0c265ab.

cpldcpu commented 8 years ago

Ok, recompiled and tested my code. Looks looks good now.