This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Initial migration of AVR target
ClosedPublic

Authored by xiangzhai on May 8 2017, 11:10 PM.

Details

Summary

Hi LLVM developers,

I want to take place of avr-ld, it is from the AVR-GCC package, and Dylan forked LLVM's linker supporting AVR target before, but the structure of NEWLLD had been changed a lot! so I am preparing to migrate it, please point out my fault before I drove in the wrong direction too far away! and Dylan also suggested:

All of the AVR relocations have been implemented inside llvm for quite some time now. If we write AVR-LLD, we should factor out all of the fixup application code out of AVRAsmBackend.cpp and into a LLVM header file which can then be used from LLD.

So I have to do some work for llvm/lib/Target/AVR, correct?

Thanks a lot!

Regards,
Leslie Zhai

Diff Detail

Repository
rL LLVM

Event Timeline

xiangzhai created this revision.May 8 2017, 11:10 PM
ruiu edited edge metadata.May 8 2017, 11:34 PM

Hi Leslie,

Thank you for working on AVR support! I have a couple of questions for you.

  • I'm not familiar with AVR (I wrote a few programs for Arduino but haven't done any non-toy programming for the chip). Is there any ABI documentation with which I can learn?
  • Does it support full functionality of ELF? I guess it might be missing some features due to small address space. (I'm not even sure if that supports dynamic linking.)
  • Does it run Unix-like OS? Or is it usually used for baremetal programming?
xiangzhai added a comment.EditedMay 8 2017, 11:55 PM

Dear Rui,

Thanks for your reply!

I wrote a few programs for Arduino ... Is there any ABI documentation

I am also an Arduino fans since 2013, and I bought a UNO atmega328p Beginner Kit just for fun :) Dylan gathered some documents or just copy-and-paste from avr-gcc, I am a newbie in many fields, so I also have to learn a lot of manuals and source code directly.

I guess it might be missing some features due to small address space ...

Yes, it is not only migration but also need to implement some features.

Or is it usually used for baremetal programming?

UNO atmega328p is a single chip, very cheap! so I have not experienced running OS like Raspberry Pi, but just upload program by Arduino IDE to the board.

Regards,
Leslie Zhai

ruiu added a comment.May 9 2017, 12:14 AM

As I understand it, people do not usually run Unix-ish OS on AVR. You want to use it for baremetal programming. So, even though input files are ELF, output files are headerless binary. Naturally you don't use dynamic linking, which means most ELF features, such as PLT/GOT, are not used for AVR. Is this my understanding correct?

xiangzhai updated this revision to Diff 98256.May 9 2017, 12:31 AM

Dear Rui,

Thanks for your review!

As you suggested I have removed unnecessary functions, please review it, thanks a lot!

Regards,
Leslie Zhai

ruiu added a comment.May 9 2017, 12:40 AM

They were more like questions than review comments. I assumed you know much more than I about AVR, and I wanted to know more about AVR to see where AVR support would eventually go. What is your eventual goal for AVR support?

As a patch, I think this is too small to submit. I want to something that is minimally useful (in some sense), such as one that can link a program that does nothing but is still valid. This patch is too small that we cannot test.

Why do you need getDynRel, isPicRel, and needsThunk?

xiangzhai added a comment.EditedMay 9 2017, 1:01 AM

Dear Rui,

What is your eventual goal for AVR support?

Like you mentioned the goal is one that can link a program that does nothing but is still valid, so I just try to migrate Dylan's fork for test-the-waters at the very beginning, it is a long journey. and the sharp difference between old lld and NewLLD is a big chanllege to me.

This patch is too small that we cannot test.

Yes, it is just a bare-bones AVRTargetInfo to check whether or not I drove in the wrong direction, and you are my mentor :) you can always find the key point anyway thanks for your suggestion!

Regards,
Leslie Zhai

dylanmckay edited edge metadata.May 9 2017, 4:27 PM

Duplicating my comment here as I posted via email accidentally

Hey Rui,

You are correct in that we never run ELF files directly on the AVR - we copy the text and data sections out of the linked ELF file and flash them directly to the chip.

For example, do you want to support just bare-bone programs? Or ,do you want to eventually create a Unix-like OS on AVR which supports dynamic linking? Or something else?

We only want/need to support bare-bone programs. Obviously that means that there is no dynamic linker, which answers your question.

When I originally implemented AVR support from LLD, by far the biggest component that I needed to write was the AVR-specific relocation handling code. I was able to link program successfully, and I am fairly certain that we don't need anything more complex than that.

Naturally you don't use dynamic linking, which means most ELF features, such as PLT/GOT, are not used for AVR. Is this my understanding correct?

No PLT/GOT stuff at all.

Dear Dylan,

You, the owner of AVR backend, are always my mentor :)

the sharp difference between old lld and NewLLD

It is a big challenge to me, and I am learning Linux Kernel ELF implementation as extended reading, also your architecture related docs.

Regards,
Leslie Zhai

ruiu added a comment.May 9 2017, 5:02 PM

Thanks Dylan for explaining. From your explanation, I think supporting AVR is fairly easy. I'd think we probably need ~100 lines of code.

Leslie, can you add more code to this patch so that you can produce a minimal executable for AVR? Then I think you want to play with it to learn more about both AVR and LLD. For example, isPicRel, getDynRel and needsThunk should be irrelevant for AVR baremetal programs, but you added code for that in this patch.

Dear Rui,

Yes just for fun and learn! and the structure of old lld ELF/AVR is similar with LLVM's AVR target, for example, lib/Target/AVR, but NewLLD put different backends together, for example, Target.cpp so I have to be familiar with it and also need to carefully read related docs.

Regards,
Leslie Zhai

xiangzhai updated this revision to Diff 98581.May 10 2017, 9:46 PM

Dear Rui,

Sorry to bother you my mentor, but I have some questions:

  1. old lld just override the applyRelocation function, for example MipsRelocationHandler, to handle relocation, but NewLLD separate it into getImplicitAddend and relocateOne?
  1. old lld needs the tgtAddr, relAddr, or addend to calculateRelocation by special calculation depending on the kind of Type, but NewLLD just simply use SignExtend64<32>(read32le(Buf)) or some complicated one:
SignExtend64<20>(((Hi & 0x0400) << 10) | // S                            
                            ((Lo & 0x0800) << 8) |  // J2                           
                            ((Lo & 0x2000) << 5) |  // J1                           
                            ((Hi & 0x003f) << 12) | // imm6                         
                            ((Lo & 0x07ff) << 1));  // imm11:0

I doubt that I might drove in the wrong direction? it should not be simple as my illusion? so I only test R_AVR_32 at the very beginning.

  1. Are there any other bare-bones need to be implemented, for example handleAVRTlsRelocation in ELF/Relocations.cpp to produce a minimal executable for AVR?

Regards,
Leslie Zhai

ruiu added a comment.May 10 2017, 11:08 PM

Leslie,

The new ELF linker is completely different from the old one, and the new one is by far simpler than before. It should be much much easier to add a new architecture to the linker. So, in many cases it doesn't make sense or irrelevant to try to find a mapping from the old linker infrastructure to the new one to figure out what to do. Most boilerplates are no longer needed, and you can't find any correlation between old and new code in many situations. Rather, you want to read Target.cpp of the new LLD to understand how each target is handled and mimic other architectures.

What you are doing in this patch seems correct. Good work!

I believe with this change, you can actually link an executable as long as input object files contain nothing other than R_AVR_32 relocations. Did you try that? If not, please try. If it doesn't work, why? You want to create a new feature incrementally, and attempting to create an executable at this moment is a solid step towards a goal.

(By the way, you are saying that I'm a mentor. Are you a GSoC participant? I'm just wondering.)

include/lld/Core/Reference.h
55 ↗(On Diff #98581)

Revert this change -- this is not used by the ELF linker anymore. Basically you don't need to edit no files outside lld/ELF directory.

xiangzhai updated this revision to Diff 98585.EditedMay 10 2017, 11:44 PM

Dear Rui,

link an executable as long as input object files contain nothing other than R_AVR_32 relocations.

cat main.c the bare-bones example

void setup() {}

void loop() {}

compile main.c into the AVR ELF object file main.o https://reviews.llvm.org/D29827

clang -c --target=avr main.c -o main.o -mmcu=atmega328p

linking

ld.lld -o main main.o

But I have to look after my little kid now, I will check the output of linking tomorrow :)

By the way, you are saying that I'm a mentor. Are you a GSoC participant? I'm just wondering.

I respect you leading NewLLD, so respectfully call you 先生! I am not a GSoC partcipant, to me, as a KDE developer, I started to use gcc when I was a university student in 2002 and only developed a toy-compiler in Assembly as my "Compile Principle" home assignment, but not deep into the internals of gcc. During 2014, at iSOFTLinux, we tried to work on a GNU-free base system, so I switched to LLVM just for fun and learn, then tried to fix libclang segfault issue when parsing and Sema's destructor relative issue recently I am teaching the MallocChecker about Glib API just simply use AST, PathSensitive and ADT, so basically I am only a LLVM "user". so it is a long jouney to be familiar with LLVM by reading source code directly to reduce the gap between other smarter LLVM developers.

Regards,
Leslie Zhai

ruiu added inline comments.May 11 2017, 2:48 PM
ELF/Target.cpp
2424

In order to make R_AVR32_32 work, I think you need to change this to R_ABS.

grimar edited edge metadata.May 12 2017, 1:08 AM

This needs testcase.

xiangzhai updated this revision to Diff 98738.EditedMay 12 2017, 2:11 AM

Dear George,

Thanks for your reply!

This needs testcase.

I haven't written it under test or unittests directories, but instead use bare-bones:

#include <math.h>

#include "Bar.h"
#include "Foo.h"

void setup() {
  // put your setup code here, to run once:
  bar();
  foo();
  double s = sin(M_PI);
}

void loop() {
  // put your main code here, to run repeatedly:
}

#if __GNUC__
int main(int argc, char *argv[]) {
  return 0;
}
#endif

To test the relocation R_AVR_13_PCREL (haven't been implemented yet) by avr-ld (from the AVR-GCC RPM packages available for Fedora) and NewLLD, please see the diff:

readelf -r avr-gcc-BareMinimum.o

Relocation section '.rela.text' at offset 0x1d0 contains 2 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000d03 R_AVR_13_PCREL    00000000   bar + 0
00000002  00000e03 R_AVR_13_PCREL    00000000   foo + 0

with

readelf -r clang-BareMinimum.o

Relocation section '.rela.text' at offset 0xb8 contains 2 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000212 R_AVR_CALL        00000000   bar + 0
00000004  00000312 R_AVR_CALL        00000000   foo + 0

I doubt that I might drove in the wrong direction? it should not be simple as my illusion?

The relocation of R_AVR_32 in my patch might be wrong! so I am reading the source code of binutils and also Dylan's fork. but I have to look after my little kid in the weekend :) miss you, see you next week.

Regards,
Leslie Zhai

xiangzhai marked an inline comment as done.May 12 2017, 2:12 AM

Dear George,

Thanks for your reply!

This needs testcase.

I haven't written it under test or unittests directories

When we write patches we always use testcases. You can find them in llvm\tools\lld\test\ELF\*.
Things like new target always needs some, because testcase is a proof of work.

readelf -r avr-gcc-BareMinimum.o

Relocation section '.rela.text' at offset 0x1d0 contains 2 entries:
Offset Info Type Sym.Value Sym. Name + Addend
00000000 00000d03 R_AVR_13_PCREL 00000000 bar + 0
00000002 00000e03 R_AVR_13_PCREL 00000000 foo + 0

with

readelf -r clang-BareMinimum.o

Relocation section '.rela.text' at offset 0xb8 contains 2 entries:
Offset Info Type Sym.Value Sym. Name + Addend
00000000 00000212 R_AVR_CALL 00000000 bar + 0
00000004 00000312 R_AVR_CALL 00000000 foo + 0

So according to output you provide, there are R_AVR_13_PCREL and R_AVR_CALL relocations in objects
that you probably need to implement to link them.
Though I think minimal possible runnable code would not have calls to bar() or foo() and you would not have these
relocations. I would suggest to link and run minimal possible example with just empty setup() and loop() for start.

I doubt that I might drove in the wrong direction? it should not be simple as my illusion?

The relocation of R_AVR_32 in my patch might be wrong! so I am reading the source code of binutils and also Dylan's fork. but I have to look after my little kid in the weekend :) miss you, see you next week.

Regards,
Leslie Zhai

xiangzhai added a comment.EditedMay 12 2017, 2:32 AM

Dear George,

One more question, I have to gotta home :) why the Type of relocation by avr-gcc and clang --target=avr is different?

readelf -r avr-gcc-BareMinimum.o

Relocation section '.rela.text' at offset 0x1d0 contains 2 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000d03 R_AVR_13_PCREL    00000000   bar + 0
00000002  00000e03 R_AVR_13_PCREL    00000000   foo + 0
                   ^----- see

and

readelf -r clang-BareMinimum.o

Relocation section '.rela.text' at offset 0xb8 contains 2 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000212 R_AVR_CALL        00000000   bar + 0
00000004  00000312 R_AVR_CALL        00000000   foo + 0
                    ^----- see

Is the bug of LLVM's AVR target?

Regards,
Leslie Zhai

Dear George,

One more question, I have to gotta home :) why the Type of relocation by avr-gcc and clang --target=avr is different?

readelf -r avr-gcc-BareMinimum.o

Relocation section '.rela.text' at offset 0x1d0 contains 2 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000d03 R_AVR_13_PCREL    00000000   bar + 0
00000002  00000e03 R_AVR_13_PCREL    00000000   foo + 0
                   ^----- see

and

readelf -r clang-BareMinimum.o

Relocation section '.rela.text' at offset 0xb8 contains 2 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000212 R_AVR_CALL        00000000   bar + 0
00000004  00000312 R_AVR_CALL        00000000   foo + 0
                    ^----- see

Is the bug of LLVM's AVR target?

Regards,
Leslie Zhai

Honestly I don't know, I know nothing avout AVR. But I don't think it is a bug.
I guess you can try to link both object with a known workable avr linker and should be able to run them without problems.
Looks compilers just produce different relocations because of different code they produce probably,

I also agree that this is not an AVR-LLVM bug, it is simply that LLVM and GCC are selecting different instructions and lowering different relocations. There is no expectation that program X has the exact same relocations across different compilers.

I think it's good that you've only implemented R_AVR_32 because once this has landed, we should factor out the fixup application code in AVRAsmBackend.cpp and reuse it from lld.

So far as automated tests go, I think that we should be fine if we have a single test case to check that we can link an AVR executable with no relocations. This would verify that --target avr works okay. On top of that, we should also have a test for the R_AVR_32 relocation.

I'm not entirely sure what LLD tests look like, but if it's anything like LLVM tests, it will be quite straightforward to figure out from looking at existing files in the tests/ directory.

Nice work by the way!

xiangzhai updated this revision to Diff 99111.EditedMay 16 2017, 1:23 AM

Sorry for my late reply, I have to look after my wife she had cold :(

Dear Dylan,

we should factor out the fixup application code in AVRAsmBackend.cpp and reuse it from lld.

Please leading me, as my mentor, I have no direction about lib/Target/AVR/MCTargetDesc and relocation type R_AVR_CALL has not been implemented yet so I need to read the source code of binutils about Relocation for CALL command in ATmega.

Dear George,

Things like new target always needs some, because testcase is a proof of work.

I am not familiar with how to write the testcase for lld, it is much easier for clang analyzer but I might make stupid mistakes for avr-call.s please point out my fault, thanks a lot!

Though I think minimal possible runnable code would not have calls to bar() or foo() and you would not have these relocations. I would suggest to link and run minimal possible example with just empty setup() and loop() for start.

empty setup() and loop() is able to work, but Dylan's fork

When I originally implemented AVR support from LLD, by far the biggest component that I needed to write was the AVR-specific relocation handling code. I was able to link program successfully, and I am fairly certain that we don't need anything more complex than that.

so I have to implement R_AVR_32 and R_AVR_CALL as Initial migration of AVR target at the very beginning.

I guess you can try to link both object with a known workable avr linker and should be able to run them without problems.

Yes! avr-ld works :)

Regards,
Leslie Zhai

grimar added inline comments.May 16 2017, 1:50 AM
ELF/Target.cpp
2427

Why do you need this ?

2449

We do not do like that. If some relocation is not implemented, the error below will catch it anyways, so you do not need to have logic for R_AVR_CALL.

test/ELF/avr-call.s
5 ↗(On Diff #99111)

So here you're dumping disassembly what looks right direction, but you do not check output anywhere.

FileCheck will search for "CHECK" label by default to perform validating,
Please review how "CHECK:" is used in other our testcases with llvm-objdump -d.

7 ↗(On Diff #99111)

That is my personal opinion, and other reviewers probably have different one, but personally I prefer to put requirements for test on a first line to make clear what needed for run.

9 ↗(On Diff #99111)

Does not seem loc is used anywhere ?

15 ↗(On Diff #99111)

So you have R_AVR_CALL relocation in code, but your implementation does not implement it and errors out. I think you want to implement it to write proper testcase.

ruiu added a comment.May 16 2017, 10:49 AM

Did you try to run a program generated with this patch on an actual AVR (or a simulator)? Did it work?

ELF/Target.cpp
251

Move this above MipsTargetInfo.

253

Since this constructor is the same as the default constructor, remove this line.

297–298

Move this after EM_ARM as these enums are sorted in alphabetical order.

2422

Move these new functions above Mips.

2425–2426

I think what you want to do for now is to return R_ABS for R_AVR_32, and report an error for any other relocation type just like other getRelExprs do.

2449

Yes, please remove unless you actually implement R_AVR_CALL.

test/ELF/avr-call.s
1 ↗(On Diff #99111)

For consistency with other architectures, you want to name this file basic-avr.s.

xiangzhai updated this revision to Diff 99251.May 17 2017, 12:31 AM

Dear Rui,

Did you try to run a program generated with this patch on an actual AVR (or a simulator)? Did it work?

I only tested BareMinimum on an AVR simulator:

simavr -m atmega328p clang-BareMinimum
Loaded 10 .text at address 0x0
avr_gdb_init listening on port 1234

For an actual AVR atmega328p chip, I have to implement all reloc types as binutils , but I have no idea about AVR only for GCC __builtin functions, for example:

#ifndef F_CPU
#define F_CPU 1000000UL
#endif
#include <util/delay.h>


int main(int argc, char *argv[]) {
#if __clang__
#pragma message("AVR __builtin only for GCC")
#else
  _delay_ms(1000); // __builtin_avr_delay_cycles
#endif
  return 0;
}

Although in addition to the GCC builtins, Clang supports a number of builtins that GCC does not AVR fans might choose AVR GNU toolchains because easy-to-use Arduino IDE, the same story is people often use GNU compiler for Linux Kernel but not the patch please give me some LLVM Belief support :) thanks a lot!

Regards,
Leslie Zhai

xiangzhai marked 8 inline comments as done.May 17 2017, 12:32 AM
xiangzhai updated this revision to Diff 99255.May 17 2017, 12:37 AM

make check-lld to check the testcase:

...
[100%] Running lld test suite
Testing Time: 28.58s
  Expected Passes    : 1125
  Unsupported Tests  : 12
[100%] Built target check-lld

Dear Dylan,

LLVM AVR backend's printInstruction issue:

generated by clang

lldb /data/project/LLVM/llvm/build/bin/llvm-objdump
(lldb) target create "/data/project/LLVM/llvm/build/bin/llvm-objdump"
Current executable set to '/data/project/LLVM/llvm/build/bin/llvm-objdump' (x86_64).
(lldb) r -d clang-BareMinimum
Process 16278 launched: '/data/project/LLVM/llvm/build/bin/llvm-objdump' (x86_64)

clang-BareMinimum:      file format ELF32-avr

Disassembly of section .text:
setup:
   11000:       08 95   ret

loop:
   11002:       08 95   ret

main:
   11004:       80 e0   ldi     0, Process 16278 stopped
* thread #1, name = 'llvm-objdump', stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
    frame #0: 0x00007ffff6ca35d3 libLLVM-5.0svn.so`llvm::MCExpr::print(llvm::raw_ostream&, llvm::MCAsmInfo const*, bool) const + 67
libLLVM-5.0svn.so`llvm::MCExpr::print:
->  0x7ffff6ca35d3 <+67>: movl   (%rbx), %eax
    0x7ffff6ca35d5 <+69>: cmpq   $0x3, %rax
    0x7ffff6ca35d9 <+73>: ja     0x7ffff6ca3ae7            ; <+1367>
    0x7ffff6ca35df <+79>: movslq (%rbp,%rax,4), %rax
(lldb) bt
* thread #1, name = 'llvm-objdump', stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
  * frame #0: 0x00007ffff6ca35d3 libLLVM-5.0svn.so`llvm::MCExpr::print(llvm::raw_ostream&, llvm::MCAsmInfo const*, bool) const + 67
    frame #1: 0x00007ffff7912c88 libLLVM-5.0svn.so`llvm::AVRInstPrinter::printInstruction(llvm::MCInst const*, llvm::raw_ostream&) + 872
    frame #2: 0x00007ffff7913858 libLLVM-5.0svn.so`llvm::AVRInstPrinter::printInst(llvm::MCInst const*, llvm::raw_ostream&, llvm::StringRef, llvm::MCSubtargetInfo const&) + 520
    frame #3: 0x0000000000226c9f llvm-objdump`(anonymous namespace)::PrettyPrinter::printInst(llvm::MCInstPrinter&, llvm::MCInst const*, llvm::ArrayRef<unsigned char>, unsigned long, llvm::raw_ostream&, llvm::StringRef, llvm::MCSubtargetInfo const&, (anonymous namespace)::SourcePrinter*) + 207
    frame #4: 0x0000000000234ec0 llvm-objdump`DisassembleObject(llvm::object::ObjectFile const*, bool) + 14288
    frame #5: 0x0000000000230e1f llvm-objdump`DumpObject(llvm::object::ObjectFile*, llvm::object::Archive const*) + 687
    frame #6: 0x0000000000226a4d llvm-objdump`main + 1309
    frame #7: 0x00007ffff3b902f1 libc.so.6`__libc_start_main(main=(llvm-objdump`main), argc=3, argv=0x00007fffffffdaa8, init=<unavailable>, fini=<unavailable>, rtld_fini=<unavailable>, stack_end=0x00007fffffffda98) at libc-start.c:289
    frame #8: 0x000000000022102a llvm-objdump`_start + 42
(lldb)

or by avr-gcc

lldb /data/project/LLVM/llvm/build/bin/llvm-objdump
(lldb) target create "/data/project/LLVM/llvm/build/bin/llvm-objdump"
Current executable set to '/data/project/LLVM/llvm/build/bin/llvm-objdump' (x86_64).
(lldb) r -d avr-gcc-BareMinimum
Process 15772 launched: '/data/project/LLVM/llvm/build/bin/llvm-objdump' (x86_64)

avr-gcc-BareMinimum:    file format ELF32-avr

Disassembly of section .text:
__vectors:
       0:       0c 94 34 00  <unknown>
       4:       0c 94 3e 00  <unknown>
       8:       0c 94 3e 00  <unknown>
       c:       0c 94 3e 00  <unknown>
      10:       0c 94 3e 00  <unknown>
      14:       0c 94 3e 00  <unknown>
      18:       0c 94 3e 00  <unknown>
      1c:       0c 94 3e 00  <unknown>
      20:       0c 94 3e 00  <unknown>
      24:       0c 94 3e 00  <unknown>
      28:       0c 94 3e 00  <unknown>
      2c:       0c 94 3e 00  <unknown>
      30:       0c 94 3e 00  <unknown>
      34:       0c 94 3e 00  <unknown>
      38:       0c 94 3e 00  <unknown>
      3c:       0c 94 3e 00  <unknown>
      40:       0c 94 3e 00  <unknown>
      44:       0c 94 3e 00  <unknown>
      48:       0c 94 3e 00  <unknown>
      4c:       0c 94 3e 00  <unknown>
      50:       0c 94 3e 00  <unknown>
      54:       0c 94 3e 00  <unknown>
      58:       0c 94 3e 00  <unknown>
      5c:       0c 94 3e 00  <unknown>
      60:       0c 94 3e 00  <unknown>
      64:       0c 94 3e 00  <unknown>

__trampolines_start:
      68:       11 24   eor     Process 15772 stopped
* thread #1, name = 'llvm-objdump', stop reason = signal SIGSEGV: invalid address (fault address: 0x6)
    frame #0: 0x00007ffff6ca35ed libLLVM-5.0svn.so`llvm::MCExpr::print(llvm::raw_ostream&, llvm::MCAsmInfo const*, bool) const + 93
libLLVM-5.0svn.so`llvm::MCExpr::print:
->  0x7ffff6ca35ed <+93>:  movl   (%rdi), %eax
    0x7ffff6ca35ef <+95>:  decl   %eax
    0x7ffff6ca35f1 <+97>:  cmpl   $0x1, %eax
    0x7ffff6ca35f4 <+100>: ja     0x7ffff6ca3685            ; <+245>
(lldb) bt
* thread #1, name = 'llvm-objdump', stop reason = signal SIGSEGV: invalid address (fault address: 0x6)
  * frame #0: 0x00007ffff6ca35ed libLLVM-5.0svn.so`llvm::MCExpr::print(llvm::raw_ostream&, llvm::MCAsmInfo const*, bool) const + 93
    frame #1: 0x00007ffff7912a83 libLLVM-5.0svn.so`llvm::AVRInstPrinter::printInstruction(llvm::MCInst const*, llvm::raw_ostream&) + 355
    frame #2: 0x00007ffff7913858 libLLVM-5.0svn.so`llvm::AVRInstPrinter::printInst(llvm::MCInst const*, llvm::raw_ostream&, llvm::StringRef, llvm::MCSubtargetInfo const&) + 520
    frame #3: 0x0000000000226c9f llvm-objdump`(anonymous namespace)::PrettyPrinter::printInst(llvm::MCInstPrinter&, llvm::MCInst const*, llvm::ArrayRef<unsigned char>, unsigned long, llvm::raw_ostream&, llvm::StringRef, llvm::MCSubtargetInfo const&, (anonymous namespace)::SourcePrinter*) + 207
    frame #4: 0x0000000000234ec0 llvm-objdump`DisassembleObject(llvm::object::ObjectFile const*, bool) + 14288
    frame #5: 0x0000000000230e1f llvm-objdump`DumpObject(llvm::object::ObjectFile*, llvm::object::Archive const*) + 687
    frame #6: 0x0000000000226a4d llvm-objdump`main + 1309
    frame #7: 0x00007ffff3b902f1 libc.so.6`__libc_start_main(main=(llvm-objdump`main), argc=3, argv=0x00007fffffffda98, init=<unavailable>, fini=<unavailable>, rtld_fini=<unavailable>, stack_end=0x00007fffffffda88) at libc-start.c:289
    frame #8: 0x000000000022102a llvm-objdump`_start + 42
(lldb)

Regards,
Leslie Zhai

ruiu added a comment.May 17 2017, 10:32 AM

As to builtin functions for AVR that GCC has but Clang doesn't, you might want to add that to Clang, but that is not directly related to LLD. So I'll defer that question to someone more familiar with Clang. If you need to use GCC to generate object files to test LLD, just use GCC.

ELF/InputFiles.cpp
806–807 ↗(On Diff #99255)

Move above Triple::mips.

ELF/Target.cpp
2051–2052

Return R_ABS only for R_AVR_32 and report an error for any other relocation for now, as you are not actually handling anything other than R_AVR_32.

See X86_64TargetInfo<ELFT>::getRelExp.

2072–2073

It doesn't seem correct. If you are not handling R_AVR_CALL now, please remove. If you want, please add actual code to handle R_AVR_CALL.

test/ELF/basic-avr.s
1 ↗(On Diff #99255)

This assembly file doesn't contain anything other than comment. Add at least one instruction like this.

main:
  jmp main
4–5 ↗(On Diff #99255)

I don't think you need this. Just check the output of objdump -d.

7 ↗(On Diff #99255)

Move this to beginning of this file.

12 ↗(On Diff #99255)

Remove

15 ↗(On Diff #99255)

Remove

xiangzhai updated this revision to Diff 99411.May 18 2017, 2:16 AM

Dear Rui,

Thanks for your review!

If you need to use GCC to generate object files to test LLD, just use GCC

Good idea! I will use avr-gcc and ld.lld :)

Regards,
Leslie Zhai

xiangzhai marked 8 inline comments as done.May 18 2017, 2:17 AM
ruiu added a comment.May 18 2017, 8:28 AM

Did you try to run this code on a simulator?

main:
  jmp main

If not, please do. If it works (by "work", I mean the program successfully spins in the main function), I think this patch is ready to submit.

I tried to run llvm-mc to check which relocations are produced here but got an error:

/home/umb/LLVM/llvm-build/bin/llvm-mc: : error: unable to get target for 'avr-unknown-linux', see --version and --triple.

Could anyone point me what is wrong ?

test/ELF/basic-avr.s
11 ↗(On Diff #99411)

I think you need to add space here: # CHECK-NEXT: main:
and explain what is 80 e0 in comments, like we do in other tests.

ruiu added a comment.May 19 2017, 11:06 AM

Please verify that the following program spins inside function foo.

main:
  call foo

foo:
  jmp foo

Please update basic-avr.s with this test.

ELF/Target.cpp
2062

I asked you to return R_ABS for R_AVR_CALL, but I don't know about the AVR instruction set, so I may be wrong. Is R_AVR_CALL is actually an absolute relocation? It may be a relative one.

xiangzhai added a comment.EditedMay 19 2017, 7:58 PM

Dear Rui,

elf32-avr Not PC relative:

/* Relocation for CALL command in ATmega.  */
HOWTO (R_AVR_CALL,            /* type */
       1,                     /* rightshift */
       2,                     /* size (0 = byte, 1 = short, 2 = long) */
       23,                    /* bitsize */
       FALSE,                 /* pc_relative */
       0,                     /* bitpos */
       complain_overflow_dont,/* complain_on_overflow */
       bfd_elf_generic_reloc, /* special_function */
       "R_AVR_CALL",          /* name */
       FALSE,                 /* partial_inplace */
       0xffffffff,            /* src_mask */
       0xffffffff,            /* dst_mask */
       FALSE),                        /* pcrel_offset */

I am on vacation, not available until May 31th, miss you, see you next month :)

Hey Leslie,

Please leading me, as my mentor, I have no direction about lib/Target/AVR/MCTargetDesc and relocation type R_AVR_CALL has not been implemented yet so I need to read the source code of binutils about Relocation for CALL command in ATmega.

For this change, don't worry too much about it as you've only implemented one relocation type.

In the future, we should refactor LLVM so that it is something like this quick dirty patch

That way, we can use the same relocation code in LLD and LLVM.

xiangzhai updated this revision to Diff 101851.Jun 7 2017, 10:04 PM

Dear,

Sorry for my late response, I am doing my company's work assignment about L4 recently, so I couldn't quickly response to LLVM, KDE and other open source projects right now, sorry again!

Dear Rafael,

Perhaps Dylan's AVR target need to be migrated something else for fixing llvm-mc not enabled instruction issue?

llvm-mc -filetype=obj -triple=avr-unknown-linux ../tools/lld/test/ELF/basic-avr.s -o /tmp/basic-avr.o
../tools/lld/test/ELF/basic-avr.s:7:3: error: instruction requires a CPU feature not currently enabled
  call foo
  ^
../tools/lld/test/ELF/basic-avr.s:10:3: error: instruction requires a CPU feature not currently enabled
  jmp foo
  ^

Regards,
Leslie Zhai

ruiu added a comment.Jun 7 2017, 10:29 PM

You are not handling R_AVR_CALL only in getImplicitAddend().

As I said, you need to handle the same relocations in getRelExpr(), getImplicitAddend(), relocateOne() and a test file. You can choose an arbitrary subset of relocations for the first implementation, but after you make a selection, you need to stick with it.

but after you make a selection, you need to stick with it.

I will!

xiangzhai updated this revision to Diff 101864.EditedJun 8 2017, 12:58 AM

Dear Rui,

you need to handle the same relocations in getRelExpr(), getImplicitAddend(), relocateOne()

I followed elf32-avr R_AVR_32 and R_AVR_CALL to handle the relocations in getRelExpr, getImplicitAddend and relocateOne.

a test file

Dear Rafael,

but I met llvm-mc not enabled instruction issue:

/home/zhaixiang/project/llvm/build/bin/llvm-mc -filetype=obj -triple=avr-unknown-linux ../tools/lld/test/ELF/basic-avr.s -o /tmp/basic-avr.o
../tools/lld/test/ELF/basic-avr.s:7:3: error: instruction requires a CPU feature not currently enabled
  call foo
  ^
../tools/lld/test/ELF/basic-avr.s:10:3: error: instruction requires a CPU feature not currently enabled
  jmp foo
  ^

Please point out my fault, thanks a lot!

but after you make a selection, you need to stick with it

Rui, you are my example, you are learning in the University and meanwhile working in the company, it is very hard game mode, I am too lazy, often use child and work as my excuse, ashamed...

Regards,
Leslie Zhai

ruiu added a comment.Jun 8 2017, 12:59 PM

I'm no expert of AVR support of LLVM, but you need to enable AVR backend when building your LLVM, I guess?

../tools/lld/test/ELF/basic-avr.s:7:3: error: instruction requires a CPU feature not currently enabled

call foo
^

../tools/lld/test/ELF/basic-avr.s:10:3: error: instruction requires a CPU feature not currently enabled

jmp foo

This is caused because unless you specify a target mcu, it will default to the bare minimum - the avr2 architecture, as with avr-gcc. Devices that fit into this category do not even have call/jmp instructions, and so you likely just need to pass something like atmega328p as the target CPU to llvm-mc.

xiangzhai updated this revision to Diff 101987.Jun 8 2017, 7:22 PM

Dear Dylan,

Thanks for your hint!

This is caused because unless you specify a target mcu, it will default to the bare minimum - the avr2 architecture, as with avr-gcc. Devices that fit into this category do not even have call/jmp instructions, and so you likely just need to pass something like atmega328p as the target CPU to llvm-mc.

Worked :)

$ /home/zhaixiang/project/llvm/build/bin/llvm-mc -filetype=obj -triple=avr-unknown-linux -mcpu=atmega328p basic-avr.s -o /tmp/basic-avr.o
$ file /tmp/basic-avr.o
/tmp/basic-avr.o: ELF 32-bit LSB relocatable, Atmel AVR 8-bit, version 1 (SYSV), not stripped
$ readelf -r /tmp/basic-avr.o

Relocation section '.rela.text' at offset 0x7c contains 2 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000312 R_AVR_CALL        00000000   .text + 4
00000004  00000312 R_AVR_CALL        00000000   .text + 4

Dear Rui,

but you need to enable AVR backend when building your LLVM, I guess?

$ /home/zhaixiang/project/llvm/build/bin/llvm-mc --version
LLVM (http://llvm.org/):
  LLVM version 5.0.0svn
  Optimized build.
  Default target: x86_64-redhat-linux
  Host CPU: broadwell

  Registered Targets:
    ...
    avr        - Atmel AVR Microcontroller
    ...

And I run the testcase:

$ make check-lld
[100%] Running lld test suite
Testing Time: 27.90s
  Expected Passes    : 1146
  Unsupported Tests  : 12
[100%] Built target check-lld

PS: @dylanmckay please update Buildbot's build optioin, I have experienced False Positive build failures, I directly added AVR to LLVM_ALL_TARGETS instead of -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD="AVR" D33348 thoughts?

Then run for the AVR simulator:

$ simavr -m atmega328p ./basic-avr
Loaded 8 .text at address 0x0
avr_gdb_init listening on port 1234

Regards,
Leslie Zhai

Dear Dylan,

Thanks for your patch!

In the future, we should refactor LLVM so that it is something like this quick dirty patch

That way, we can use the same relocation code in LLD and LLVM.

Sorry again for my late response, recently I am doing my company's work assignment about L4 microkernel, and it is really really really difficult to debug with JDB in QEMU http://os.inf.tu-dresden.de/pipermail/l4-hackers/2017/008051.html but I will try to catch up with you :)

Regards,
Leslie Zhai

ruiu added a comment.Jun 8 2017, 7:53 PM

How can I test this? Looks like my AVR target is not enabled in my LLVM build. What is the CMAKE option to enable it?

Dear Rui,

How can I test this? Looks like my AVR target is not enabled in my LLVM build. What is the CMAKE option to enable it?

Plan A: I directly added AVR into LLVM_ALL_TARGETS D33348

Plan B: -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD="AVR"
but llvm/tools/llvm-mc/LLVMBuild.txt just required_libraries = ... all-targets EXPERIMENTAL_TARGETS is not be included in all-targets, right?

So I am using Plan A for my local environment, thoughts?

Regards,
Leslie Zhai

ruiu added a comment.Jun 8 2017, 8:40 PM

Your test expects the following outputs.

11000:       02 88 00 00  <unknown>
11004:       02 88 00 00  <unknown>

It seems that llvm-objdump failed to decode the instructions. Does this mean the output is actually broken, or is it just that llvm-objdump -d can't decode instructions for AVR yet?

Dear Rui,

What about avr-objdump?

$ avr-objdump -v
GNU objdump (GNU Binutils) 2.27
Copyright (C) 2016 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) any later version.
This program has absolutely no warranty.

$ avr-objdump -d basic-avr

basic-avr:     file format elf32-avr


Disassembly of section .text:

00011000 <main>:
   11000:       02 88           ldd     r0, Z+18        ; 0x12
        ...

00011004 <foo>:
   11004:       02 88           ldd     r0, Z+18        ; 0x12
        ...

basic-avr was generated by:

$ llvm/build/bin/llvm-mc -filetype=obj -triple=avr-unknown-linux -mcpu=atmega328p llvm/tools/lld/test/ELF/basic-avr.s -o /tmp/basic-avr.o
$ llvm/build/bin/lld /tmp/basic-avr.o -o /tmp/basic-avr

Regards,
Leslie Zhai

davide added a subscriber: davide.Jun 8 2017, 8:56 PM

Dear Rui,

What about avr-objdump?

$ avr-objdump -v
GNU objdump (GNU Binutils) 2.27
Copyright (C) 2016 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) any later version.
This program has absolutely no warranty.

$ avr-objdump -d basic-avr

basic-avr:     file format elf32-avr


Disassembly of section .text:

00011000 <main>:
   11000:       02 88           ldd     r0, Z+18        ; 0x12
        ...

00011004 <foo>:
   11004:       02 88           ldd     r0, Z+18        ; 0x12
        ...

basic-avr was generated by:

$ llvm/build/bin/llvm-mc -filetype=obj -triple=avr-unknown-linux -mcpu=atmega328p llvm/tools/lld/test/ELF/basic-avr.s -o /tmp/basic-avr.o
$ llvm/build/bin/lld /tmp/basic-avr.o -o /tmp/basic-avr

Regards,
Leslie Zhai

Yes, llvm-objdump (i.e. MC) misses the decoding for these two instructions. You may want to add that first.

Dear David,

Thanks for your hint!

In the future, we should refactor LLVM so that it is something like this quick dirty patch

That way, we can use the same relocation code in LLD and LLVM.

@dylanmckay And I noticed that Dylan moved fixup_call from lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp into include/llvm/Support/AVRRelocations.h so other LLVM's tools such as lld, llvm-objdump, etc are able to reuse the same relocaiton code, correct? then this patch is depends on Dylan's work, he is the owner of AVR target, but I could not confirm that I have enough time to work together with him owing to work assignment :(

Regards,
Leslie Zhai

@xiangzhai: That patch was just a super quick thing I wrote up to illustrate how we should factor out the fixup application code so that LLD can use it.

Don't worry about it for this change, as you've only implemented R_AVR_CALL. When we go to implement more relocation types in lld, we should look again and see if it's worth sharing code between LLD and LLVM for this.

Dear Dylan,

we should look again and see if it's worth sharing code between LLD and LLVM for this.

it worth!

Regards,
Leslie Zhai

Dear David,

Thanks for your response!

Yes, llvm-objdump (i.e. MC) misses the decoding for these two instructions. You may want to add that first.

Please show me where (for example, which files under llvm/lib/Target/AVR/MCTargetDesc) I could implement the missing decoding part for Machine Code related, thanks a lot!

Regards,
Leslie Zhai

The file you want is in lib/Target/AVR/Disassembler/AVRDisassembler.cpp

xiangzhai added a comment.EditedJun 12 2017, 7:47 PM

Dear Dylan,

Thanks for your hint!

The file you want is in lib/Target/AVR/Disassembler/AVRDisassembler.cpp

AVR disassembler is bare-bones? there are only empty Decode functions , correct? but compare with Mips, there are real Decode functions .

so I need to read about GNU binutils' objdump to find out how GCC disassemble AVR, thoughts?

Regards,
Leslie Zhai

AVR disassembler is bare-bones?

Yes, very. I had essentially just copied the Mips disassembler and then removed all of the Mips-specific stuff.

so I need to read about GNU binutils' objdump to find out how GCC disassemble AVR, thoughts?

I wouldn't say that. Most of our instructions can be disassembled already, even though the disassembler is bare boned. During compilation, LLVM run's the TableGen command on the AVRInstrInfo.td file to generate disassembly tables for all instructions that it can. Because of this, the disassembler can already disassemble a number of programs..

The issue arises when there are ambiguities or custom encodings. A good example is the ldd instruction. There is an opcode bit which is set depending on which registers are given as operands. We have a custom hook (in AVRMCCodeEmitter.cpp) to set that bit depending manually. Because of this, disassembly for this instruction will not always work because there is no custom code to handle this in the disassembler yet and it obviously cannot disassemble just looking at the tables.

From my understanding, Davide wants you to modify the disassembler so that two specific instructions have custom code for disassembly. I am not entirely sure what that would look like because it has been a long time since I've looked at disassembly.

Dear Dylan,

Thanks for your kind response!

so I need to read about GNU binutils' objdump to find out how GCC disassemble AVR, thoughts?

I wouldn't say that. Most of our instructions can be disassembled already, even though the disassembler is bare boned. During compilation, LLVM run's the TableGen command on the AVRInstrInfo.td file to generate disassembly tables for all instructions that it can. Because of this, the disassembler can already disassemble a number of programs..

Could you please kindly show me the testcase for proving AVRInstrInfo.td is correct? git log commit info, thanks!

The issue arises when there are ambiguities or custom encodings. A good example is the ldd instruction. There is an opcode bit which is set depending on which registers are given as operands. We have a custom hook (in AVRMCCodeEmitter.cpp) to set that bit depending manually. Because of this, disassembly for this instruction will not always work because there is no custom code to handle this in the disassembler yet and it obviously cannot disassemble just looking at the tables.

Yes, there are more real Encode functions in the AVRMCCodeEmitter, the same story, testcase please.

From my understanding, Davide wants you to modify the disassembler so that two specific instructions have custom code for disassembly. I am not entirely sure what that would look like because it has been a long time since I've looked at disassembly.

OK, I need to learn about disassembly :)

Let's compare with GNU cross compiler toolchain for AVR target:

$ cat basic-avr.s

main:
  call foo

foo:
  jmp foo

Then build with avr-gcc

$ avr-gcc -o avr-gcc-basic-avr.o -c -mmcu=atmega328p basic-avr.s

$ file avr-gcc-basic-avr.o 
avr-gcc-basic-avr.o: ELF 32-bit LSB relocatable, Atmel AVR 8-bit, version 1 (SYSV), not stripped

$ readelf -r avr-gcc-basic-avr.o 

Relocation section '.rela.text' at offset 0xa8 contains 2 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000112 R_AVR_CALL        00000000   .text + 4
00000004  00000112 R_AVR_CALL        00000000   .text + 4

And avr-objdump

$ avr-objdump -d avr-gcc-basic-avr.o 

avr-gcc-basic-avr.o:     file format elf32-avr


Disassembly of section .text:

00000000 <main>:
   0:   0e 94 00 00     call    0       ; 0x0 <main>

00000004 <foo>:
   4:   0c 94 00 00     jmp     0       ; 0x0 <main>

and LLVM's llvm-mc, ld.lld, and llvm-objdump shown above .

Regards,
Leslie Zhai

ruiu added a comment.Jun 13 2017, 5:26 PM

Leslie,

This patch is work-in-progress for more than a month, and looks like it needs some more time. That is understandable -- it is probably not that easy to land a first patch for a new architecture. So I decided to take a little time to help you. I installed avr-{gcc,objdump,as,ld} using apt-get and read the Wikipedia page of the AVR instruction set (I should have read the official instruction manual, but what's on the Wikipedia page seems correct.)

What I found are:

  • R_AVR_32 is not used nor tested in your patch. You want to simply remove it.
  • .text segments in AVR ELF outputs should be linked against absolute address zero because AVR is a Harvard-architecture microcontroller on which programs usually reside at address zero. Your output is on the other hand linked against address 0x11000. That's too high for AVR. You want to pass -Ttext=0 to the linker so that .text sections are linked against 0.
  • The AVR ELF seems to use RELA as opposed to REL. That means relocation addends are in relocation tables and not in relocated segments. So you don't need to write getImplicitAddend().
  • The relocation is not applied correctly. Operands of JMP or CALL instructions are 22-bit long, and it can only jump to even-numbered addresses (as all instructions are 2 byte long). So, the least significant bit is implicit and always 0. JMP or CALL instructions are in the following form in binary in an object file

    1001 0100 0000 11x0 0000 0000 0000 0000

    and after applying a R_AVR_CALL relocation with a 22-bit address aa aaab cccc cccc cccc cccc (in binary), the result should look like

    1001 010a aaaa 11xb cccc cccc cccc cccc

    You can see the instruction encoding here: https://en.wikipedia.org/wiki/Atmel_AVR_instruction_set#Instruction_encoding

I fixed these problems based on your patch. See it here: https://github.com/rui314/llvm-project/commit/880cd5c1c262169dc8bc6b36b5824dd2acfa2132. I confirmed that the output matches with the avr-ld.bfd's output.

Can you update your patch based on mine?

xiangzhai updated this revision to Diff 102474.Jun 13 2017, 9:55 PM

Dear Rui,

Thanks for your kind teaching!

This patch is work-in-progress for more than a month, and looks like it needs some more time. That is understandable -- it is probably not that easy to land a first patch for a new architecture.

Thanks for your comfort, but that is not understandable, as Chinese old saying:

难者不会,会者不难

To me, a newbie without direction, it cost me more than a month; but to you, full stack expertise ranges from HTML/JavaScript to hard-core systems programming, might more than a hour.

So I decided to take a little time to help you.

I feel guilty that you are so busy but still help me, nobody (can) help me in my surrounding:

朝の8時半から試験っていうのは慌ただしい。

and lack of sleep:

昼の会社のソファーでちょっと寝てしまった。やや寝不足。

I would like to sincerely thank you, if I traveled to 大阪, and when you were visiting relatives.

I installed avr-{gcc,objdump,as,ld} using apt-get and read the Wikipedia page of the AVR instruction set (I should have read the official instruction manual, but what's on the Wikipedia page seems correct.)

Yes, there is the same story for elf32-mips about R_MIPS_PC16 and MIPS GOT you mentioned.

  • .text segments in AVR ELF outputs should be linked against absolute address zero because AVR is a Harvard-architecture microcontroller on which programs usually reside at address zero. Your output is on the other hand linked against address 0x11000. That's too high for AVR. You want to pass -Ttext=0 to the linker so that .text sections are linked against 0.

Thanks for your hint, I also used GNU avr-ld as:

$ avr-ld -o avr-gcc-basic-avr avr-gcc-basic-avr.o -Ttext=0
  • The AVR ELF seems to use RELA as opposed to REL. That means relocation addends are in relocation tables and not in relocated segments. So you don't need to write getImplicitAddend().

bookmarked.

  • The relocation is not applied correctly. Operands of JMP or CALL instructions are 22-bit long, and it can only jump to even-numbered addresses (as all instructions are 2 byte long). So, the least significant bit is implicit and always 0. JMP or CALL instructions are in the following form in binary in an object file

    1001 0100 0000 11x0 0000 0000 0000 0000

    and after applying a R_AVR_CALL relocation with a 22-bit address aa aaab cccc cccc cccc cccc (in binary), the result should look like

    1001 010a aaaa 11xb cccc cccc cccc cccc

    You can see the instruction encoding here: https://en.wikipedia.org/wiki/Atmel_AVR_instruction_set#Instruction_encoding

I fixed these problems based on your patch. See it here: https://github.com/rui314/llvm-project/commit/880cd5c1c262169dc8bc6b36b5824dd2acfa2132. I confirmed that the output matches with the avr-ld.bfd's output.

I am stupid to wrongly translate elf32-avr about R_AVR_CALL to NewLLD, I need to read more source code and doc.

$ /home/zhaixiang/project/llvm/build/bin/llvm-objdump -d /tmp/basic-avr

/tmp/basic-avr: file format ELF32-avr

Disassembly of section .text:
main:
       0:       0e 94 02 00  <unknown>

foo:
       4:       0c 94 02 00  <unknown>

As David and Dylan suggested, I will try to implement the decode part by other patch.

Can you update your patch based on mine?

It is my pleasure :) and I updated the patch, please review it, if LGTU, may I commit it?

Regards,
Leslie Zhai

ruiu accepted this revision.Jun 14 2017, 10:54 AM

LGTM. Please commit.

This revision is now accepted and ready to land.Jun 14 2017, 10:54 AM
This revision was automatically updated to reflect the committed changes.