[PATCH] D150870: [lld][Arm] Big Endian - Byte invariant support.

[Simi Pallipurath via Phabricator via llvm-commits]

simpal01 marked 26 inline comments as done.
simpal01 added inline comments.


================
Comment at: lld/ELF/Arch/ARM.cpp:999-1004
+    if (isThumbMapSymbol(msym))
+      new_state = HALFWORDCODE;
+    else if (isArmMapSymbol(msym))
+      new_state = WORDCODE;
+    else if (isDataMapSymbol(msym))
+      new_state = DATA;
----------------
amilendra wrote:
> This can be in its own function. Maybe a lambda function? That will let you rewrite the `case DATA:` block like
> ```
>         start = msym->value;
>         cur_state = function(msym);
> ```
isArmMapSymbol, isThumbMapSymbol and isDataMapSymbol are used by both the functions convertArmInstructionstoBE8 and getArmMappingSymbolList. 


================
Comment at: lld/ELF/Arch/ARM.cpp:1007-1022
+    case WORDCODE:
+      if (new_state == HALFWORDCODE || new_state == DATA) {
+        end = msym->value;
+        BytesexReverseInstructions(buf, start, end, 4);
+        cur_state = new_state;
+        start = msym->value;
+      }
----------------
amilendra wrote:
> These two blocks can probably be folded together.
> 
>     case WORDCODE:
>     case HALFWORDCODE:
>       if (new_state != cur_state) {
>         end = msym->value;
>         BytesexReverseInstructions(buf, start, end, cur_state);
>         cur_state = new_state;
>         start = msym->value;
>       }
>       break;
Refactored this part of the code as suggested by Amilendra and Peter.


================
Comment at: lld/ELF/OutputSections.cpp:499
+      // instructions, but not data.
+      if (config->emachine == EM_ARM && !config->isLE && config->be8)
+        if (flags & SHF_EXECINSTR)
----------------
peter.smith wrote:
> I've just thought that we probably do not want to endian reverse instructions when doing a relocatable link (the -r option which maps to config->relocatable; this produces a relocatable object file). We will want to double check what GNU ld does to make sure.
I have checked with gcc and found that gcc does endian reverse instructions in BE8 mode  when doing a relocatable link. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150870