[PATCH] D37615: [AVR] implement the relocation of LDI for LLD

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 8 13:29:06 PDT 2017


ruiu added a comment.

Even though I do not fully understand what linkers should do for AVR, I don't think this patch is correct and ready for code review. For example, doing nothing for `R_AVR_HI8_LDI` seems apparently odd. I believe it happens to generate the same result as ld.bfd because in the given test case, high address happens to be zero.

Firstly, can you give me a documentation about AVR relocations? I cannot review this patch without knowing the relocations this patch is trying to implement.

Secondly, can you review your patch yourself until you think everything perfectly makes sense? Obviously, not implementing `R_AVR_HI8_LDI` is odd, for example, and I don't have enough time to investigate all questions that raised when I was reading your patch (I did that this time to see if `R_AVR_HI8_LDI` is not a nop relocation though). Last AVR patch (https://reviews.llvm.org/D32991) took 70+ messages and more than a month to review despite its smallness, and I don't want both of us to spend that much time this time.


Repository:
  rL LLVM

https://reviews.llvm.org/D37615





More information about the llvm-commits mailing list