[AVR] Override ParseDirective

[xiangzhai]

Summary

Hi LLVM developers,

Motivation:

• avr-ld supports directive, relax.s testcase
• arm-linux-gnu-ld supports that too, tls-gdlerelax.s testcase

But:

$ llvm-mc -filetype=obj -triple=avr-unknown-linux -mcpu=atmega328p relax.s -o relax.o
relax.s:12:2: error: unknown directive
        .word   L2 - L1
        ^

So I simply override ParseDirective for llvm/lib/Target/AVR/AsmParser/AVRAsmParser.cpp, please review it, thanks a lot!

Regards,
Leslie Zhai

[xiangzhai]

Dear Dylan,

BTW: AVR GNU toolchain can *not* support ldi r16, -lo8(abc) but ldi r16, lo8(-(abc)) instead,

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

relocations.s: Assembler messages:
relocations.s:36: Error: garbage at end of line
relocations.s:39: Error: garbage at end of line
relocations.s:45: Error: garbage at end of line
relocations.s:51: Error: garbage at end of line
relocations.s:57: Error: garbage at end of line
relocations.s:69: Error: garbage at end of line
relocations.s:72: Error: garbage at end of line
relocations.s:75: Error: garbage at end of line
relocations.s:36: Error: can't resolve `0' {*UND* section} - `lo8' {*UND* section}
relocations.s:36: Error: expression too complex
relocations.s:39: Error: can't resolve `0' {*UND* section} - `hi8' {*UND* section}
relocations.s:39: Error: expression too complex
relocations.s:45: Error: can't resolve `0' {*UND* section} - `hh8' {*UND* section}
relocations.s:45: Error: expression too complex
relocations.s:51: Error: can't resolve `0' {*UND* section} - `hlo8' {*UND* section}
relocations.s:51: Error: expression too complex
relocations.s:57: Error: can't resolve `0' {*UND* section} - `hhi8' {*UND* section}
relocations.s:57: Error: expression too complex
relocations.s:69: Error: can't resolve `0' {*UND* section} - `pm_lo8' {*UND* section}
relocations.s:69: Error: expression too complex
relocations.s:72: Error: can't resolve `0' {*UND* section} - `pm_hi8' {*UND* section}
relocations.s:72: Error: expression too complex
relocations.s:75: Error: can't resolve `0' {*UND* section} - `pm_hh8' {*UND* section}
relocations.s:75: Error: expression too complex

So do I need to rewrite Check for sign for LLD to compatible GNU toolchain?

Regards,
Leslie Zhai

[xiangzhai]

Add R_AVR_DIFF8, R_AVR_DIFF16 and R_AVR_DIFF32 testcase:

$ avr-readelf -r avr-relax.o 

Relocation section '.rela.text' at offset 0xe8 contains 1 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000212 R_AVR_CALL        00000000   .text + 0

Relocation section '.rela.data' at offset 0xf4 contains 3 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  0000021e R_AVR_DIFF8       00000000   .text + 4
00000001  0000021f R_AVR_DIFF16      00000000   .text + 4
00000003  00000220 R_AVR_DIFF32      00000000   .text + 4

[xiangzhai]

Implement Parse R_AVR_LO8_LDI_NEG and other R_AVR_XXX_NEG Expression for GCC compatible.

Because AVR GNU toolchain only support such Expression:

ldi r16, lo8(-(abc))

But LLVM AVR prefers:

ldi r16, -lo8(abc)

So I just re-implement Check for sign, please review my patch, thanks a lot!

Regards,
Leslie Zhai

[xiangzhai]

Implement R_AVR_LO8_LDI_GS and R_AVR_HI8_LDI_GS:

$ llvm/build/bin/llvm-mc -filetype=obj -triple=avr-unknown-linux -mcpu=atmega328p relocations.s -o avr-llvm-relocations.o
$ avr-readelf -r avr-llvm-relocations.o

...
00000038  00000b18 R_AVR_LO8_LDI_GS  00000000   foo + 0
0000003a  00000b19 R_AVR_HI8_LDI_GS  00000000   foo + 0
...

[dylanmckay]

include/llvm/BinaryFormat/ELFRelocs/AVR.def

42

These integers correspond to numeric ELF relocation codes.

Looking at binutils-gdb, the integers for R_AVR_DIFF{8,16,32} are 30, 31, and 32 respectively.

This is concerning because 30 and 31 are already occupied by R_AVR_SYM_DIFF and R_AVR_16_LDST in master.

It looks like the AVR backend doesn't actually generate fixups for either of these, so this will have never caused any problems in the past.

If you rename SYM_DIFF to DIFF8, and then change R_AVR_16_LDST to 33, and then change your new constants to their correct values, it should be fine. I can fix up the relocations in the future.

[xiangzhai]

Change RELOC_NUMBER for AVR Relocation DIFF Types.

[xiangzhai]

Implement R_AVR_16_PM:

relocations.s testcase:

...
.short foo
.short gs(foo)
...

$ avr-readelf -r avr-reloc.o 

Relocation section '.rela.text' at offset 0xa8 contains 4 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
...
00000002  00000504 R_AVR_16          00000000   foo + 0
00000004  00000505 R_AVR_16_PM       00000000   foo + 0
...

[xiangzhai]

Does it need to Refactory ParseDirective for adding parameter OperandVector Operands to support Modifier?

[xiangzhai]

Waiting for D38304 about Refactoring ParseDirective.

[dylanmckay]

This patch is looking really nice by the way.

lib/Target/AVR/AsmParser/AVRAsmParser.cpp

441

Can we hoist the "gs" into a static constant named `GENERATE_ at the top of the file?

It looks like gs stands for "generate stubs"

https://lists.nongnu.org/archive/html/avr-gcc-list/2010-05/msg00028.html

610

Can we do something like this at the top of the file

cpp
const int SIZE_LONG = 8;
const int SIZE_WORD = 4;

By the way, I think these sizes are for a 32-bit machine. On AVR, long is 5 bytes and int (which I think corresponds to .word) is 2 bytes

Here is a document detailing the AVR integer types and their sizes

620

Can we be more specific and name Size -> SizeInBytes

640

Replace this with

cpp
FIXME: does not support .byte lo8(foo)

Could you also add a bit more information to the comment, because it's not clear why it isn't supported.

[dylanmckay]

lib/Target/AVR/MCTargetDesc/AVRMCExpr.cpp

124

I believe we should mask out all of the upper bits here.

It looks like none of the other labels in this method do it either, which looks like a bug to me. I have filed PR34755 to fix these existing ones up.

For these new expressions, can you please mask out the irrelevant bits after shifting?

169

Related to an earlier comment; I don't think GS directly maps to program memory

[xiangzhai]

Dear Dylan,

I am not available until October 10th https://mail.kde.org/pipermail/k3b/2017-September/001783.html Happy Chinese National Day!

Regards,
Leslie Zhai

[xiangzhai]

Dear Dylan,

I updated my patch as your suggestion, please review it, thanks a lot!

PS: still waiting for D38304 about Refactoring ParseDirective.

Regards,
Leslie Zhai

[xiangzhai]

Supported .byte lo8(foo) now :)

$ llvm/build/bin/llvm-mc -filetype=obj -triple=avr-unknown-linux -mcpu=atmega328p relocations.s -o avr-llvm-relocations.o
$ avr-readelf -r avr-llvm-relocations.o 

Relocation section '.rela.text' at offset 0x170 contains 60 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
...
00000071  00000b1b R_AVR_8_LO8       00000000   foo + 0
...

[xiangzhai]

Refactory EmitValue for AVRMCELFStreamer.

[xiangzhai]

Support R_AVR_DIFF8, R_AVR_DIFF16 and R_AVR_DIFF32 now :)

$ llvm-mc -filetype=obj -triple=avr-unknown-linux -mcpu=atmega328p relax.s -o avr-relax-llvm.o

$ avr-readelf -r avr-relax-llvm.o 

Relocation section '.rela.text' at offset 0xd8 contains 1 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  00000512 R_AVR_CALL        00000000   .text + 0

Relocation section '.rela.data' at offset 0xe4 contains 3 entries:
 Offset     Info    Type            Sym.Value  Sym. Name + Addend
00000000  0000051e R_AVR_DIFF8       00000000   .text + 0
00000002  0000051f R_AVR_DIFF16      00000000   .text + 0
00000006  00000520 R_AVR_DIFF32      00000000   .text + 0

[arsenm]

lib/Target/AVR/AsmParser/AVRAsmParser.cpp

35

Generally I don't think we use these kinds of hardcoded macro options

[xiangzhai]

Remove hardcoded macro REFACTORY_PARSEDIRECTIVE.

[xiangzhai]

Rebase for rL315531

[xiangzhai]

Rebase for rL316076 and nicely Ping Dylan and Matt :)

[dylanmckay]

Hey Leslie,

I don't have much time to review this in the next 3 weeks, and I also think that D38304 may take even longer to land (which this patch depends on) because it looks to have gone somewhat stale.

[xiangzhai]

Dear Dylan,

Never mind, you are busy for university class :) D38304 is optional dependent, only depends on it when built with -DREFACTORY_PARSEDIRECTIVE.

Regards,
Leslie Zhai

[xiangzhai]

Nicely ping Dylan :)

[xiangzhai]

Ping

[dylanmckay]

Have finished an initial review - change looks pretty good, it should be good to go once my comments are fixed

Sorry for taking so long to review @xiangzhai!

lib/Target/AVR/AsmParser/AVRAsmParser.cpp

14

These headers should be in alphabetical order

58

Before this can be committed, we will need to remove these REFACTORY_PARSEDIRECTIVE guards.

Once this patch has been comitted, D38304 should then be rebased

lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp

270

gs stands for generate stubs - I don't think we want to apply the program memory adjustment for these. As far as I understand it, AVR::fixuo_lo8_ldi_gs is a load from RAM.

277–279

Same here regarding gs

lib/Target/AVR/MCTargetDesc/AVRELFObjectWriter.cpp

18

Place this in alphabetical order

lib/Target/AVR/MCTargetDesc/AVRMCELFStreamer.h

1 ↗

(On Diff #120182)

s/MCElfStreamer/MCELFStreamer

13 ↗

(On Diff #120182)

Header files should be in alphabetical order

39 ↗

(On Diff #120182)

Indent this constructor list like the one above

42 ↗

(On Diff #120182)

Pop an override qualifier on this function

lib/Target/AVR/MCTargetDesc/AVRMCExpr.cpp

133

We want to use a different mask for each expression type

For example

cpp
switch (Kind) {
case AVRMCExpr::VK_AVR_LO8_GS:
    Value &= 0xff;
    Value >>= 1;
    break;
  case AVRMCExpr::VK_AVR_HI8_GS:
    Value &= 0xff00;
    Value >>= 9;
    break;
  case AVRMCExpr::VK_AVR_GS:
    Value >>= 1;
    break;

Different variants require masking out different bitsets - lo8 should isolate only the lower 8-bits, hi8 should to the upper.

lib/Target/AVR/MCTargetDesc/AVRMCTargetDesc.cpp

18 ↗

(On Diff #120182)

Alphabetical order

26 ↗

(On Diff #120182)

Alphabetical order

lib/Target/AVR/MCTargetDesc/CMakeLists.txt

10 ↗

(On Diff #120182)

Add this in alphabetical order

test/MC/AVR/relocations.s

92

Nice test

[xiangzhai]

Dear Dylan,

Thanks for your review! I updated my patch as your suggestion, please review it again.

Regards,
Leslie Zhai

[xiangzhai]

lib/Target/AVR/MCTargetDesc/AVRMCELFStreamer.h

42 ↗

(On Diff #120182)

I don't want to override EmitValue, sorry for my FunctionName misleading!

test/MC/AVR/relocations.s

92

Thanks :)

[dylanmckay]

@xiangzhai once the new comments are addressed, this should be good to commit

lib/Target/AVR/AsmParser/AVRAsmParser.cpp

626

Nitpick: Can we move this up to line 624 where we get the parser?

647

Same here regarding moving the variable up to 624

lib/Target/AVR/MCTargetDesc/AVRFixupKinds.h

119–120

We can probably remove this comment now that the fixups are always lowered to relocations

lib/Target/AVR/MCTargetDesc/AVRMCExpr.cpp

109

If I remember correctly, HH8 means bit 16-23, and so this mask will mask out the bits we actually want

Change the mask to 0xff0000

112

Same here, but mask should be 0xff000000

121

This needs the updated mask from the other HH8 fixup

[xiangzhai]

Dear Dylan,

Please review it again, thanks a lot!

Regards,
Leslie Zhai

[dylanmckay]

Patch is looking great, thanks Leslie!

include/llvm/BinaryFormat/ELFRelocs/AVR.def

 ELF_RELOC(R_AVR_MS8_LDI,              22)
 ELF_RELOC(R_AVR_MS8_LDI_NEG,          23)
 ELF_RELOC(R_AVR_LO8_LDI_GS,           24)
 ELF_RELOC(R_AVR_HI8_LDI_GS,           25)
 ELF_RELOC(R_AVR_8,                    26)
 ELF_RELOC(R_AVR_8_LO8,                27)
 ELF_RELOC(R_AVR_8_HI8,                28)
 ELF_RELOC(R_AVR_8_HLO8,               29)
-ELF_RELOC(R_AVR_SYM_DIFF,             30)
-ELF_RELOC(R_AVR_16_LDST,              31)
+ELF_RELOC(R_AVR_DIFF8,                30)
+ELF_RELOC(R_AVR_DIFF16,               31)
+ELF_RELOC(R_AVR_DIFF32,               32)
 ELF_RELOC(R_AVR_LDS_STS_16,           33)
 ELF_RELOC(R_AVR_PORT6,                34)
 ELF_RELOC(R_AVR_PORT5,                35)

lib/Target/AVR/AsmParser/AVRAsmParser.cpp

 //===---- AVRAsmParser.cpp - Parse AVR assembly to MCInst instructions ----===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "AVR.h"
 #include "AVRRegisterInfo.h"
 #include "MCTargetDesc/AVRMCExpr.h"
 #include "MCTargetDesc/AVRMCTargetDesc.h"
 
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstBuilder.h"
 #include "llvm/MC/MCParser/MCAsmLexer.h"
 #include "llvm/MC/MCParser/MCParsedAsmOperand.h"
 #include "llvm/MC/MCParser/MCTargetAsmParser.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/MCValue.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/TargetRegistry.h"
 
 #include <sstream>
 
 #define DEBUG_TYPE "avr-asm-parser"
 
 namespace llvm {
 
 /// Parses AVR assembly from a stream.
 class AVRAsmParser : public MCTargetAsmParser {
   const MCSubtargetInfo &STI;
   MCAsmParser &Parser;
   const MCRegisterInfo *MRI;
 
 #define GET_ASSEMBLER_HEADER
 #include "AVRGenAsmMatcher.inc"
 
   bool MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
                                OperandVector &Operands, MCStreamer &Out,
                                uint64_t &ErrorInfo,
                                bool MatchingInlineAsm) override;
 
   bool ParseRegister(unsigned &RegNo, SMLoc &StartLoc, SMLoc &EndLoc) override;
 
   bool ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
                         SMLoc NameLoc, OperandVector &Operands) override;
 
   bool ParseDirective(AsmToken directiveID) override;
 
   OperandMatchResultTy parseMemriOperand(OperandVector &Operands);
 
   bool parseOperand(OperandVector &Operands);
   int parseRegisterName(unsigned (*matchFn)(StringRef));
   int parseRegisterName();
   int parseRegister();
   bool tryParseRegisterOperand(OperandVector &Operands);
   bool tryParseExpression(OperandVector &Operands);
   bool tryParseRelocExpression(OperandVector &Operands);
   void eatComma();
 
   unsigned validateTargetOperandClass(MCParsedAsmOperand &Op,
                                       unsigned Kind) override;
 
   unsigned toDREG(unsigned Reg, unsigned From = AVR::sub_lo) {
     MCRegisterClass const *Class = &AVRMCRegisterClasses[AVR::DREGSRegClassID];
     return MRI->getMatchingSuperReg(Reg, From, Class);
   }
 
   bool emit(MCInst &Instruction, SMLoc const &Loc, MCStreamer &Out) const;
   bool invalidOperand(SMLoc const &Loc, OperandVector const &Operands,
                       uint64_t const &ErrorInfo);
   bool missingFeature(SMLoc const &Loc, uint64_t const &ErrorInfo);
 
+  bool parseLiteralValues(unsigned Size, SMLoc L);
+
 public:
   AVRAsmParser(const MCSubtargetInfo &STI, MCAsmParser &Parser,
                const MCInstrInfo &MII, const MCTargetOptions &Options)
       : MCTargetAsmParser(Options, STI), STI(STI), Parser(Parser) {
     MCAsmParserExtension::Initialize(Parser);
     MRI = getContext().getRegisterInfo();
 
     setAvailableFeatures(ComputeAvailableFeatures(STI.getFeatureBits()));
bool AVRAsmParser::tryParseRelocExpression(OperandVector &Operands) {
 
   SMLoc S = Parser.getTok().getLoc();
 
   // Check for sign
   AsmToken tokens[2];
   size_t ReadCount = Parser.getLexer().peekTokens(tokens);
 
   if (ReadCount == 2) {
-    if (tokens[0].getKind() == AsmToken::Identifier &&
-        tokens[1].getKind() == AsmToken::LParen) {
+    if ((tokens[0].getKind() == AsmToken::Identifier &&
+         tokens[1].getKind() == AsmToken::LParen) ||
+        (tokens[0].getKind() == AsmToken::LParen &&
+         tokens[1].getKind() == AsmToken::Minus)) {
 
       AsmToken::TokenKind CurTok = Parser.getLexer().getKind();
-      if (CurTok == AsmToken::Minus) {
+      if (CurTok == AsmToken::Minus ||
+          tokens[1].getKind() == AsmToken::Minus) {
         isNegated = true;
       } else {
         assert(CurTok == AsmToken::Plus);
         isNegated = false;
       }
 
       // Eat the sign
+      if (CurTok == AsmToken::Minus || CurTok == AsmToken::Plus)
         Parser.Lex();
     }
   }
 
   // Check if we have a target specific modifier (lo8, hi8, &c)
   if (Parser.getTok().getKind() != AsmToken::Identifier ||
       Parser.getLexer().peekTok().getKind() != AsmToken::LParen) {
     // Not a reloc expr
     return true;
   }
   StringRef ModifierName = Parser.getTok().getString();
   ModifierKind = AVRMCExpr::getKindByName(ModifierName.str().c_str());
 
   if (ModifierKind != AVRMCExpr::VK_AVR_None) {
     Parser.Lex();
     Parser.Lex(); // Eat modifier name and parenthesis
+    if (Parser.getTok().getString() == "gs" &&
+        Parser.getTok().getKind() == AsmToken::Identifier) {
+      std::string GSModName = ModifierName.str() += "_gs";
+      ModifierKind = AVRMCExpr::getKindByName(GSModName.c_str());
+      if (ModifierKind != AVRMCExpr::VK_AVR_None)
+        Parser.Lex(); // Eat gs modifier name
+    }
   } else {
     return Error(Parser.getTok().getLoc(), "unknown modifier");
   }
 
+  if (tokens[1].getKind() == AsmToken::Minus) {
+    Parser.Lex();
+    assert(Parser.getTok().getKind() == AsmToken::LParen);
+    Parser.Lex(); // Eat the sign and parenthesis
+  }
+
   MCExpr const *InnerExpression;
   if (getParser().parseExpression(InnerExpression))
     return true;
 
+  if (tokens[1].getKind() == AsmToken::Minus) {
+    assert(Parser.getTok().getKind() == AsmToken::RParen);
+    Parser.Lex(); // Eat closing parenthesis
+  }
+
   // If we have a modifier wrap the inner expression
   assert(Parser.getTok().getKind() == AsmToken::RParen);
   Parser.Lex(); // Eat closing parenthesis
 
   MCExpr const *Expression = AVRMCExpr::create(ModifierKind, InnerExpression,
                                                isNegated, getContext());
 
   SMLoc E = SMLoc::getFromPointer(Parser.getTok().getLoc().getPointer() - 1);
if (parseOperand(Operands)) {
       Parser.eatToEndOfStatement();
       return Error(Loc, "unexpected token in argument list");
     }
   }
   Parser.Lex(); // Consume the EndOfStatement
   return false;
 }
 
-bool AVRAsmParser::ParseDirective(llvm::AsmToken DirectiveID) { return true; }
+bool AVRAsmParser::ParseDirective(llvm::AsmToken DirectiveID) {
+  StringRef IDVal = DirectiveID.getIdentifier();
+  if (IDVal == ".long")
+    parseLiteralValues(8, DirectiveID.getLoc());
+  else if (IDVal == ".word")
+    parseLiteralValues(4, DirectiveID.getLoc());
+  else if (IDVal == ".short")
+    parseLiteralValues(2, DirectiveID.getLoc());
+  else if (IDVal == ".byte")
+    parseLiteralValues(1, DirectiveID.getLoc());
+  return true;
+}
+
+bool AVRAsmParser::parseLiteralValues(unsigned Size, SMLoc L) {
+  MCAsmParser &Parser = getParser();
+  if (Parser.getTok().getKind() == AsmToken::Identifier &&
+      Parser.getLexer().peekTok().getKind() == AsmToken::LParen) {
+    StringRef ModifierName = Parser.getTok().getString();
+    AVRMCExpr::VariantKind ModifierKind =
+        AVRMCExpr::getKindByName(ModifierName.str().c_str());
+    if (ModifierKind != AVRMCExpr::VK_AVR_None) {
+      Parser.Lex();
+      Parser.Lex(); // Eat the modifier and parenthesis
+    } else {
+      return Error(Parser.getTok().getLoc(), "unknown modifier");
+    }
+    const MCExpr *InnerExpression;
+    return Parser.parseExpression(InnerExpression);
+    // FIXME: not supports .byte lo8(foo)
+    // Does it need to re-architecture ParseDirective add Operands?
+  }
+  auto parseOne = [&]() -> bool {
+    const MCExpr *Value;
+    if (Parser.parseExpression(Value))
+      return true;
+    Parser.getStreamer().EmitValue(Value, Size, L);
+    return false;
+  };
+  return (parseMany(parseOne));
+}
 
 extern "C" void LLVMInitializeAVRAsmParser() {
   RegisterMCAsmParser<AVRAsmParser> X(getTheAVRTarget());
 }
 
 #define GET_REGISTER_MATCHER
 #define GET_MATCHER_IMPLEMENTATION
 #include "AVRGenAsmMatcher.inc"
 

lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp

void AVRAsmBackend::adjustFixupValue(const MCFixup &Fixup,
   case AVR::fixup_call:
     adjust::fixup_call(Size, Fixup, Value, Ctx);
     break;
   case AVR::fixup_ldi:
     adjust::ldi::fixup(Size, Fixup, Value, Ctx);
     break;
   case AVR::fixup_lo8_ldi:
   case AVR::fixup_lo8_ldi_pm:
-    if (Kind == AVR::fixup_lo8_ldi_pm) adjust::pm(Value);
+  case AVR::fixup_lo8_ldi_gs:
+    if (Kind == AVR::fixup_lo8_ldi_pm || Kind == AVR::fixup_lo8_ldi_gs)
+      adjust::pm(Value);
 
     adjust::ldi::lo8(Size, Fixup, Value, Ctx);
     break;
   case AVR::fixup_hi8_ldi:
   case AVR::fixup_hi8_ldi_pm:
-    if (Kind == AVR::fixup_hi8_ldi_pm) adjust::pm(Value);
+  case AVR::fixup_hi8_ldi_gs:
+    if (Kind == AVR::fixup_hi8_ldi_pm || Kind == AVR::fixup_hi8_ldi_gs)
+      adjust::pm(Value);
 
     adjust::ldi::hi8(Size, Fixup, Value, Ctx);
     break;
   case AVR::fixup_hh8_ldi:
   case AVR::fixup_hh8_ldi_pm:
     if (Kind == AVR::fixup_hh8_ldi_pm) adjust::pm(Value);
 
     adjust::ldi::hh8(Size, Fixup, Value, Ctx);
const static MCFixupKindInfo Infos[AVR::NumTargetFixupKinds] = {
       {"fixup_lo8_ldi_gs", 0, 8, 0},
       {"fixup_hi8_ldi_gs", 0, 8, 0},
 
       {"fixup_8", 0, 8, 0},
       {"fixup_8_lo8", 0, 8, 0},
       {"fixup_8_hi8", 0, 8, 0},
       {"fixup_8_hlo8", 0, 8, 0},
 
-      {"fixup_sym_diff", 0, 32, 0},
-      {"fixup_16_ldst", 0, 16, 0},
+      {"fixup_diff8", 0, 8, 0},
+      {"fixup_diff16", 0, 16, 0},
+      {"fixup_diff32", 0, 32, 0},
 
       {"fixup_lds_sts_16", 0, 16, 0},
 
       {"fixup_port6", 0, 16, 0}, // non-contiguous
       {"fixup_port5", 3, 5, 0},
   };
 
   if (Kind < FirstTargetFixupKind)

lib/Target/AVR/MCTargetDesc/AVRELFObjectWriter.cpp

 #include "MCTargetDesc/AVRFixupKinds.h"
 #include "MCTargetDesc/AVRMCTargetDesc.h"
 
 #include "llvm/MC/MCAssembler.h"
 #include "llvm/MC/MCELFObjectWriter.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCSection.h"
 #include "llvm/MC/MCValue.h"
 #include "llvm/Support/ErrorHandling.h"
 
 namespace llvm {
 
 /// Writes AVR machine code into an ELF32 object file.
 class AVRELFObjectWriter : public MCELFObjectTargetWriter {
 public:
   AVRELFObjectWriter(uint8_t OSABI);
 
   virtual ~AVRELFObjectWriter() {}
 
   unsigned getRelocType(MCContext &Ctx,
                         const MCValue &Target,
                         const MCFixup &Fixup,
                         bool IsPCRel) const override;
 };
 
 AVRELFObjectWriter::AVRELFObjectWriter(uint8_t OSABI)
     : MCELFObjectTargetWriter(false, OSABI, ELF::EM_AVR, true, false) {}
 
 unsigned AVRELFObjectWriter::getRelocType(MCContext &Ctx,
                                           const MCValue &Target,
                                           const MCFixup &Fixup,
                                           bool IsPCRel) const {
+  MCSymbolRefExpr::VariantKind Modifier = Target.getAccessVariant();
   switch ((unsigned) Fixup.getKind()) {
   case FK_Data_1:
+    switch (Modifier) {
+    case MCSymbolRefExpr::VK_None:
+      return ELF::R_AVR_8;
+    }
+  case AVR::fixup_8:
+    return ELF::R_AVR_8;
   case FK_Data_4:
     llvm_unreachable("unsupported relocation type");
   case FK_Data_2:
     return ELF::R_AVR_16_PM;
   case AVR::fixup_32:
     return ELF::R_AVR_32;
   case AVR::fixup_7_pcrel:
     return ELF::R_AVR_7_PCREL;
unsigned AVRELFObjectWriter::getRelocType(MCContext &Ctx,
   case AVR::fixup_ms8_ldi:
     return ELF::R_AVR_MS8_LDI;
   case AVR::fixup_ms8_ldi_neg:
     return ELF::R_AVR_MS8_LDI_NEG;
   case AVR::fixup_lo8_ldi_gs:
     return ELF::R_AVR_LO8_LDI_GS;
   case AVR::fixup_hi8_ldi_gs:
     return ELF::R_AVR_HI8_LDI_GS;
-  case AVR::fixup_8:
-    return ELF::R_AVR_8;
   case AVR::fixup_8_lo8:
     return ELF::R_AVR_8_LO8;
   case AVR::fixup_8_hi8:
     return ELF::R_AVR_8_HI8;
   case AVR::fixup_8_hlo8:
     return ELF::R_AVR_8_HLO8;
-  case AVR::fixup_sym_diff:
-    return ELF::R_AVR_SYM_DIFF;
-  case AVR::fixup_16_ldst:
-    return ELF::R_AVR_16_LDST;
+  case AVR::fixup_diff8:
+    return ELF::R_AVR_DIFF8;
+  case AVR::fixup_diff16:
+    return ELF::R_AVR_DIFF16;
+  case AVR::fixup_diff32:
+    return ELF::R_AVR_DIFF32;
   case AVR::fixup_lds_sts_16:
     return ELF::R_AVR_LDS_STS_16;
   case AVR::fixup_port6:
     return ELF::R_AVR_PORT6;
   case AVR::fixup_port5:
     return ELF::R_AVR_PORT5;
   default:
     llvm_unreachable("invalid fixup kind!");

lib/Target/AVR/MCTargetDesc/AVRFixupKinds.h

enum Fixups {
   fixup_hi8_ldi_gs,
 
   fixup_8,
   fixup_8_lo8,
   fixup_8_hi8,
   fixup_8_hlo8,
 
   /// Fixup to calculate the difference between two symbols.
   /// Is the only stateful fixup. We do not support it yet.
-  fixup_sym_diff,
-  fixup_16_ldst,
+  fixup_diff8,
+  fixup_diff16,
+  fixup_diff32,
 
   fixup_lds_sts_16,
 
   /// A 6-bit port address.
   fixup_port6,
   /// A 5-bit port address.
   fixup_port5,
 

lib/Target/AVR/MCTargetDesc/AVRMCExpr.h

enum VariantKind {
 
     VK_AVR_HI8,  ///< Corresponds to `hi8()`.
     VK_AVR_LO8,  ///< Corresponds to `lo8()`.
     VK_AVR_HH8,  ///< Corresponds to `hlo8() and hh8()`.
     VK_AVR_HHI8, ///< Corresponds to `hhi8()`.
 
     VK_AVR_PM_LO8, ///< Corresponds to `pm_lo8()`.
     VK_AVR_PM_HI8, ///< Corresponds to `pm_hi8()`.
-    VK_AVR_PM_HH8  ///< Corresponds to `pm_hh8()`.
+    VK_AVR_PM_HH8, ///< Corresponds to `pm_hh8()`.
+
+    VK_AVR_LO8_GS, ///< Corresponds to `lo8(gs())`.
+    VK_AVR_HI8_GS, ///< Corresponds to `hi8(gs())`.
   };
 
 public:
   /// Creates an AVR machine code expression.
   static const AVRMCExpr *create(VariantKind Kind, const MCExpr *Expr,
                                  bool isNegated, MCContext &Ctx);
 
   /// Gets the type of the expression.

lib/Target/AVR/MCTargetDesc/AVRMCExpr.cpp

const struct ModifierEntry {
   AVRMCExpr::VariantKind VariantKind;
 } ModifierNames[] = {
     {"lo8", AVRMCExpr::VK_AVR_LO8},       {"hi8", AVRMCExpr::VK_AVR_HI8},
     {"hh8", AVRMCExpr::VK_AVR_HH8}, // synonym with hlo8
     {"hlo8", AVRMCExpr::VK_AVR_HH8},      {"hhi8", AVRMCExpr::VK_AVR_HHI8},
 
     {"pm_lo8", AVRMCExpr::VK_AVR_PM_LO8}, {"pm_hi8", AVRMCExpr::VK_AVR_PM_HI8},
     {"pm_hh8", AVRMCExpr::VK_AVR_PM_HH8},
+
+    {"lo8_gs", AVRMCExpr::VK_AVR_LO8_GS}, {"hi8_gs", AVRMCExpr::VK_AVR_HI8_GS},
 };
 
 } // end of anonymous namespace
 
 const AVRMCExpr *AVRMCExpr::create(VariantKind Kind, const MCExpr *Expr,
                                    bool Negated, MCContext &Ctx) {
   return new (Ctx) AVRMCExpr(Kind, Expr, Negated);
 }
int64_t AVRMCExpr::evaluateAsInt64(int64_t Value) const {
 
   switch (Kind) {
   case AVRMCExpr::VK_AVR_LO8:
     break;
   case AVRMCExpr::VK_AVR_HI8:
     Value >>= 8;
     break;
   case AVRMCExpr::VK_AVR_HH8:
     Value >>= 16;
     break;
   case AVRMCExpr::VK_AVR_HHI8:
     Value >>= 24;
     break;
   case AVRMCExpr::VK_AVR_PM_LO8:
     Value >>= 1;
     break;
   case AVRMCExpr::VK_AVR_PM_HI8:
     Value >>= 9;
     break;
   case AVRMCExpr::VK_AVR_PM_HH8:
     Value >>= 17;
     break;
+  case AVRMCExpr::VK_AVR_LO8_GS:
+    Value >>= 1;
+    break;
+  case AVRMCExpr::VK_AVR_HI8_GS:
+    Value >>= 9;
+    break;
 
   case AVRMCExpr::VK_AVR_None:
     llvm_unreachable("Uninitialized expression.");
   }
   return static_cast<uint64_t>(Value) & 0xff;
 }
 
 AVR::Fixups AVRMCExpr::getFixupKind() const {
   AVR::Fixups Kind = AVR::Fixups::LastTargetFixupKind;
 
   switch (getKind()) {
   case VK_AVR_LO8:
     Kind = isNegated() ? AVR::fixup_lo8_ldi_neg : AVR::fixup_lo8_ldi;
case VK_AVR_PM_LO8:
     Kind = isNegated() ? AVR::fixup_lo8_ldi_pm_neg : AVR::fixup_lo8_ldi_pm;
     break;
   case VK_AVR_PM_HI8:
     Kind = isNegated() ? AVR::fixup_hi8_ldi_pm_neg : AVR::fixup_hi8_ldi_pm;
     break;
   case VK_AVR_PM_HH8:
     Kind = isNegated() ? AVR::fixup_hh8_ldi_pm_neg : AVR::fixup_hh8_ldi_pm;
     break;
+  case VK_AVR_LO8_GS:
+    Kind = AVR::fixup_lo8_ldi_gs;
+    break;
+  case VK_AVR_HI8_GS:
+    Kind = AVR::fixup_hi8_ldi_gs;
+    break;
 
   case VK_AVR_None:
     llvm_unreachable("Uninitialized expression");
   }
 
   return Kind;
 }
 
 void AVRMCExpr::visitUsedExpr(MCStreamer &Streamer) const {
   Streamer.visitUsedExpr(*getSubExpr());

test/MC/AVR/relocations.s

 ; RUN: llvm-mc -filetype=obj -triple=avr %s -mattr=avr6 | llvm-objdump -r - | FileCheck %s
 
 ; CHECK: RELOCATION RECORDS FOR
 
+; CHECK: RELOCATION RECORDS BAR
+bar:
+  jmp bar
+
 ; CHECK-NEXT: R_AVR_LDI SYMBOL+3
 ldi r21, SYMBOL+3
 
 ; CHECK-NEXT: R_AVR_6_ADIW FOO
 adiw r24, FOO
 
 ; CHECK-NEXT: R_AVR_16 BAR+2
 lds r8, BAR+2

 ; CHECK-NEXT: R_AVR_LO8_LDI bar+3
 ldi r24, lo8(bar+3)
 
 ; CHECK-NEXT: R_AVR_HI8_LDI abc
 ldi r30, hi8(abc)
 
 ; CHECK-NEXT: R_AVR_LO8_LDI_NEG abc
 ldi r16, -lo8(abc)
+ldi r16, lo8(-(abc))
 
 ; CHECK-NEXT: R_AVR_HI8_LDI_NEG abc
 ldi r16, -hi8(abc)
+ldi r16, hi8(-(abc))
 
 ; CHECK-NEXT: R_AVR_HH8_LDI foo
 ldi r16, hh8(foo)
 
 ; CHECK-NEXT: R_AVR_HH8_LDI_NEG foo
 ldi r16, -hh8(foo)
+ldi r16, hh8(-(foo))
 
 ; CHECK-NEXT: R_AVR_HH8_LDI foo
 ldi r24, hlo8(foo)
 
 ; CHECK-NEXT: R_AVR_HH8_LDI_NEG foo
 ldi r24, -hlo8(foo)
+ldi r24, hlo8(-(foo))
 
 ; CHECK-NEXT: R_AVR_MS8_LDI bar
 ldi r24, hhi8(bar)
 
 ; CHECK-NEXT: R_AVR_MS8_LDI_NEG bar
 ldi r24, -hhi8(bar)
+ldi r24, hhi8(-(bar))
 
 ; CHECK-NEXT: R_AVR_LO8_LDI_PM foo
 ldi r17, pm_lo8(foo)
 
 ; CHECK-NEXT: R_AVR_HI8_LDI_PM bar
 ldi r22, pm_hi8(bar)
 
 ; CHECK-NEXT: R_AVR_HH8_LDI_PM baz
 ldi r25, pm_hh8(baz)
 
 ; CHECK-NEXT: R_AVR_LO8_LDI_PM_NEG
 ldi r25, -pm_lo8(foo)
+ldi r25, pm_lo8(-(foo))
 
 ; CHECK-NEXT: R_AVR_HI8_LDI_PM_NEG
 ldi r25, -pm_hi8(foo)
+ldi r25, pm_hi8(-(foo))
 
 ; CHECK-NEXT: R_AVR_HH8_LDI_PM_NEG
 ldi r25, -pm_hh8(foo)
+ldi r25, pm_hh8(-(foo))
+
+; CHECK-NEXT: R_AVR_LO8_LDI_GS
+ldi r17, lo8(gs(foo))
+
+; CHECK-NEXT: R_AVR_HI8_LDI_GS
+ldi r18, hi8(gs(foo))
+
+; CHECK-NEXT: R_AVR_16_PM
+.short foo
+
+; CHECK-NEXT: R_AVR_8
+.byte foo
+
+; CHECK-NEXT: R_AVR_8_LO8
+.byte lo8(foo)
+
+; CHECK-NEXT: R_AVR_8_HI8
+.byte hi8(foo)
+
+; CHECK-NEXT: R_AVR_8_HLO8
+.byte hlo8(foo)
+
+; CHECK-NEXT: R_AVR_DIFF8
+.byte foo - bar
+
+; CHECK-NEXT: R_AVR_DIFF16
+.word foo - bar
+
+; CHECK-NEXT: R_AVR_DIFF32
+.long foo - bar