This is an archive of the discontinued LLVM Phabricator instance.

[AVR] Add instruction selection lowering code
ClosedPublic

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

Diff Detail

Repository
rL LLVM

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
llvm/trunk/lib/Target/AVR/AVRISelLowering.cpp
97–98

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

Capitalize

844–846

Don't repeat the documentation

849

Comment doesn't make sense to me here

1134

nullptr

1136–1140

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

1321–1326

Formatting

1509

static, no inline

1674

!CallOperandVal

1782–1783

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
llvm/trunk/lib/Target/AVR/AVRISelLowering.cpp
1136–1140

We need the GV variable in the call to getTargetGlobalAddress

1782–1783

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.

lib/Target/AVR/AVRISelLowering.cpp
884 ↗(On Diff #73387)

Else after return, same below.

1802 ↗(On Diff #73387)

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.