Page MenuHomePhabricator

[AVR] Add instruction selection lowering code

Authored by dylanmckay on Sep 28 2016, 9:25 AM.

Event Timeline

dylanmckay updated this revision to Diff 72849.Sep 28 2016, 9:25 AM
dylanmckay retitled this revision from to [AVR] Add instruction selection lowering code.
dylanmckay updated this object.
dylanmckay added reviewers: kparzysz, arsenm.
This revision was automatically updated to reflect the committed changes.
dylanmckay reopened this revision.Sep 29 2016, 10:16 PM

I accidentally committed this and immediately reverted.

arsenm added inline comments.Oct 2 2016, 9:22 PM
97–98 ↗(On Diff #72968)

Setting these to Expand should be unnecessary since you don't add i32/i64 as legal types (same for all of the other operations)

100 ↗(On Diff #72968)


844–846 ↗(On Diff #72968)

Don't repeat the documentation

849 ↗(On Diff #72968)

Comment doesn't make sense to me here

1134 ↗(On Diff #72968)


1136–1140 ↗(On Diff #72968)

Why not just immediately cast to Function and then remove GV variable

1321–1326 ↗(On Diff #72968)


1509 ↗(On Diff #72968)

static, no inline

1674 ↗(On Diff #72968)


1782–1783 ↗(On Diff #72968)

You should be able to avoid a cast with getSubtarget<AVRSubtarget>

japaric added a subscriber: japaric.Oct 3 2016, 6:46 AM
dylanmckay updated this revision to Diff 73387.Oct 3 2016, 7:10 PM
dylanmckay marked 8 inline comments as done.

Code review from Matt

  • Don't both setting operations to expand on already illegal types
  • Clean up some comments
  • Use nullptr rather than 0
dylanmckay added inline comments.Oct 3 2016, 7:13 PM
1136–1140 ↗(On Diff #72968)

We need the GV variable in the call to getTargetGlobalAddress

1782–1783 ↗(On Diff #72968)

We don't have access to a Function object needed to call that functions.

dylanmckay set the repository for this revision to rL LLVM.Oct 6 2016, 11:39 PM
dylanmckay added a subscriber: llvm-commits.
kparzysz accepted this revision.Nov 1 2016, 6:30 AM
kparzysz edited edge metadata.

LGTM with a few comments.


Else after return, same below.


Same here.

This revision is now accepted and ready to land.Nov 1 2016, 6:30 AM
This revision was automatically updated to reflect the committed changes.
dylanmckay marked an inline comment as done.