MC: Add .data. and .rodata. prefixes to MCContext section classification

[davemarchevsky]

Summary

Commit 463da422f019 ("MC: make section classification a bit more
thorough") changed MCContext::getELFSection section classification logic
to default to SectionKind::getText (previously default was
SectionKind::getReadOnly) and added some matching based on section name
to determine internal section classification.

The BPF runtime implements global variables using 'BPF map'
datastructures, specifically the arraymap BPF map type. Global variables
in a section are placed in a single arraymap value at predictable byte
offsets. Variables in different sections are placed in separate
arraymaps, so in this example:

#define SEC(name) __attribute__((section(name)))
SEC(".data.A") u32 one;
SEC(".data.A") u32 two;
SEC(".data.B") u32 three;
SEC(".data.B") u32 four;

variables one and two would correspond to some byte offsets (probably 0
and 4) in one arraymap, while three and four would be in a separate
arraymap. Variables of a bpf_spin_lock type are considered to protect
next-generation BPF datastructure types in the same arraymap value and
there can only be a single bpf_spin_lock variable per arraymap value -
and thus per section.

As a result it's necessary to keep bpf_spin_locks and the datastructures
they guard in separate data sections. Before the aforementioned commit,
a section whose name starts with ".data." - like ".data.A" - would be
classified as SectionKind::getReadOnly, whereas after it is
SectionKind::getText. If 4-byte padding is required in such a section due to
alignment of some symbol within it, classification of the section as
SectionKind::getText will result in compilation of those variables to
BPF backend failing with an error like "unable to write nop sequence of
4 bytes". This is due to nop instruction emitted in
BPFAsmBackend::writeNopData being 8 bytes, so the function fails since
it cannot emit a 4-byte nop instruction.

Let's follow the pattern of matching section names starting with ".bss."
and ".tbss." prefixes resulting in proper classification of the section
as data by adding similar matches for ".data." and ".rodata." prefixes.
This will bring padding behavior for these sections back to what it was
before that commit and fix the crash.

Signed-off-by: Dave Marchevsky <[e-mail]>

[compnerd]

llvm/test/MC/ELF/data-section-prefix.ll

2

Could you please verify the output?

[davemarchevsky]

v1 -> v2: Use llvm-readobj and FileCheck to confirm that .data.A section is emitted with correct flags

[davemarchevsky]

llvm/test/MC/ELF/data-section-prefix.ll

2

sorry for delay, addressed!

[compnerd]

Could you please also add a test case for .rodata.suffix as that is added as a case as well.

[davemarchevsky]

Add test data and FileCheck annotations to exercise .rodata.suffix logic

[chapuni]

llvm/test/MC/ELF/data-section-prefix.ll

20

Would it run if bpf is not configured?

[yonghong-song]

llvm/test/MC/ELF/data-section-prefix.ll

20

Thanks for pointing this out. Yes, the test will fail if bpf is not configured. I pushed https://github.com/llvm/llvm-project/commit/5922d361229430a373e007a8e82ad69c7186d56d which fixed the issue.

[Amir]

Hi,

This commit might have caused multi-stage lto+pgo build breakage: https://lab.llvm.org/buildbot/#/builders/246/builds/1358.
There are other commits included in the build but they seem to be unrelated.
Can you please verify?

[yonghong-song]

@Amir

Somehow, CCACHE does not work for me, which has an error message like missing 'malloc/malloc.h'. I used the following scripts without CCACHE.

$ cat build_llvm_debug.sh
cd llvm-project/llvm/build

#    -DLLVM_CCACHE_BUILD=ON \
#    -C llvm-project/clang/cmake/caches/BOLT-PGO.cmake
#    -DBOOTSTRAP_LLVM_CCACHE_BUILD=ON \

cmake .. \
    -DLLVM_APPEND_VC_REV=OFF \
    -DLLVM_ENABLE_LLD=ON \
    -DBOOTSTRAP_LLVM_ENABLE_LLD=ON \
    -DBOOTSTRAP_BOOTSTRAP_LLVM_ENABLE_LLD=ON \
    -DPGO_INSTRUMENT_LTO=Thin \
    -DLLVM_ENABLE_PROJECTS="clang;lld;bolt;llvm" \
    -DCMAKE_BUILD_TYPE=Release \
    -DLLVM_ENABLE_ASSERTIONS=ON \
    -DLLVM_LIT_ARGS="-v -vv" \
    -G Ninja \
    -DCMAKE_INSTALL_PREFIX=$PWD/install

ninja && ninja install

It works fine with me,

...
[4878/4882] Building CXX object tools/obj2yaml/CMakeFiles/obj2yaml.dir/elf2yaml.cpp.o
[4879/4882] Linking CXX executable bin/obj2yaml
[4880/4882] Building CXX object tools/llvm-readobj/CMakeFiles/llvm-readobj.dir/ELFDumper.cpp.o
[4881/4882] Linking CXX executable bin/llvm-readobj
[4882/4882] Generating ../../bin/llvm-readelf
[1/4] Performing build step for 'bolt_rt'
ninja: no work to do.
[2/4] Performing install step for 'bolt_rt'
[0/1] Install the project...
-- Install configuration: "Release"
-- Up-to-date: /home/yhs/work/llvm-project/llvm/build/lib/libbolt_rt_instr.a
-- Up-to-date: /home/yhs/work/llvm-project/llvm/build/lib/libbolt_rt_hugify.a
[3/4] Completed 'bolt_rt'
[3/4] Install the project...
-- Install configuration: "Release"
-- Installing: /home/yhs/work/llvm-project/llvm/build/install/include/llvm
...

Does CCACHE impact the build? If you can help me resolve CCACHE build issue, I am happy to try the build again.

[Amir]

In D138477#4019166, @yonghong-song wrote:

@Amir

Somehow, CCACHE does not work for me, which has an error message like missing 'malloc/malloc.h'. I used the following scripts without CCACHE.

$ cat build_llvm_debug.sh
cd llvm-project/llvm/build

#    -DLLVM_CCACHE_BUILD=ON \
#    -C llvm-project/clang/cmake/caches/BOLT-PGO.cmake
#    -DBOOTSTRAP_LLVM_CCACHE_BUILD=ON \

cmake .. \
    -DLLVM_APPEND_VC_REV=OFF \
    -DLLVM_ENABLE_LLD=ON \
    -DBOOTSTRAP_LLVM_ENABLE_LLD=ON \
    -DBOOTSTRAP_BOOTSTRAP_LLVM_ENABLE_LLD=ON \
    -DPGO_INSTRUMENT_LTO=Thin \
    -DLLVM_ENABLE_PROJECTS="clang;lld;bolt;llvm" \
    -DCMAKE_BUILD_TYPE=Release \
    -DLLVM_ENABLE_ASSERTIONS=ON \
    -DLLVM_LIT_ARGS="-v -vv" \
    -G Ninja \
    -DCMAKE_INSTALL_PREFIX=$PWD/install

ninja && ninja install

It works fine with me,

...
[4878/4882] Building CXX object tools/obj2yaml/CMakeFiles/obj2yaml.dir/elf2yaml.cpp.o
[4879/4882] Linking CXX executable bin/obj2yaml
[4880/4882] Building CXX object tools/llvm-readobj/CMakeFiles/llvm-readobj.dir/ELFDumper.cpp.o
[4881/4882] Linking CXX executable bin/llvm-readobj
[4882/4882] Generating ../../bin/llvm-readelf
[1/4] Performing build step for 'bolt_rt'
ninja: no work to do.
[2/4] Performing install step for 'bolt_rt'
[0/1] Install the project...
-- Install configuration: "Release"
-- Up-to-date: /home/yhs/work/llvm-project/llvm/build/lib/libbolt_rt_instr.a
-- Up-to-date: /home/yhs/work/llvm-project/llvm/build/lib/libbolt_rt_hugify.a
[3/4] Completed 'bolt_rt'
[3/4] Install the project...
-- Install configuration: "Release"
-- Installing: /home/yhs/work/llvm-project/llvm/build/install/include/llvm
...

Does CCACHE impact the build? If you can help me resolve CCACHE build issue, I am happy to try the build again.

The build succeeded today without any intervention, sorry for bothering you. Might have been a stale ccache issue.

llvm/lib/MC/MCContext.cpp

                .Case(".data", SectionKind::getData())
                .Case(".data1", SectionKind::getData())
                .Case(".data.rel.ro", SectionKind::getReadOnlyWithRel())
+               .StartsWith(".data.", SectionKind::getData())
                .Case(".rodata", SectionKind::getReadOnly())
                .Case(".rodata1", SectionKind::getReadOnly())
+               .StartsWith(".rodata.", SectionKind::getReadOnly())
                .Case(".tbss", SectionKind::getThreadBSS())
                .StartsWith(".tbss.", SectionKind::getThreadData())
                .StartsWith(".gnu.linkonce.tb.", SectionKind::getThreadData())

llvm/test/MC/ELF/data-section-prefix.ll

+; RUN: llc -filetype obj -o - %s | llvm-readobj --sections - | FileCheck --check-prefix="SECTIONS" %s
+;
+; SECTIONS:         Name: .data.A
+; SECTIONS-NEXT:    Type: SHT_PROGBITS (0x1)
+; SECTIONS-NEXT:        Flags [ (0x3)
+; SECTIONS-NEXT:          SHF_ALLOC (0x2)
+; SECTIONS-NEXT:          SHF_WRITE (0x1)
+; SECTIONS-NEXT:    ]
+
+target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
+target triple = "bpf"
+
+@glock = dso_local local_unnamed_addr global i32 0, section ".data.A", align 8
+@ghead = dso_local local_unnamed_addr global i32 0, section ".data.A", align 8