This is an archive of the discontinued LLVM Phabricator instance.

Differential D75099

[AVR] Include AVR by default in LLVM builds
ClosedPublic

Authored by dylanmckay on Feb 24 2020, 9:21 PM.

Details

Reviewers
chandlerc
lattner
rengolin
tstellar
arsenm
thakis
simoll
asb
Commits
rGc480c584a0b7: [AVR] Include AVR by default in LLVM builds
rGdfecec65e679: [AVR] Include AVR by default in LLVM builds
Summary

This patch makes the AVR backend an official target of LLVM, serving
as a request for comments for moving the AVR backend out of
experimental.

A future patch will move the LLVM AVR buildbot (llvm-avr-linux) from the
staging buildmaster to the production buildmaster, so error emails will
start to go out.

Summary of the backend

  • 16-bit little endian
  • AsmParser based assembly parser
  • uses the MC library for generating AVR ELFs
  • most logic driven from standard TableGen-erated tables like other backends
  • passes all of the test suite under check-all, including generic CodeGen and DebugInfo tests
  • Used in two frontends
  • Limited, but functional support for DebugInfo and LLVM DWARF dumping
  • Binary compatible with AVR-GCC and avr-{libc,libgcc} for the most part
  • Cannot lower 32-bit shifts due to a bug, can lower shifts larger or smaller
  • Supports assembly/MC for all the entire AVR ISA, generally generates poorly optimized machine instructions, with most focus thus far on correctness

I've added reviewers and subscribers from previous patches where backends were made official,
and those who participated in the recent thread on llvm-dev, please add anybody I've missed.

The most recent discussion on this topic can be found in the llvm-dev thread Moving the AVR backend out of experimental

Diff Detail

Event Timeline

dylanmckay created this revision.Feb 24 2020, 9:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 24 2020, 9:21 PM
Herald added subscribers: Jim, aprantl, mgorny, wdng. · View Herald Transcript
Harbormaster completed remote builds in B47182: Diff 246367.Feb 24 2020, 9:23 PM
dylanmckay edited the summary of this revision. (Show Details)Feb 24 2020, 9:29 PM
dylanmckay edited the summary of this revision. (Show Details)
thakis accepted this revision.Feb 25 2020, 4:35 AM

The thread sounded overall positive to me, and especially with gcc apparently dropping their AVR backend this seems like a good time for this. (But I'm in favor of this independent of gcc's decision.)

This revision is now accepted and ready to land.Feb 25 2020, 4:35 AM
aykevl added a comment.Feb 25 2020, 7:23 AM

Great! I've been looking forward to this.

Used in two frontends

I count 3: Rust, TinyGo, and Swift for Arduino.

Limited, but functional support for DebugInfo and LLVM DWARF dumping

In my experience this only started working with D74213, but maybe you mean something different.

Just nitpicking here, I don't want to hold this change back.

CryZe added a subscriber: CryZe.Feb 26 2020, 12:43 AM
dylanmckay added a comment.Feb 26 2020, 2:00 AM
Limited, but functional support for DebugInfo and LLVM DWARF dumping

In my experience this only started working with D74213, but maybe you mean something different.

Good point, I was getting mixed up with the LLVM MC dumper and its fixup support, I will amend the description.

dylanmckay updated this revision to Diff 246651.Feb 26 2020, 2:01 AM

Remove DWARF-dumping claims from the commit message.

Harbormaster completed remote builds in B47278: Diff 246651.Feb 26 2020, 2:03 AM
asb added a comment.Feb 27 2020, 5:22 AM

Hi Dylan - on the mailing list thread for this I raised a concern about disassembler support based on comments I'd seen in a recent patch. Can you please clarify the current status there?

rengolin added a comment.Feb 27 2020, 1:52 PM

Parachuting here, but, for reference, this is the builder that seems to be mostly green for a while: http://lab.llvm.org:8014/builders/llvm-avr-linux

You should first move the buildbot to the noisy master and then move it built by default. We really don't want it breaking silently while built on everyone else's development environments.

dylanmckay added a comment.Mar 2 2020, 10:53 PM

You should first move the buildbot to the noisy master and then move it built by default. We really don't want it breaking silently while built on everyone else's development environments.

Good suggestion @rengolin, I have moved it to the official buildmaster: http://lab.llvm.org:8011/builders/llvm-avr-linux/builds/0

dylanmckay added a comment.Mar 2 2020, 10:57 PM

Hi Dylan - on the mailing list thread for this I raised a concern about disassembler support based on comments I'd seen in a recent patch. Can you please clarify the current status there?

@aykevl perhaps you might have a more up-to-date answer than me here given the recent DWARF-dumping fixes you've made?

dylanmckay added a comment.Mar 6 2020, 7:17 PM

The AVR buildbot has been running successfully for a few days now: http://lab.llvm.org:8011/builders/llvm-avr-linux/builds/63.

aykevl added a comment.Mar 7 2020, 9:54 AM
In D75099#1902339, @dylanmckay wrote:

Hi Dylan - on the mailing list thread for this I raised a concern about disassembler support based on comments I'd seen in a recent patch. Can you please clarify the current status there?

@aykevl perhaps you might have a more up-to-date answer than me here given the recent DWARF-dumping fixes you've made?

I don't think those two are related.

  • Disassembler support has improved but is not nearly there. I have a number of local changes not even sent for review (went to work on other things). At least it shouldn't crash llvm-objdump now and should disassemble roughly half of the available instructions as I started with low hanging fruit. In my personal opinion, I don't think this should now block marking the AVR backend as stable.
  • I haven't tested that DWARF patch exactly as it was when I committed it, but did test it when I submitted it for review and at that time it seemed to fix DWARF emission completely (llvm-dwarfdump does not show an error). I could have missed something, of course.
dylanmckay added a comment.Mar 10 2020, 10:01 PM

I'm keen to get at least one more approval before merging this one.

rengolin accepted this revision.Mar 11 2020, 3:15 AM

Based on mailing list discussions I think it should be fine for the AVR backend to be built by default. The code owners should prepare for a higher influx of bugs and be extra-responsive for a while. Thanks!

Closed by commit rGdfecec65e679: [AVR] Include AVR by default in LLVM builds (authored by dylanmckay). · Explain WhyMar 11 2020, 11:18 PM
This revision was automatically updated to reflect the committed changes.
hans added a comment.Mar 12 2020, 4:34 AM

Dylan, can you please add a note about this in llvm/docs/ReleaseNotes.rst?

Revision Contents

PathSize
llvm/
1 line

Diff 249843

llvm/CMakeLists.txt

Show First 20 Lines • Show All 283 Lines • ▼ Show 20 Lines
set(LLVM_EXAMPLES_BINARY_DIR ${LLVM_BINARY_DIR}/examples) set(LLVM_EXAMPLES_BINARY_DIR ${LLVM_BINARY_DIR}/examples)
set(LLVM_INCLUDE_DIR ${CMAKE_CURRENT_BINARY_DIR}/include) set(LLVM_INCLUDE_DIR ${CMAKE_CURRENT_BINARY_DIR}/include)
# List of all targets to be built by default: # List of all targets to be built by default:
set(LLVM_ALL_TARGETS set(LLVM_ALL_TARGETS
AArch64 AArch64
AMDGPU AMDGPU
ARM ARM
AVR
BPF BPF
Hexagon Hexagon
Lanai Lanai
Mips Mips
MSP430 MSP430
NVPTX NVPTX
PowerPC PowerPC
RISCV RISCV
▲ Show 20 Lines • Show All 797 Lines • Show Last 20 Lines