In D155808#4528543, @MaskRay wrote:

Add NFC if this is NFC?

Addressing review comments.

isArmBigEndian -> isARMBigEndian

[Gentle Reminder] I have implemented the requested changes. Could you please have a look again. Thanks.

But I'm not sure there needs to be a 70-line C program given in full in the commit message :-)

i have tested this locally using opensource clang with GCC Linaro toolchain and by running it on qemu arm big endian system.

Addressing Review comment - Removing the variable IsBigEndian.

Addressing Review comments.

Thank you for the heads up. I had tested everything locally before landing the changes. Not sure what caused the premege check to fail. Yes as you said i will run the tests with asan enabled and see . Thanks.

Since there is no new comments after Peter approved this patch, i am going to submit this. If there is any more comments later, i can work on that as a separate patch.

typo locallly -> locally.

Addressing review comments.

Addressing review comments.

In D150870#4387598, @MaskRay wrote:

Instructions are big-endian in objects but little-endian in executables.
Data is big-endian.
ELF files are big-endian.

Instructions are big-endian in relocatable object files but little-endian in executables and shared objects. ("objects files" include executable files and shared object files as well)
Data is big-endian.
The data encoding of the ELF files is ELFDATA2MSB

In D150870#4387593, @MaskRay wrote:

Should there be an error when --be8 is used by non-EM_ARM or little-endian? You may add arm-be8.s

Addressed review comments.

In D150870#4355800, @peter.smith wrote:

One more thought I had where we might need changes, although they could be done in a different patch as they are more of a bug fix.

The Arm thunks come in short and long forms. If a thunk is within range of a branch instruction it will write this out, otherwise it will use a longer sequence involving an indirect branch. In most thunks this doesn't change the mapping symbols, but in some cases it does because there are either state changes or inline literal data. For example:

void ThumbV6MABSLongThunk::addSymbols(ThunkSection &isec) {
  addSymbol(saver().save("__Thumbv6MABSLongThunk_" + destination.getName()),
            STT_FUNC, 1, isec);
  addSymbol("$t", STT_NOTYPE, 0, isec);
  addSymbol("$d", STT_NOTYPE, 8, isec);
}

Note that if this is written as a short thunk then the second mapping symbol will be incorrect, and will end up overlapping with the next thunk. At the moment this may just mess the disassembly up, but with BE8 this could end up messing up the conversion to little endian.

I think that there may only be one or two thunks that have multiple mapping symbols and are used on CPUs that support BE8, however it would be good to fix all the cases. Which can be done by checking if a short thunk is being used and only adding the non 0 offset mapping symbols if short thunks aren't being used. For example, the AArch64 thunks add a check:

void AArch64ABSLongThunk::addSymbols(ThunkSection &isec) {
  addSymbol(saver().save("__AArch64AbsLongThunk_" + destination.getName()),
            STT_FUNC, 0, isec);
  addSymbol("$x", STT_NOTYPE, 0, isec);
  if (!getMayUseShortThunk())
    addSymbol("$d", STT_NOTYPE, 8, isec);
}

Address review comments.

Thanks for the reviews. I am away the whole next week. I will work on this review comments the week after next.

There is some similar code in the ARMErrataFix.cpp that also collects the mapping symbols. Now i have kept the code to collect Mapping symbols getArmMappingSymbolList in a generic place (Arch/ARM.cpp) so that it can be called by ARMErrataFix.cpp as well. If this decision is accepted, i will make an another patch to reflect the changes in ARMErrataFix.cpp.

Yeah, It does look like we do not need to add the template to the Arm target classes.
I can not see any places that we use the template parameter. Detemplating the classes
which makes it more readable.

Addressed Peter's review comments.

This patch:

• get rid of config->be32 for this patch
• Fixed the problem found in .ARM.exidx table compression. Now isDuplicateArmExidxSec works for both little and big endians. .ARM.exidx table is now identical between little and big-endian.

This patch now

This revision is now redundant as I have taken all these changes from here to the same patch (https://reviews.llvm.org/D140202) where we are making all the functional changes.

I am taking over this change from @Milosz-Plichta to move the change forward.

Addressed all the comments. I have added all the tests in this same
patch where we are making all the functional changes. I have literally
taken all the changes from https://reviews.llvm.org/D140203 to here.
Also added few more testcases.

We also saw failures on nofpclass-invalid.ll and VerifierTest.cpp in our internal build/test.

I am taking over this change from the original author @Milosz-Plichta with his permission to move the change forward.

Rebased this patch. It was needed a bit more changes to work with the below changes:
https://reviews.llvm.org/D141272
https://reviews.llvm.org/D139888

I am taking over this change from the original author @Milosz-Plichta with his permission to move the change forward.

Comments addressed.

Comments Addressed.

Adding assert statement in case "Fell off end of block" case comes

Addressed @efriedma suggestions.

Uploaded a new patch. I think this is more sensible way of fixing than the previous one.