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
98–99

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

101

Capitalize

845–847

Don't repeat the documentation

850

Comment doesn't make sense to me here

1135

nullptr

1137–1141

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

1322–1327

Formatting

1510

static, no inline

1675

!CallOperandVal

1783–1784

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
1137–1141

We need the GV variable in the call to getTargetGlobalAddress

1783–1784

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.