[PATCH] D67926: Fix endianness handling in AVR MC

[Dylan McKay via Phabricator via llvm-commits]

dylanmckay accepted this revision.
dylanmckay added a comment.
This revision is now accepted and ready to land.

Nice patch, sorry for taking so long to getting around to this.

> @dylanmckay Why is the byte-swapping need here in the first place?

It was quite a long time ago when written, I can't recall this particular logic. I suspect it was a misunderstanding on my part of the format of  `uint64_t Val`.

I've left a couple comments, I'm happy to accept this patch now with the two minor nitpicks fixed.

I've tested this locally and it doesn't cause any regressions, I agree that the reinterpret cast in its prior form makes incorrect assumptions about the host byte ordering, good catch @serge-sans-paille!



================
Comment at: llvm/lib/Target/AVR/MCTargetDesc/AVRMCCodeEmitter.cpp:29
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/EndianStream.h"
 
----------------
Place `#include`s in alphabetical order [CodingStandards](https://llvm.org/docs/CodingStandards.html#include-style)


================
Comment at: llvm/lib/Target/AVR/MCTargetDesc/AVRMCCodeEmitter.cpp:275
   for (int64_t i = WordCount - 1; i >= 0; --i) {
-    uint16_t Word = Words[i];
-
-    OS << (uint8_t) ((Word & 0x00ff) >> 0);
-    OS << (uint8_t) ((Word & 0xff00) >> 8);
+    uint16_t Word = (Val >> i * 16) & 0xFFFF;
+    support::endian::write(OS, Word, support::endianness::little);
----------------
Nitpick: I recommend adding parentheses around `i * 16` so the reader doesn't have to be conscious about precedence rules to mentally parse this


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67926/new/

https://reviews.llvm.org/D67926