[PATCH] Improve big-endian support for ARM and AArch64

Charlie Turner charlie.turner at arm.com
Wed Sep 24 05:22:28 PDT 2014


Hi Renato,

> Patch 1 can be split into three changes:
>  * refactoring the if/else -> switch
>  * adding AArch64 be
>  * ppc small change
>
> It also needs some tests, I'm not sure where, though.

I agree that I should have moved the change to visitELF_ARM_ABS32 to another patch with its own test.

The reason I added the small ppc change was because in order to complete the refactoring, I needed to convert

    else if (FileFormat == "ELF32-ppc") {
       ...
    }

Into an equivalent switch case. Since ELFObjectFile::getArch() was not returning Triple::ppc, there was no way for me to complete the refactoring without first removing functionality to add back in a later patch.

I'm not sure how you would like me to test the refactoring. The only approach I can think of is to add 11 test cases where llvm-dwarfdump is run on each of the 11 object files supported by RelocToApply::visit. The test would check that the message "error: failed to compute relocation" was not issued for any of these object files, which would indicate that the refactoring missed an if/else branch. Is that what you had in mind, or is a manual check by eye to see if I got each of the 11 cases converted enough? The 11 cases I'm referring to are the various file format strings like "ELF64-x86_64", "ELF32-i386", and so on. I counted 11 of them.

> Patch 3 looks good, too, though I'm worried if that output could be used for something else external.

Can you elaborate on your worry? I'm not sure what action you would like me to take.

Thanks for reviewing!

Charlie.








More information about the llvm-commits mailing list