This is an archive of the discontinued LLVM Phabricator instance.

[AVR] Add the machine code backend
ClosedPublic

Authored by dylanmckay on Sep 28 2016, 8:32 AM.

Details

Summary

This adds the AVR machine code backend (AVRAsmBackend.cpp). This will
allow us to generate machine code from assembled AVR instructions.

Diff Detail

Repository
rL LLVM

Event Timeline

dylanmckay updated this revision to Diff 72837.Sep 28 2016, 8:32 AM
dylanmckay retitled this revision from to [AVR] Add the machine code backend.
dylanmckay updated this object.
dylanmckay added reviewers: kparzysz, arsenm.
kparzysz added inline comments.Sep 30 2016, 11:55 AM
lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp
38 ↗(On Diff #72837)

Is T ever anything other than uint64_t? Do you really need all these templates?

250 ↗(On Diff #72837)

Can Ctx ever be null? You are getting it from MCAssembler that has it as a member, so it will always be there. Unless you anticipate that it won't always be available (e.g. you will call this function from another place), you could make Ctx a reference.

dylanmckay added inline comments.Sep 30 2016, 10:33 PM
lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp
38 ↗(On Diff #72837)

I plan on moving these under a common header so that it can be shared between the AsmBackend and the LLVM linker.

Seems good form to not force either user to use uint64_t, but I'm happy to remove the templates if you still think they should go.

250 ↗(On Diff #72837)

We normally get the MCContext from MCAssembler but not always.

When using -show-encoding with llvm-mc, fixups are adjusted but there is no accompanied MCContext, in which case we have to handle it being null.

kparzysz added inline comments.Oct 3 2016, 5:46 AM
lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp
38 ↗(On Diff #72837)

All of the adjustments only access the low 32 bits of the value. The target-independent MC code uses uint64_t. I don't think there is any genericity to be gained by using templates in this case.

114 ↗(On Diff #72837)

Shouldn't the shift be by 18?

japaric added a subscriber: japaric.Oct 3 2016, 6:45 AM
dylanmckay updated this revision to Diff 73389.Oct 3 2016, 7:47 PM
dylanmckay marked 4 inline comments as done.

Code review from Krzystof

  • Fixed encoding of 'call' fixup
  • Collapsed template into uint64_t
dylanmckay added inline comments.Oct 3 2016, 7:47 PM
lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp
114 ↗(On Diff #72837)

It looks like that top value was completely wrong - we were duplicating the lower 4 bits and moving it to the top.

I have fixed this up so we instead grab the top 4 bits and shift it into the correct position.

kparzysz accepted this revision.Oct 4 2016, 5:35 AM
kparzysz edited edge metadata.
This revision is now accepted and ready to land.Oct 4 2016, 5:35 AM
This revision was automatically updated to reflect the committed changes.