[llvm] 401f768 - [PGO] Setting ValueProfNode Array's Alignment

[Qiongsi Wu via llvm-commits]

Author: Qiongsi Wu
Date: 2023-02-23T19:07:29-05:00
New Revision: 401f768061e6b7498f6f5d50f8e31143c4d6ae2d

URL: https://github.com/llvm/llvm-project/commit/401f768061e6b7498f6f5d50f8e31143c4d6ae2d
DIFF: https://github.com/llvm/llvm-project/commit/401f768061e6b7498f6f5d50f8e31143c4d6ae2d.diff

LOG: [PGO] Setting ValueProfNode Array's Alignment

`instrprof` currently does not set `__llvm_prf_vnds`'s alignment after creating it. The consequence is that the alignment is set to 16 later (https://github.com/llvm/llvm-project/blob/c0f3ac1d0015fd051144a987ff500b888a32be86/llvm/lib/IR/DataLayout.cpp#L1019). This can lead to undefined behaviour when we calculate `NumVNodes` in `lprofGetLoadModuleSignature` (https://github.com/llvm/llvm-project/blob/c0f3ac1d0015fd051144a987ff500b888a32be86/compiler-rt/lib/profile/InstrProfilingMerge.c#L32). The reason is that when the `__llvm_prf_vnds` array is 16 byte aligned, `__llvm_profile_end_vnodes() - __llvm_profile_begin_vnodes()` may not be a multiple of the size of ValueProfNode (which is 24, 20 on 32 bit targets).

This patch sets `__llvm_prf_vnds`'s alignment to its ABI alignment, which always divides its size. Then `__llvm_profile_end_vnodes() - __llvm_profile_begin_vnodes()` will be a multiple of `sizeof(ValueProfNode)`.

Reviewed By: w2yehia, MaskRay

Differential Revision: https://reviews.llvm.org/D144302

Added: 
    llvm/test/Instrumentation/InstrProfiling/align32.ll

Modified: 
    llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
    llvm/test/Instrumentation/InstrProfiling/icall-comdat.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
index 54e76eca2cb26..aea70fd4b26d1 100644
--- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -1180,6 +1180,7 @@ void InstrProfiling::emitVNodes() {
       Constant::getNullValue(VNodesTy), getInstrProfVNodesVarName());
   VNodesVar->setSection(
       getInstrProfSectionName(IPSK_vnodes, TT.getObjectFormat()));
+  VNodesVar->setAlignment(M->getDataLayout().getABITypeAlign(VNodesTy));
   // VNodesVar is used by runtime but not referenced via relocation by other
   // sections. Conservatively make it linker retained.
   UsedVars.push_back(VNodesVar);

diff  --git a/llvm/test/Instrumentation/InstrProfiling/align32.ll b/llvm/test/Instrumentation/InstrProfiling/align32.ll
new file mode 100644
index 0000000000000..4cb9870fb6ea5
--- /dev/null
+++ b/llvm/test/Instrumentation/InstrProfiling/align32.ll
@@ -0,0 +1,38 @@
+;; Check that the prf_vnodes variable has the correct alignment
+;; when pointers are 4 byte long.
+; RUN: opt %s -passes=instrprof -S | FileCheck %s --check-prefix=ALIGN32
+
+target datalayout = "e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-f64:32:64-f80:32-n8:16:32-S128"
+target triple = "i386-unknown-linux-gnu"
+
+ at __profn_foo = private constant [3 x i8] c"foo"
+ at __profn_bar = private constant [3 x i8] c"bar"
+
+define i32 @foo(ptr ) {
+  call void @llvm.instrprof.increment(ptr @__profn_foo, i64 12884901887, i32 1, i32 0)
+  %2 = ptrtoint ptr %0 to i64
+  call void @llvm.instrprof.value.profile(ptr @__profn_foo, i64 12884901887, i64 %2, i32 0, i32 0)
+  %3 = tail call i32 %0()
+  ret i32 %3
+}
+
+$bar = comdat any
+
+define i32 @bar(ptr ) comdat {
+entry:
+  call void @llvm.instrprof.increment(ptr @__profn_bar, i64 12884901887, i32 1, i32 0)
+  %1 = ptrtoint ptr %0 to i64
+  call void @llvm.instrprof.value.profile(ptr @__profn_bar, i64 12884901887, i64 %1, i32 0, i32 0)
+  %2 = tail call i32 %0()
+  ret i32 %2
+}
+
+; Function Attrs: nounwind
+declare void @llvm.instrprof.increment(ptr, i64, i32, i32) #0
+
+; Function Attrs: nounwind
+declare void @llvm.instrprof.value.profile(ptr, i64, i64, i32, i32) #0
+
+attributes #0 = { nounwind }
+
+; ALIGN32: @__llvm_prf_vnodes = private global {{.*}} section "__llvm_prf_vnds", align 4

diff  --git a/llvm/test/Instrumentation/InstrProfiling/icall-comdat.ll b/llvm/test/Instrumentation/InstrProfiling/icall-comdat.ll
index 124eaca56e42b..c90f37c820895 100644
--- a/llvm/test/Instrumentation/InstrProfiling/icall-comdat.ll
+++ b/llvm/test/Instrumentation/InstrProfiling/icall-comdat.ll
@@ -10,7 +10,11 @@
 ; RUN: opt < %s -mtriple=mips64-unknown-linux -passes=instrprof -vp-static-alloc=true -S | FileCheck %s --check-prefix=STATIC-SEXT
 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes=instrprof -vp-static-alloc=false -S | FileCheck %s --check-prefix=DYN
 
-
+;; Check that counters have the correct alignments.
+; RUN: opt %s -mtriple=powerpc64-unknown-linux -passes=instrprof -S | FileCheck %s --check-prefix=ALIGN
+; RUN: opt %s -mtriple=powerpc-ibm-aix -passes=instrprof -S | FileCheck %s --check-prefix=ALIGN
+; RUN: opt %s -mtriple=powerpc64-ibm-aix -passes=instrprof -S | FileCheck %s --check-prefix=ALIGN
+; RUN: opt %s -mtriple=x86_64-unknown-linux -passes=instrprof -S | FileCheck %s --check-prefix=ALIGN
 
 @__profn_foo = private constant [3 x i8] c"foo"
 @__profn_bar = private constant [3 x i8] c"bar"
@@ -62,3 +66,12 @@ attributes #0 = { nounwind }
 ; STATIC: declare void @__llvm_profile_instrument_target(i64, ptr, i32)
 ; STATIC-EXT: declare void @__llvm_profile_instrument_target(i64, ptr, i32 zeroext)
 ; STATIC-SEXT: declare void @__llvm_profile_instrument_target(i64, ptr, i32 signext)
+
+; ALIGN: @__profc_foo = private global {{.*}} section "__llvm_prf_cnts",{{.*}} align 8
+; ALIGN: @__profvp_foo = private global {{.*}} section "__llvm_prf_vals",{{.*}} align 8
+; ALIGN: @__profd_foo = private global {{.*}} section "__llvm_prf_data",{{.*}} align 8
+; ALIGN: @__profc_bar = private global {{.*}} section "__llvm_prf_cnts",{{.*}} align 8
+; ALIGN: @__profvp_bar = private global {{.*}} section "__llvm_prf_vals",{{.*}}  align 8
+; ALIGN: @__profd_bar = private global {{.*}} section "__llvm_prf_data",{{.*}} align 8
+; ALIGN: @__llvm_prf_vnodes = private global {{.*}} section "__llvm_prf_vnds", align 8
+; ALIGN: @__llvm_prf_nm = private constant {{.*}} section "__llvm_prf_names", align 1