This is an archive of the discontinued LLVM Phabricator instance.

[Clang][Driver] Pass through the --be8 endian flag to linker in BareMetal driver For Arm.
ClosedPublic

Authored by simpal01 on Jul 9 2023, 5:33 AM.

Details

Summary

When linking a big-endian image for Arm, clang has
to select between BE8 and BE32 formats. The default
is dependent on the selected target architecture.
For ARMv6 and later architectures the default is
BE8, for older architectures the default is BE32.
For BE8 and BE32, compiler outputs a big endian ELF
relocatable object file with the instructions and
data both big endian. The difference is that at
link time, for BE8 a linker must endian reverse
the instructions to little endian. For BE8, the
clang has to pass --be8 to the linker for Arm.

At the moment clang is not passing the --be8 flag
to linker for the baremetal target architectures
above ArmV6 for Arm. This patch passes through --be8
and -BE or EL to the linker, taking into account the
target and the -mbig-endian and -mlittle-endian flag.
Also there are few more changes in the baremetal
driver so that the code can cope with AArch64 being
big-endian as well.

Diff Detail

Event Timeline

simpal01 created this revision.Jul 9 2023, 5:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 9 2023, 5:33 AM
simpal01 requested review of this revision.Jul 9 2023, 5:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 9 2023, 5:33 AM
MaskRay added inline comments.Jul 9 2023, 11:35 AM
clang/lib/Driver/ToolChains/Arch/ARM.cpp
48

We can check the options first and do an early return, then check getArch. We can then avoid [[fallthrough]].

arm::setArchNameInTriple has very similar code. We should factor it out.

clang/lib/Driver/ToolChains/Arch/ARM.h
17

This should not be commented out. IWYU we need this header.

clang/lib/Driver/ToolChains/BareMetal.cpp
433

no blank line

clang/test/Driver/baremetal.cpp
121

I think we can use a more dense style by packing more options on one line.

I often find the large number of lines impacts readability.

126

Delete -no-canonical-prefixes.

https://maskray.me/blog/2021-03-28-compiler-driver-and-cross-compilation#misc

-no-canonical-prefixes uses the dereferenced absolute path for the -cc1 command. For most tests "-cc1" is sufficient to identify the command line, no need to specifically test the "clang" command, and -no-canonical-prefixes can be removed.

simpal01 updated this revision to Diff 539031.Jul 11 2023, 5:17 AM

Addressing Review comments.

simpal01 marked 4 inline comments as done.Jul 11 2023, 5:19 AM
MaskRay added inline comments.Jul 11 2023, 9:40 AM
clang/lib/Driver/ToolChains/Arch/ARM.cpp
40

We do not need this variable. After an early return, we can just return Triple.getArch() == llvm::Triple::armeb || ...

simpal01 updated this revision to Diff 539582.Jul 12 2023, 8:33 AM

Addressing Review comment - Removing the variable IsBigEndian.

simpal01 marked an inline comment as done.Jul 12 2023, 8:34 AM

@MaskRay

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

clang/lib/Driver/ToolChains/Arch/ARM.h
77

Arm the company has rebranded but for consistency with other code use the all-caps name.

simpal01 updated this revision to Diff 541522.Jul 18 2023, 7:26 AM

isArmBigEndian -> isARMBigEndian

michaelplatings accepted this revision.Jul 18 2023, 8:11 AM

LGTM but please allow a day for others to comment.

This revision is now accepted and ready to land.Jul 18 2023, 8:11 AM
MaskRay accepted this revision.Jul 18 2023, 9:02 AM