Skip to content

Conversation

@ellishg
Copy link
Contributor

@ellishg ellishg commented Oct 10, 2024

Autogenerate .ll code from cpp code in some -icf-safe-thunk tests using update_test_body.py

PATH=build/bin:$PATH llvm/utils/update_test_body.py lld/test/MachO/icf-safe-thunks.ll lld/test/MachO/icf-safe-thunks-dwarf.ll

https://llvm.org/docs/TestingGuide.html#elaborated-tests

I recently became aware of this tool and I wanted to practice using it. This also allows to remove the custom instructions to generate the .ll code.

@llvmbot
Copy link
Member

llvmbot commented Oct 10, 2024

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-macho

Author: Ellis Hoag (ellishg)

Changes

Autogenerate .ll code from cpp code in some -icf-safe-thunk tests using update_test_body.py

PATH=build/bin:$PATH llvm/utils/update_test_body.py lld/test/MachO/icf-safe-thunks.ll lld/test/MachO/icf-safe-thunks-dwarf.ll

https://llvm.org/docs/TestingGuide.html#elaborated-tests

I recently became aware of this tool and I wanted to practice using it. This also allows to remove the custom instructions to generate the .ll code.


Patch is 22.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/111927.diff

2 Files Affected:

  • (modified) lld/test/MachO/icf-safe-thunks-dwarf.ll (+67-79)
  • (modified) lld/test/MachO/icf-safe-thunks.ll (+108-152)
diff --git a/lld/test/MachO/icf-safe-thunks-dwarf.ll b/lld/test/MachO/icf-safe-thunks-dwarf.ll
index 74f3fb7a033db8..8c07f9fc98a935 100644
--- a/lld/test/MachO/icf-safe-thunks-dwarf.ll
+++ b/lld/test/MachO/icf-safe-thunks-dwarf.ll
@@ -1,17 +1,17 @@
 ; REQUIRES: aarch64
 
-;;; Build the
-; RUN: rm -rf %t; mkdir %t
-; RUN: llc -filetype=obj %s -O3 -o %t/icf-obj-safe-thunks-dwarf.o -enable-machine-outliner=never -mtriple arm64-apple-macos -addrsig
-; RUN: %lld -arch arm64 -lSystem --icf=safe_thunks -dylib -o %t/icf-safe-dwarf.dylib %t/icf-obj-safe-thunks-dwarf.o
+; RUN: rm -rf %t && split-file %s %t
+
+; RUN: llc -filetype=obj %t/a.ll -O3 -o %t/a.o -enable-machine-outliner=never -mtriple arm64-apple-macos -addrsig
+; RUN: %lld -arch arm64 -lSystem --icf=safe_thunks -dylib -o %t/a.dylib %t/a.o
 
 ;;; Check that we generate valid dSYM
-; RUN: dsymutil %t/icf-safe-dwarf.dylib -o %t/icf-safe.dSYM
-; RUN: llvm-dwarfdump --verify %t/icf-safe.dSYM | FileCheck %s --check-prefix=VERIFY-DSYM
+; RUN: dsymutil %t/a.dylib -o %t/a.dSYM
+; RUN: llvm-dwarfdump --verify %t/a.dSYM | FileCheck %s --check-prefix=VERIFY-DSYM
 ; VERIFY-DSYM: No errors.
 
 ;;; Check that we don't generate STABS entries (N_FUN) for ICF'ed function thunks
-; RUN: dsymutil -s %t/icf-safe-dwarf.dylib | FileCheck %s --check-prefix=VERIFY-STABS
+; RUN: dsymutil -s %t/a.dylib | FileCheck %s --check-prefix=VERIFY-STABS
 ; VERIFY-STABS-NOT:  N_FUN{{.*}}_func_B
 ; VERIFY-STABS-NOT:  N_FUN{{.*}}_func_C
 
@@ -19,97 +19,85 @@
 ; VERIFY-STABS:  N_FUN{{.*}}_func_A
 ; VERIFY-STABS:  N_FUN{{.*}}_take_func_addr
 
+;--- a.cpp
+#define ATTR __attribute__((noinline)) extern "C"
+typedef unsigned long long ULL;
+
+ATTR int func_A() { return 1; }
+ATTR int func_B() { return 1; }
+ATTR int func_C() { return 1; }
+
+ATTR ULL take_func_addr() {
+    ULL val = 0;
+    val += (ULL)(void*)func_A;
+    val += (ULL)(void*)func_B;
+    val += (ULL)(void*)func_C;
+    return val;
+}
+
+;--- gen
+clang -target arm64-apple-macos11.0 -S -emit-llvm a.cpp -O3 -g -o -
 
-; ModuleID = 'icf-safe-thunks-dwarf.cpp'
-source_filename = "icf-safe-thunks-dwarf.cpp"
+;--- a.ll
+; ModuleID = 'a.cpp'
+source_filename = "a.cpp"
 target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128-Fn32"
 target triple = "arm64-apple-macosx11.0.0"
 
-; Function Attrs: mustprogress noinline nounwind optnone ssp uwtable(sync)
-define i32 @func_A() #0 !dbg !13 {
-entry:
-  ret i32 1
+; Function Attrs: mustprogress nofree noinline norecurse nosync nounwind ssp willreturn memory(none) uwtable(sync)
+define noundef i32 @func_A() #0 !dbg !12 {
+  ret i32 1, !dbg !16
 }
 
-; Function Attrs: mustprogress noinline nounwind optnone ssp uwtable(sync)
-define i32 @func_B() #0 !dbg !18 {
-entry:
-  ret i32 1
+; Function Attrs: mustprogress nofree noinline norecurse nosync nounwind ssp willreturn memory(none) uwtable(sync)
+define noundef i32 @func_B() #0 !dbg !17 {
+  ret i32 1, !dbg !18
 }
 
-; Function Attrs: mustprogress noinline nounwind optnone ssp uwtable(sync)
-define i32 @func_C() #0 !dbg !20 {
-entry:
-  ret i32 1
+; Function Attrs: mustprogress nofree noinline norecurse nosync nounwind ssp willreturn memory(none) uwtable(sync)
+define noundef i32 @func_C() #0 !dbg !19 {
+  ret i32 1, !dbg !20
 }
 
-; Function Attrs: mustprogress noinline nounwind optnone ssp uwtable(sync)
-define i64 @take_func_addr() #0 !dbg !22 {
-entry:
-  %val = alloca i64, align 8
-  store i64 0, ptr %val, align 8
-  %0 = load i64, ptr %val, align 8
-  %add = add i64 %0, ptrtoint (ptr @func_A to i64)
-  store i64 %add, ptr %val, align 8
-  %1 = load i64, ptr %val, align 8
-  %add1 = add i64 %1, ptrtoint (ptr @func_B to i64)
-  store i64 %add1, ptr %val, align 8
-  %2 = load i64, ptr %val, align 8
-  %add2 = add i64 %2, ptrtoint (ptr @func_C to i64)
-  store i64 %add2, ptr %val, align 8
-  %3 = load i64, ptr %val, align 8
-  ret i64 %3
+; Function Attrs: mustprogress nofree noinline norecurse nosync nounwind ssp willreturn memory(none) uwtable(sync)
+define noundef i64 @take_func_addr() local_unnamed_addr #0 !dbg !21 {
+    #dbg_value(i64 0, !25, !DIExpression(), !26)
+    #dbg_value(i64 ptrtoint (ptr @func_A to i64), !25, !DIExpression(), !26)
+    #dbg_value(i64 add (i64 ptrtoint (ptr @func_A to i64), i64 ptrtoint (ptr @func_B to i64)), !25, !DIExpression(), !26)
+    #dbg_value(i64 add (i64 add (i64 ptrtoint (ptr @func_A to i64), i64 ptrtoint (ptr @func_B to i64)), i64 ptrtoint (ptr @func_C to i64)), !25, !DIExpression(), !26)
+  ret i64 add (i64 add (i64 ptrtoint (ptr @func_A to i64), i64 ptrtoint (ptr @func_B to i64)), i64 ptrtoint (ptr @func_C to i64)), !dbg !27
 }
 
-attributes #0 = { noinline nounwind }
+attributes #0 = { mustprogress nofree noinline norecurse nosync nounwind ssp willreturn memory(none) uwtable(sync) "frame-pointer"="non-leaf" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="apple-m1" "target-features"="+aes,+altnzcv,+ccdp,+ccidx,+complxnum,+crc,+dit,+dotprod,+flagm,+fp-armv8,+fp16fml,+fptoint,+fullfp16,+jsconv,+lse,+neon,+pauth,+perfmon,+predres,+ras,+rcpc,+rdm,+sb,+sha2,+sha3,+specrestrict,+ssbs,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8a,+zcm,+zcz" }
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!6, !7, !8, !9, !10, !11}
-!llvm.ident = !{!12}
 
-!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 20.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: Apple, sysroot: "/")
-!1 = !DIFile(filename: "icf-safe-thunks-dwarf.cpp", directory: "/tmp/test")
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, retainedTypes: !2, splitDebugInlining: false, nameTableKind: Apple, sysroot: "/")
+!1 = !DIFile(filename: "a.cpp", directory: "/proc/self/cwd")
+!2 = !{!3, !5}
+!3 = !DIDerivedType(tag: DW_TAG_typedef, name: "ULL", file: !1, line: 2, baseType: !4)
+!4 = !DIBasicType(name: "unsigned long long", size: 64, encoding: DW_ATE_unsigned)
+!5 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 64)
 !6 = !{i32 7, !"Dwarf Version", i32 4}
 !7 = !{i32 2, !"Debug Info Version", i32 3}
 !8 = !{i32 1, !"wchar_size", i32 4}
 !9 = !{i32 8, !"PIC Level", i32 2}
 !10 = !{i32 7, !"uwtable", i32 1}
 !11 = !{i32 7, !"frame-pointer", i32 1}
-!12 = !{!"clang version 20.0.0"}
-!13 = distinct !DISubprogram(name: "func_A", scope: !1, file: !1, line: 4, type: !14, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0)
-!14 = !DISubroutineType(types: !15)
-!15 = !{}
-!18 = distinct !DISubprogram(name: "func_B", scope: !1, file: !1, line: 5, type: !14, scopeLine: 5, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0)
-!20 = distinct !DISubprogram(name: "func_C", scope: !1, file: !1, line: 6, type: !14, scopeLine: 6, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0)
-!22 = distinct !DISubprogram(name: "take_func_addr", scope: !1, file: !1, line: 8, type: !14, scopeLine: 8, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0)
-
-
-
-
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;;;;;;;;;;;;;; Generate the above LLVM IR with the below script ;;;;;;;;;;;;;;;
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-; #!/bin/bash
-; set -ex
-; TOOLCHAIN_BIN="llvm-project/build/Debug/bin"
-;
-; # Create icf-safe-thunks-dwarf.cpp file
-; cat > icf-safe-thunks-dwarf.cpp <<EOF
-; #define ATTR __attribute__((noinline)) extern "C"
-; typedef unsigned long long ULL;
-;
-; ATTR int func_A() { return 1; }
-; ATTR int func_B() { return 1; }
-; ATTR int func_C() { return 1; }
-;
-; ATTR ULL take_func_addr() {
-;     ULL val = 0;
-;     val += (ULL)(void*)func_A;
-;     val += (ULL)(void*)func_B;
-;     val += (ULL)(void*)func_C;
-;     return val;
-; }
-; EOF
-;
-; $TOOLCHAIN_BIN/clang -target arm64-apple-macos11.0 -S -emit-llvm -g \
-;                      icf-safe-thunks-dwarf.cpp
+!12 = distinct !DISubprogram(name: "func_A", scope: !1, file: !1, line: 4, type: !13, scopeLine: 4, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!13 = !DISubroutineType(types: !14)
+!14 = !{!15}
+!15 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!16 = !DILocation(line: 4, column: 21, scope: !12)
+!17 = distinct !DISubprogram(name: "func_B", scope: !1, file: !1, line: 5, type: !13, scopeLine: 5, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!18 = !DILocation(line: 5, column: 21, scope: !17)
+!19 = distinct !DISubprogram(name: "func_C", scope: !1, file: !1, line: 6, type: !13, scopeLine: 6, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!20 = !DILocation(line: 6, column: 21, scope: !19)
+!21 = distinct !DISubprogram(name: "take_func_addr", scope: !1, file: !1, line: 8, type: !22, scopeLine: 8, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !24)
+!22 = !DISubroutineType(types: !23)
+!23 = !{!3}
+!24 = !{!25}
+!25 = !DILocalVariable(name: "val", scope: !21, file: !1, line: 9, type: !3)
+!26 = !DILocation(line: 0, scope: !21)
+!27 = !DILocation(line: 13, column: 5, scope: !21)
diff --git a/lld/test/MachO/icf-safe-thunks.ll b/lld/test/MachO/icf-safe-thunks.ll
index 95e00a5b98385b..82e00ebbc2126f 100644
--- a/lld/test/MachO/icf-safe-thunks.ll
+++ b/lld/test/MachO/icf-safe-thunks.ll
@@ -1,10 +1,11 @@
 ; REQUIRES: aarch64
 
-; RUN: rm -rf %t; mkdir %t
-; RUN: llc -filetype=obj %s -O3 -o %t/icf-obj-safe-thunks.o -enable-machine-outliner=never -mtriple arm64-apple-macos -addrsig
-; RUN: %lld -arch arm64 -lSystem --icf=safe_thunks -dylib -o %t/icf-safe.dylib -map %t/icf-safe.map %t/icf-obj-safe-thunks.o
-; RUN: llvm-objdump %t/icf-safe.dylib -d --macho | FileCheck %s --check-prefixes=CHECK-ARM64
-; RUN: cat %t/icf-safe.map | FileCheck %s --check-prefixes=CHECK-ARM64-MAP
+; RUN: rm -rf %t && split-file %s %t
+
+; RUN: llc -filetype=obj %t/a.ll -O3 -o %t/a.o -enable-machine-outliner=never -mtriple arm64-apple-macos -addrsig
+; RUN: %lld -arch arm64 -lSystem --icf=safe_thunks -dylib -o %t/a.dylib -map %t/a.map %t/a.o
+; RUN: llvm-objdump %t/a.dylib -d --macho | FileCheck %s --check-prefixes=CHECK-ARM64
+; RUN: cat %t/a.map | FileCheck %s --check-prefixes=CHECK-ARM64-MAP
 
 ; CHECK-ARM64:        (__TEXT,__text) section
 ; CHECK-ARM64-NEXT:   _func_unique_1:
@@ -59,6 +60,76 @@
 ; CHECK-ARM64-MAP-NEXT: 0x00000004 [  2] _func_3identical_v2
 ; CHECK-ARM64-MAP-NEXT: 0x00000004 [  2] _func_3identical_v3
 
+;--- a.cpp
+#define ATTR __attribute__((noinline)) extern "C"
+typedef unsigned long long ULL;
+
+volatile char g_val = 0;
+void *volatile g_ptr = 0;
+
+ATTR void func_unique_1() { g_val = 1; }
+
+ATTR void func_unique_2_canmerge() { g_val = 2; }
+
+ATTR void func_2identical_v1() { g_val = 2; }
+
+ATTR void func_2identical_v2() { g_val = 2; }
+
+ATTR void func_3identical_v1() { g_val = 3; }
+
+ATTR void func_3identical_v2() { g_val = 3; }
+
+ATTR void func_3identical_v3() { g_val = 3; }
+
+ATTR void func_3identical_v1_canmerge() { g_val = 33; }
+
+ATTR void func_3identical_v2_canmerge() { g_val = 33; }
+
+ATTR void func_3identical_v3_canmerge() { g_val = 33; }
+
+ATTR void func_call_thunked_1_nomerge() {
+    func_2identical_v1();
+    g_val = 77;
+}
+
+ATTR void func_call_thunked_2_nomerge() {
+    func_2identical_v2();
+    g_val = 77;
+}
+
+ATTR void func_call_thunked_2_merge() {
+    func_2identical_v2();
+    g_val = 77;
+}
+
+ATTR void call_all_funcs() {
+    func_unique_1();
+    func_unique_2_canmerge();
+    func_2identical_v1();
+    func_2identical_v2();
+    func_3identical_v1();
+    func_3identical_v2();
+    func_3identical_v3();
+    func_3identical_v1_canmerge();
+    func_3identical_v2_canmerge();
+    func_3identical_v3_canmerge();
+}
+
+ATTR void take_func_addr() {
+    g_ptr = (void*)func_unique_1;
+    g_ptr = (void*)func_2identical_v1;
+    g_ptr = (void*)func_2identical_v2;
+    g_ptr = (void*)func_3identical_v1;
+    g_ptr = (void*)func_3identical_v2;
+    g_ptr = (void*)func_3identical_v3;
+}
+
+;--- gen
+clang -target arm64-apple-macos11.0 -S -emit-llvm a.cpp -O3 -o -
+
+;--- a.ll
+; ModuleID = 'a.cpp'
+source_filename = "a.cpp"
 target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128-Fn32"
 target triple = "arm64-apple-macosx11.0.0"
 
@@ -67,101 +138,87 @@ target triple = "arm64-apple-macosx11.0.0"
 
 ; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
 define void @func_unique_1() #0 {
-entry:
-  store volatile i8 1, ptr @g_val, align 1, !tbaa !5
+  store volatile i8 1, ptr @g_val, align 1, !tbaa !4
   ret void
 }
 
 ; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
 define void @func_unique_2_canmerge() local_unnamed_addr #0 {
-entry:
-  store volatile i8 2, ptr @g_val, align 1, !tbaa !5
+  store volatile i8 2, ptr @g_val, align 1, !tbaa !4
   ret void
 }
 
 ; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
 define void @func_2identical_v1() #0 {
-entry:
-  store volatile i8 2, ptr @g_val, align 1, !tbaa !5
+  store volatile i8 2, ptr @g_val, align 1, !tbaa !4
   ret void
 }
 
 ; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
 define void @func_2identical_v2() #0 {
-entry:
-  store volatile i8 2, ptr @g_val, align 1, !tbaa !5
+  store volatile i8 2, ptr @g_val, align 1, !tbaa !4
   ret void
 }
 
 ; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
 define void @func_3identical_v1() #0 {
-entry:
-  store volatile i8 3, ptr @g_val, align 1, !tbaa !5
+  store volatile i8 3, ptr @g_val, align 1, !tbaa !4
   ret void
 }
 
 ; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
 define void @func_3identical_v2() #0 {
-entry:
-  store volatile i8 3, ptr @g_val, align 1, !tbaa !5
+  store volatile i8 3, ptr @g_val, align 1, !tbaa !4
   ret void
 }
 
 ; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
 define void @func_3identical_v3() #0 {
-entry:
-  store volatile i8 3, ptr @g_val, align 1, !tbaa !5
+  store volatile i8 3, ptr @g_val, align 1, !tbaa !4
   ret void
 }
 
 ; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
 define void @func_3identical_v1_canmerge() local_unnamed_addr #0 {
-entry:
-  store volatile i8 33, ptr @g_val, align 1, !tbaa !5
+  store volatile i8 33, ptr @g_val, align 1, !tbaa !4
   ret void
 }
 
 ; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
 define void @func_3identical_v2_canmerge() local_unnamed_addr #0 {
-entry:
-  store volatile i8 33, ptr @g_val, align 1, !tbaa !5
+  store volatile i8 33, ptr @g_val, align 1, !tbaa !4
   ret void
 }
 
 ; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
 define void @func_3identical_v3_canmerge() local_unnamed_addr #0 {
-entry:
-  store volatile i8 33, ptr @g_val, align 1, !tbaa !5
+  store volatile i8 33, ptr @g_val, align 1, !tbaa !4
   ret void
 }
 
-; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
-define void @func_call_thunked_1_nomerge() local_unnamed_addr #0 {
-entry:
+; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp uwtable(sync)
+define void @func_call_thunked_1_nomerge() local_unnamed_addr #1 {
   tail call void @func_2identical_v1()
-  store volatile i8 77, ptr @g_val, align 1, !tbaa !5
+  store volatile i8 77, ptr @g_val, align 1, !tbaa !4
   ret void
 }
 
-; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
-define void @func_call_thunked_2_nomerge() local_unnamed_addr #0 {
-entry:
+; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp uwtable(sync)
+define void @func_call_thunked_2_nomerge() local_unnamed_addr #1 {
   tail call void @func_2identical_v2()
-  store volatile i8 77, ptr @g_val, align 1, !tbaa !5
+  store volatile i8 77, ptr @g_val, align 1, !tbaa !4
   ret void
 }
 
-; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
-define void @func_call_thunked_2_merge() local_unnamed_addr #0 {
-entry:
+; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp uwtable(sync)
+define void @func_call_thunked_2_merge() local_unnamed_addr #1 {
   tail call void @func_2identical_v2()
-  store volatile i8 77, ptr @g_val, align 1, !tbaa !5
+  store volatile i8 77, ptr @g_val, align 1, !tbaa !4
   ret void
 }
 
 ; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp uwtable(sync)
 define void @call_all_funcs() local_unnamed_addr #1 {
-entry:
   tail call void @func_unique_1()
   tail call void @func_unique_2_canmerge()
   tail call void @func_2identical_v1()
@@ -177,127 +234,26 @@ entry:
 
 ; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
 define void @take_func_addr() local_unnamed_addr #0 {
-entry:
-  store volatile ptr @func_unique_1, ptr @g_ptr, align 8, !tbaa !8
-  store volatile ptr @func_2identical_v1, ptr @g_ptr, align 8, !tbaa !8
-  store volatile ptr @func_2identical_v2, ptr @g_ptr, align 8, !tbaa !8
-  store volatile ptr @func_3identical_v1, ptr @g_ptr, align 8, !tbaa !8
-  store volatile ptr @func_3identical_v2, ptr @g_ptr, align 8, !tbaa !8
-  store volatile ptr @func_3identical_v3, ptr @g_ptr, align 8, !tbaa !8
+  store volatile ptr @func_unique_1, ptr @g_ptr, align 8, !tbaa !7
+  store volatile ptr @func_2identical_v1, ptr @g_ptr, align 8, !tbaa !7
+  store volatile ptr @func_2identical_v2, ptr @g_ptr, align 8, !tbaa !7
+  store volatile ptr @func_3identical_v1, ptr @g_ptr, align 8, !tbaa !7
+  store volatile ptr @func_3identical_v2, ptr @g_ptr, align 8, !tbaa !7
+  store volatile ptr @func_3identical_v3, ptr @g_ptr, align 8, !tbaa !7
   ret void
 }
 
-attributes #0 = { mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync) "frame-pointer"="non-leaf" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="apple-m1" "target-features"="+aes,+altnzcv,+ccdp,+complxnum,+crc,+dotprod,+fp-armv8,+fp16fml,+fptoint,+fullfp16,+jsconv,+lse,+neon,+pauth,+perfmon,+predres,+ras,+rcpc,+rdm,+sb,+sha2,+sha3,+specrestrict,+ssbs,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8a,+zcm,+zcz" }
-attributes #1 = { mustprogress nofree noinline norecurse nounwind ssp uwtable(sync) "frame-pointer"="non-leaf" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="apple-m1" "target-features"="+aes,+altnzcv,+ccdp,+complxnum,+crc,+dotprod,+fp-armv8,+fp16fml,+fptoint,+fullfp16,+jsconv,+lse,+neon,+pauth,+perfmon,+predres,+ras,+rcpc,+rdm,+sb,+sha2,+sha3,+specrestrict,+ssbs,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8a,+zcm,+zcz" }
+attributes #0 = { mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync) "frame-pointer"="non-leaf" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="apple-m1" "target-features"="+aes,+altnzcv,+ccdp,+ccidx,+complxnum,+crc,+dit,+dotprod,+flagm,+fp-armv8,+fp16fml,+fptoint,+fullfp16,+jsconv,+lse,+neon,+pauth,+perfmon,+predres,+ras,+rcpc,+rdm,+sb,+sha2,+sha3,+specrestrict,+ssbs,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8a,+zcm,+zcz" }
+attributes #1 = { mustprogress nofree noinline norecurse nounwind ssp uwtable(sync) "frame-pointer"="non-leaf" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="apple-m1" "target-features"="+aes,+altnzcv,+ccdp,+ccidx,+complxnum,+crc,+dit,+dotprod,+flagm,+fp-armv8,+fp16fml,+fptoint,+fullfp16,+jsconv,+lse,+neon,+pauth,+perfmon,+predres,+ras,+rcpc,+rdm,+sb,+sha2,+sha3,+specrestrict,+ssbs,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8a,+zcm,+zcz" }
 
 !llvm.module.flags = !{!0, !1, !2, !3}
-!llvm.ident = !{!4}
 
 !0 = !{i32 1, !"wchar_size", i32 4}
 !1 = !{i32 8, !"PIC Level",...
[truncated]

@alx32
Copy link
Contributor

alx32 commented Oct 11, 2024

Some of the IR was manually edited for simplicity and to remove unnecessary parts (as requested by the reviewers) - ex: removing necessary debug info, function attributes.
Having the full IR directly in there reverts back the changes that addressed this feedback.
Just want to make sure we intentionally want to do this ?

@DataCorrupted
Copy link
Member

I second @alx32 on this one. The script is nice, and I used it a lot.

But it is useful only when the test and the outcome are straightforward, which doesn't seem to be the case here, e.g. the excessive attributes generated could be confusing.

BTW, I remember the script used to generate a leading line saying something like "this is auto generated, don't modify, blah", they don't do it anymore? Like this one

@ellishg
Copy link
Contributor Author

ellishg commented Oct 11, 2024

Some of the IR was manually edited for simplicity and to remove unnecessary parts (as requested by the reviewers) - ex: removing necessary debug info, function attributes.

I think it depends on whether you think the "input test code" is the LLVM IR or the CPP code. If you think the LLVM IR is main code being tested, then we should make it as simple as possible to read and update. That's why I initially wanted some attributes simplified. However, it seems that we are treating the CPP code in the comments as the "true" input test code. Basically, we would rather call clang from lld/test and not check in any LLVM IR at all. Instead, we have to hide the CPP code in the comments and regenerate the IR manually. This PR improves that process by using existing infra that is more reproducible. I also noticed the script does nice things like setting the cwd to /proc/self/cwd and removing the clang commit hash to prevent leaking information about your machine.

But it is useful only when the test and the outcome are straightforward, which doesn't seem to be the case here, e.g. the excessive attributes generated could be confusing.

I know the IR is more complicated, but I'd argue that we should only need to review the CPP code, not the IR.

BTW, I remember the script used to generate a leading line saying something like "this is auto generated, don't modify, blah", they don't do it anymore? Like this one

For some reason the update_test_body.py script does not use the common package which will prefix the test with that line. I'm not sure why, but because it's useful I'll add a custom not at the top of these tests.

@ellishg
Copy link
Contributor Author

ellishg commented Oct 21, 2024

@alx32 Do you think this change is still worthwhile?

Copy link
Contributor

@alx32 alx32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM In general - we can use this type of test as a precedent - maybe we can have this type of tests in the future for more complex test cases.

Comment on lines +4 to +15
; RUN: rm -rf %t && split-file %s %t

; RUN: llc -filetype=obj %t/a.ll -O3 -o %t/a.o -enable-machine-outliner=never -mtriple arm64-apple-macos -addrsig
; RUN: %lld -arch arm64 -lSystem --icf=safe_thunks -dylib -o %t/a.dylib %t/a.o

;;; Check that we generate valid dSYM
; RUN: dsymutil %t/icf-safe-dwarf.dylib -o %t/icf-safe.dSYM
; RUN: llvm-dwarfdump --verify %t/icf-safe.dSYM | FileCheck %s --check-prefix=VERIFY-DSYM
; RUN: dsymutil %t/a.dylib -o %t/a.dSYM
; RUN: llvm-dwarfdump --verify %t/a.dSYM | FileCheck %s --check-prefix=VERIFY-DSYM
; VERIFY-DSYM: No errors.

;;; Check that we don't generate STABS entries (N_FUN) for ICF'ed function thunks
; RUN: dsymutil -s %t/icf-safe-dwarf.dylib | FileCheck %s --check-prefix=VERIFY-STABS
; RUN: dsymutil -s %t/a.dylib | FileCheck %s --check-prefix=VERIFY-STABS
Copy link
Contributor

@alx32 alx32 Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Why did we change the name of the dylibs - does the script not support custom names ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I didn't have to change the names. I'm just used to using %t/a.<blah> for filenames when using split-file. This also aligns icf-safe-thunks.ll and icf-safe-thunks-dwarf.ll closer together.

@@ -1,115 +1,104 @@
; NOTE: Code has been autogenerated by utils/update_test_body.py
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any way to add here the example invocation to regenerate this file ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You just need to run the script with the lit tests as the arguments. You also need to make sure to set PATH so that it uses your just-built clang. This is explained in the docs.

PATH=build/bin:$PATH llvm/utils/update_test_body.py lld/test/MachO/icf-safe-thunks{,-dwarf}.ll

https://llvm.org/docs/TestingGuide.html#elaborated-tests

@ellishg ellishg merged commit ed5072e into llvm:main Oct 21, 2024
8 checks passed
@ellishg ellishg deleted the icf-test-gen branch October 21, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants