[llvm] [BOLT][RISCV] Set minimum function alignment to 2 for RVC (PR #69837)
Job Noorman via llvm-commits
llvm-commits at lists.llvm.org
Sat Oct 21 04:05:14 PDT 2023
https://github.com/mtvec created https://github.com/llvm/llvm-project/pull/69837
In #67707, the minimum function alignment on RISC-V was set to 4. When
RVC (compressed instructions) is enabled, the minimum alignment can be
reduced to 2.
This patch implements this by delegating the choice of minimum alignment
to a new `MCPlusBuilder::getMinFunctionAlignment` function. This way,
the target-dependent code in `BinaryFunction` is minimized.
This patch depends on #69836. Please review only the last commit here.
>From d5034e57ece3572aa6ba5fe58611c35a5a642792 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Sat, 21 Oct 2023 12:39:17 +0200
Subject: [PATCH 1/2] [BOLT][RISCV] Use target features from object file
We used to hard-code target features for RISC-V. However, most features
(with the exception of relax) are stored in the object file. This patch
extracts those features to ensure BOLT's output doesn't use any features
not present in the input file.
---
bolt/lib/Core/BinaryContext.cpp | 17 +++++++++++++----
bolt/test/RISCV/call-annotations.s | 3 ++-
bolt/test/RISCV/internal-func-reloc.s | 18 +++++++-----------
3 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index f19460f8c1f529c..acc441aca480d49 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -118,7 +118,7 @@ Expected<std::unique_ptr<BinaryContext>>
BinaryContext::createBinaryContext(const ObjectFile *File, bool IsPIC,
std::unique_ptr<DWARFContext> DwCtx) {
StringRef ArchName = "";
- StringRef FeaturesStr = "";
+ std::string FeaturesStr = "";
switch (File->getArch()) {
case llvm::Triple::x86_64:
ArchName = "x86-64";
@@ -128,11 +128,20 @@ BinaryContext::createBinaryContext(const ObjectFile *File, bool IsPIC,
ArchName = "aarch64";
FeaturesStr = "+all";
break;
- case llvm::Triple::riscv64:
+ case llvm::Triple::riscv64: {
ArchName = "riscv64";
- // RV64GC
- FeaturesStr = "+m,+a,+f,+d,+zicsr,+zifencei,+c,+relax";
+ Expected<SubtargetFeatures> Features = File->getFeatures();
+
+ if (auto E = Features.takeError())
+ return E;
+
+ // We rely on relaxation for some transformations (e.g., promoting all calls
+ // to PseudoCALL and then making JITLink relax them). Since the relax
+ // feature is not stored in the object file, we manually enable it.
+ Features->AddFeature("relax");
+ FeaturesStr = Features->getString();
break;
+ }
default:
return createStringError(std::errc::not_supported,
"BOLT-ERROR: Unrecognized machine in ELF file");
diff --git a/bolt/test/RISCV/call-annotations.s b/bolt/test/RISCV/call-annotations.s
index bc539bb0ec7796b..477c4693cd6dd72 100644
--- a/bolt/test/RISCV/call-annotations.s
+++ b/bolt/test/RISCV/call-annotations.s
@@ -1,12 +1,13 @@
/// Test that annotations are properly carried over to fixed calls.
/// Note that --enable-bat is used to force offsets to be kept.
-// RUN: llvm-mc -triple riscv64 -filetype obj -o %t.o %s
+// RUN: llvm-mc -triple riscv64 -mattr=+c -filetype obj -o %t.o %s
// RUN: ld.lld --emit-relocs -o %t %t.o
// RUN: llvm-bolt --enable-bat --print-cfg --print-fix-riscv-calls \
// RUN: -o /dev/null %t | FileCheck %s
.text
+ .option norvc
.global f
.p2align 1
f:
diff --git a/bolt/test/RISCV/internal-func-reloc.s b/bolt/test/RISCV/internal-func-reloc.s
index ea99b564fe42c15..c6c74bed92efea7 100644
--- a/bolt/test/RISCV/internal-func-reloc.s
+++ b/bolt/test/RISCV/internal-func-reloc.s
@@ -2,18 +2,14 @@
/// get transformed by BOLT. The tests rely on the "remove-nops" optimization:
/// if nops got removed from the function, it got transformed by BOLT.
-// RUN: %clang %cflags -o %t %s
+// RUN: llvm-mc -triple riscv64 -filetype=obj -o %t.o %s
+// RUN: ld.lld --emit-relocs -o %t %t.o
// RUN: llvm-bolt -o %t.bolt %t
// RUN: llvm-objdump -d %t.bolt | FileCheck %s
.text
-
- /// These options are only used to make the assembler output easier to predict
- .option norelax
- .option norvc
-
.globl _start
- .p2align 1
+ .p2align 2
// CHECK: <_start>:
// CHECK-NEXT: j 0x{{.*}} <_start>
_start:
@@ -23,10 +19,10 @@ _start:
.size _start, .-_start
.globl f
- .p2align 1
+ .p2align 2
// CHECK: <f>:
-// CHECK-NEXT: auipc a0, 0
-// CHECK-NEXT: addi a0, a0, 64
+// CHECK-NEXT: auipc a0, [[#]]
+// CHECK-NEXT: addi a0, a0, [[#]]
f:
nop
1:
@@ -37,7 +33,7 @@ f:
.size f, .-f
.globl g
- .p2align 1
+ .p2align 2
g:
ret
.size g, .-g
>From b88b3b15a5108353f075915a520d162a7d986a41 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Sat, 21 Oct 2023 12:55:57 +0200
Subject: [PATCH 2/2] [BOLT][RISCV] Set minimum function alignment to 2 for RVC
In #67707, the minimum function alignment on RISC-V was set to 4. When
RVC (compressed instructions) is enabled, the minimum alignment can be
reduced to 2.
This patch implements this by delegating the choice of minimum alignment
to a new `MCPlusBuilder::getMinFunctionAlignment` function. This way,
the target-dependent code in `BinaryFunction` is minimized.
---
bolt/include/bolt/Core/BinaryFunction.h | 9 +----
bolt/include/bolt/Core/MCPlusBuilder.h | 6 +++
.../Target/AArch64/AArch64MCPlusBuilder.cpp | 2 +
bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp | 5 +++
bolt/test/RISCV/function-alignment.s | 38 +++++++++++++++++++
5 files changed, 52 insertions(+), 8 deletions(-)
create mode 100644 bolt/test/RISCV/function-alignment.s
diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h
index a4c9eb4265ec9aa..c67ddccbf4892a7 100644
--- a/bolt/include/bolt/Core/BinaryFunction.h
+++ b/bolt/include/bolt/Core/BinaryFunction.h
@@ -1721,14 +1721,7 @@ class BinaryFunction {
// Align data in code BFs minimum to CI alignment
if (!size() && hasIslandsInfo())
return getConstantIslandAlignment();
-
- // Minimal code alignment on AArch64 and RISCV is 4
- if (BC.isAArch64() || BC.isRISCV())
- return 4;
-
- // We have to use at least 2-byte alignment for functions because
- // of C++ ABI.
- return 2;
+ return BC.MIB->getMinFunctionAlignment();
}
Align getMinAlign() const { return Align(getMinAlignment()); }
diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index b6817c1d7f45ed3..d136c627bc5cc10 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -2077,6 +2077,12 @@ class MCPlusBuilder {
return BlocksVectorTy();
}
+ virtual uint16_t getMinFunctionAlignment() const {
+ // We have to use at least 2-byte alignment for functions because of C++
+ // ABI.
+ return 2;
+ }
+
// AliasMap caches a mapping of registers to the set of registers that
// alias (are sub or superregs of itself, including itself).
std::vector<BitVector> AliasMap;
diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
index c2a4607411a49e1..b31813680f12122 100644
--- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
+++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
@@ -1643,6 +1643,8 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
return Relocation({RelOffset, RelSymbol, RelType, RelAddend, 0});
}
+
+ uint16_t getMinFunctionAlignment() const override { return 4; }
};
} // end anonymous namespace
diff --git a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
index 85855fbf3ab97f4..236acc4fee47ddd 100644
--- a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
+++ b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
@@ -15,6 +15,7 @@
#include "bolt/Core/MCPlusBuilder.h"
#include "llvm/BinaryFormat/ELF.h"
#include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCSubtargetInfo.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/Format.h"
@@ -490,6 +491,10 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
assert(Second.getOpcode() == RISCV::JALR);
return true;
}
+
+ uint16_t getMinFunctionAlignment() const override {
+ return STI->getFeatureBits()[RISCV::FeatureStdExtC] ? 2 : 4;
+ }
};
} // end anonymous namespace
diff --git a/bolt/test/RISCV/function-alignment.s b/bolt/test/RISCV/function-alignment.s
new file mode 100644
index 000000000000000..c52cff9e2347a5c
--- /dev/null
+++ b/bolt/test/RISCV/function-alignment.s
@@ -0,0 +1,38 @@
+## Test that BOLT uses a minimum function alignment of 4 (or 2 for RVC) bytes.
+
+# RUN: llvm-mc -triple=riscv64 -filetype=obj -o %t.o %s
+# RUN: ld.lld -q -o %t %t.o
+# RUN: llvm-bolt --align-functions=1 --use-old-text=0 -o %t.bolt %t
+# RUN: llvm-nm -n %t.bolt | FileCheck %s
+
+# RUN: llvm-mc -triple=riscv64 -mattr=+c -filetype=obj -o %t-c.o %s
+# RUN: ld.lld -q -o %t-c %t-c.o
+# RUN: llvm-bolt --align-functions=1 --use-old-text=0 -o %t-c.bolt %t-c
+# RUN: llvm-nm -n %t-c.bolt | FileCheck --check-prefix=CHECK-C %s
+
+# CHECK: {{[048c]}} T _start
+# CHECK-NEXT: {{[048c]}} T dummy
+
+# CHECK-C: {{[02468ace]}} T _start
+# CHECK-C-NEXT: {{[02468ace]}} T dummy
+
+ .text
+
+ # Make sure input binary is only 1 byte aligned. BOLT should increase the
+ # alignment to 2 or 4 bytes.
+ .byte 0
+ .balign 1
+
+ .globl _start
+ .type _start, @function
+_start:
+ # Dummy reloc to force relocation mode.
+ .reloc 0, R_RISCV_NONE
+ ret
+ .size _start, .-_start
+
+ .globl dummy
+ .type dummy, @function
+dummy:
+ ret
+ .size dummy, .-dummy
More information about the llvm-commits
mailing list