[PATCH] D38029: [AVR] Override ParseDirective

[Dylan McKay via Phabricator via llvm-commits]

dylanmckay requested changes to this revision.
dylanmckay added a comment.
This revision now requires changes to proceed.

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



================
Comment at: lib/Target/AVR/AsmParser/AVRAsmParser.cpp:630
+      Tokens[1].getKind() == AsmToken::Identifier) {
+    AVRMCELFStreamer &AVRStreamer =
+        static_cast<AVRMCELFStreamer &>(Parser.getStreamer());
----------------
Nitpick: Can we move this up to line 624 where we get the parser?


================
Comment at: lib/Target/AVR/AsmParser/AVRAsmParser.cpp:651
+        getContext().getOrCreateSymbol(Parser.getTok().getString());
+    AVRMCELFStreamer &AVRStreamer =
+        static_cast<AVRMCELFStreamer &>(Parser.getStreamer());
----------------
Same here regarding moving the variable up to 624


================
Comment at: lib/Target/AVR/MCTargetDesc/AVRFixupKinds.h:119
   /// Fixup to calculate the difference between two symbols.
   /// Is the only stateful fixup. We do not support it yet.
+  fixup_diff8,
----------------
We can probably remove this comment now that the fixups are always lowered to relocations


================
Comment at: lib/Target/AVR/MCTargetDesc/AVRMCExpr.cpp:111
   case AVRMCExpr::VK_AVR_HH8:
+    Value &= 0xff00;
     Value >>= 16;
----------------
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`


================
Comment at: lib/Target/AVR/MCTargetDesc/AVRMCExpr.cpp:115
   case AVRMCExpr::VK_AVR_HHI8:
+    Value &= 0xff00;
     Value >>= 24;
----------------
Same here, but mask should be `0xff000000`


================
Comment at: lib/Target/AVR/MCTargetDesc/AVRMCExpr.cpp:127
   case AVRMCExpr::VK_AVR_PM_HH8:
+    Value &= 0xff00;
     Value >>= 17;
----------------
This needs the updated mask from the other `HH8` fixup


Repository:
  rL LLVM

https://reviews.llvm.org/D38029