[BPF] Use ".L" local prefix label for basic blocks #95103

aengelke · 2024-06-11T12:30:16Z

Previously, PrivateLabelPrefix was default-initialized to "L", so basic block labels were added to the symbol table. This seems like an oversight, so use ".L" for all private labels.

I'm not sure whether this is correct. But there's no documentation anywhere that states something else and MCAsmInfo.h documentation states that PrivateLabelPrefix "Defaults to the same as PrivateGlobalPrefix," so this really seems like an oversight. Also, BPF is the only target where private label and global prefixes differ.

Previously, PrivateLabelPrefix was default-initialized to "L", so basic block labels were added to the symbol table. This seems like an oversight, so use ".L" for all private labels.

4ast

There is no need to add dot. The bpf asm code is often read my humans and extra dot is a visual noise.

aengelke · 2024-06-11T18:42:47Z

The bpf asm code is often read my humans and extra dot is a visual noise.

Is there any technical reason for this deviation? On all other ELF targets, local labels are prefixed with .L. Global private labels are already prefixed with .L. This would increase consistency with other ELF targets and tooling, in particular, basic block labels would no longer be added to the symbol table (as in literally all other targets).

Would removing the dot from private global symbols be an option, so that they are prefixed with L instead .L?

(Just for clarity: I don't care whether it's L or .L. I do, however, want to know why there's a deviation between local basic block labels and other local labels and to increase consistency if possible.)

4ast · 2024-06-11T19:25:53Z

The bpf asm code is often read my humans and extra dot is a visual noise.

basic block labels would no longer be added to the symbol table

that's a regression. they didn't meant to be in symbol table.

Would removing the dot from private global symbols be an option, so that they are prefixed with L instead .L?

Yep. PrivateGlobalPrefix = PrivateLabelPrefix = "L" is certainly better.

llvmbot · 2024-06-11T19:56:56Z

@llvm/pr-subscribers-llvm-binary-utilities
@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-mc

Author: None (aengelke)

Changes

Previously, PrivateLabelPrefix was default-initialized to "L", so basic block labels were added to the symbol table. This seems like an oversight, so use ".L" for all private labels.

I'm not sure whether this is correct. But there's no documentation anywhere that states something else and MCAsmInfo.h documentation states that PrivateLabelPrefix "Defaults to the same as PrivateGlobalPrefix," so this really seems like an oversight. Also, BPF is the only target where private label and global prefixes differ.

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

92 Files Affected:

(modified) llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h (-1)
(modified) llvm/lib/Target/TargetLoweringObjectFile.cpp (+1-1)
(modified) llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id-2.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id-local.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll (+3-3)
(modified) llvm/test/CodeGen/BPF/BTF/filename.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/BTF/func-func-ptr.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/BTF/func-non-void.ll (+3-3)
(modified) llvm/test/CodeGen/BPF/BTF/func-source.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/BTF/func-typedef.ll (+3-3)
(modified) llvm/test/CodeGen/BPF/BTF/func-unused-arg.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/BTF/func-void.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/anon-struct-argument-pragma.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/anon-union-localvar-attr.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/anon-union-localvar-pragma.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/btf-id-duplicate.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/field-reloc-alu32.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/field-reloc-bitfield-1-bpfeb.ll (+33-33)
(modified) llvm/test/CodeGen/BPF/CORE/field-reloc-bitfield-1.ll (+33-33)
(modified) llvm/test/CodeGen/BPF/CORE/field-reloc-bitfield-2-bpfeb.ll (+3-3)
(modified) llvm/test/CodeGen/BPF/CORE/field-reloc-bitfield-2.ll (+3-3)
(modified) llvm/test/CodeGen/BPF/CORE/field-reloc-bitfield-record-align16.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/field-reloc-duplicate.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/field-reloc-st-imm.ll (+4-4)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-array-2.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-array.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-byte-size-1.ll (+4-4)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-byte-size-2.ll (+3-3)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-byte-size-3.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-byte-size-4.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-existence-1.ll (+4-4)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-existence-2.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-existence-3.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-existence-4.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-lshift-1-bpfeb.ll (+4-4)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-lshift-1.ll (+4-4)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-lshift-2.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-rshift-1.ll (+4-4)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-rshift-2.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-rshift-3.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-signedness-1.ll (+4-4)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-signedness-2.ll (+3-3)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-fieldinfo-signedness-3.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-struct.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-typeinfo-enum-value-opaque-pointer.ll (+3-3)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-typeinfo-enum-value.ll (+3-3)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-typeinfo-type-exist.ll (+3-3)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-typeinfo-type-match.ll (+3-3)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-typeinfo-type-size-1.ll (+3-3)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-typeinfo-type-size-2.ll (+3-3)
(modified) llvm/test/CodeGen/BPF/CORE/intrinsic-union.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-access-str.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-basic.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-cast-array-1.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-cast-array-2.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-cast-struct-1.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-cast-struct-2.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-cast-struct-3.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-cast-union-1.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-cast-union-2.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-end-load.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-end-ret.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-fieldinfo-1.ll (+6-6)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-fieldinfo-2-bpfeb.ll (+8-8)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-fieldinfo-2.ll (+8-8)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-global-1.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-global-2.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-global-3.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-middle-chain.ll (+3-3)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-multi-array-1.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-multi-array-2.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-multilevel.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-pointer-1.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-pointer-2.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-simplify-patchable-1.ll (+12-12)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-simplify-patchable-2.ll (+19-19)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-simplify-patchable-3.ll (+14-14)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-struct-anonymous.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-struct-array.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-typedef-array.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-typedef-struct-2.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-typedef-struct.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-typedef-union-2.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-typedef-union.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-typedef.ll (+2-2)
(modified) llvm/test/CodeGen/BPF/CORE/offset-reloc-union.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/CORE/simplify-patchable-liveness-bug.ll (+10-10)
(modified) llvm/test/CodeGen/BPF/GlobalISel/ir-translator-ret.ll (+1-1)
(modified) llvm/test/CodeGen/BPF/objdump_cond_op.ll (+3-6)
(modified) llvm/test/CodeGen/BPF/objdump_cond_op_2.ll (+2-4)
(modified) llvm/test/CodeGen/BPF/sanity.ll (+1-1)
(modified) llvm/test/MC/BPF/insn-unit.s (-1)

diff --git a/llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h b/llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h
index 7b2168458c930..8a3c9fb47cc00 100644
--- a/llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h
+++ b/llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h
@@ -24,7 +24,6 @@ class BPFMCAsmInfo : public MCAsmInfo {
     if (TT.getArch() == Triple::bpfeb)
       IsLittleEndian = false;
 
-    PrivateGlobalPrefix = ".L";
     WeakRefDirective = "\t.weak\t";
 
     UsesELFSectionDirectiveForBSS = true;
diff --git a/llvm/lib/Target/TargetLoweringObjectFile.cpp b/llvm/lib/Target/TargetLoweringObjectFile.cpp
index 26e9f9eb3f327..1206863010aa2 100644
--- a/llvm/lib/Target/TargetLoweringObjectFile.cpp
+++ b/llvm/lib/Target/TargetLoweringObjectFile.cpp
@@ -129,7 +129,7 @@ MCSymbol *TargetLoweringObjectFile::getSymbolWithGlobalValueBase(
   assert(!Suffix.empty());
 
   SmallString<60> NameStr;
-  NameStr += GV->getParent()->getDataLayout().getPrivateGlobalPrefix();
+  NameStr += getContext().getAsmInfo()->getPrivateGlobalPrefix();
   TM.getNameWithPrefix(NameStr, GV, *Mang);
   NameStr.append(Suffix.begin(), Suffix.end());
   return getContext().getOrCreateSymbol(NameStr);
diff --git a/llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id-2.ll b/llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id-2.ll
index 63c56c4dfec57..e034656e06330 100644
--- a/llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id-2.ll
+++ b/llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id-2.ll
@@ -39,7 +39,7 @@ entry:
 ; CHECK:             .long   16                              # FieldReloc
 ; CHECK-NEXT:        .long   10                              # Field reloc section string offset=10
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   4
 ; CHECK-NEXT:        .long   20
 ; CHECK-NEXT:        .long   7
diff --git a/llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id-local.ll b/llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id-local.ll
index 763b7345dcb83..28801fe5e4122 100644
--- a/llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id-local.ll
+++ b/llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id-local.ll
@@ -39,7 +39,7 @@ entry:
 ; CHECK:             .long   16                              # FieldReloc
 ; CHECK-NEXT:        .long   10                              # Field reloc section string offset=10
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   4
 ; CHECK-NEXT:        .long   20
 ; CHECK-NEXT:        .long   7
diff --git a/llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll b/llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
index 2fb8d25a2e07b..ab46b1b1cf1a6 100644
--- a/llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
+++ b/llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
@@ -83,15 +83,15 @@ entry:
 ; CHECK:             .long   16                              # FieldReloc
 ; CHECK-NEXT:        .long   7                               # Field reloc section string offset=7
 ; CHECK-NEXT:        .long   3
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   3
 ; CHECK-NEXT:        .long   48
 ; CHECK-NEXT:        .long   6
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   10
 ; CHECK-NEXT:        .long   48
 ; CHECK-NEXT:        .long   6
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   4
 ; CHECK-NEXT:        .long   48
 ; CHECK-NEXT:        .long   7
diff --git a/llvm/test/CodeGen/BPF/BTF/filename.ll b/llvm/test/CodeGen/BPF/BTF/filename.ll
index 2ae03a64770e9..5ec74c7c2d607 100644
--- a/llvm/test/CodeGen/BPF/BTF/filename.ll
+++ b/llvm/test/CodeGen/BPF/BTF/filename.ll
@@ -53,12 +53,12 @@ define dso_local i32 @test() local_unnamed_addr #0 !dbg !7 {
 ; CHECK-NEXT:        .long   8                       # FuncInfo
 ; CHECK-NEXT:        .long   10                      # FuncInfo section string offset=10
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Lfunc_begin{{[0-9]+}}
+; CHECK-NEXT:        .long   Lfunc_begin{{[0-9]+}}
 ; CHECK-NEXT:        .long   3
 ; CHECK-NEXT:        .long   16                      # LineInfo
 ; CHECK-NEXT:        .long   10                      # LineInfo section string offset=10
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   16
 ; CHECK-NEXT:        .long   0
 ; CHECK-NEXT:        .long   1038                    # Line 1 Col 14
diff --git a/llvm/test/CodeGen/BPF/BTF/func-func-ptr.ll b/llvm/test/CodeGen/BPF/BTF/func-func-ptr.ll
index ea93225e8d588..9abd0b8fcc6f9 100644
--- a/llvm/test/CodeGen/BPF/BTF/func-func-ptr.ll
+++ b/llvm/test/CodeGen/BPF/BTF/func-func-ptr.ll
@@ -84,12 +84,12 @@ entry:
 ; CHECK-NEXT:        .long   8                       # FuncInfo
 ; CHECK-NEXT:        .long   11                      # FuncInfo section string offset=11
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Lfunc_begin0
+; CHECK-NEXT:        .long   Lfunc_begin0
 ; CHECK-NEXT:        .long   3
 ; CHECK-NEXT:        .long   16                      # LineInfo
 ; CHECK-NEXT:        .long   11                      # LineInfo section string offset=11
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   17
 ; CHECK-NEXT:        .long   0
 ; CHECK-NEXT:        .long   3091                    # Line 3 Col 19
diff --git a/llvm/test/CodeGen/BPF/BTF/func-non-void.ll b/llvm/test/CodeGen/BPF/BTF/func-non-void.ll
index 428dfac2e53d5..5ff9e92063121 100644
--- a/llvm/test/CodeGen/BPF/BTF/func-non-void.ll
+++ b/llvm/test/CodeGen/BPF/BTF/func-non-void.ll
@@ -58,16 +58,16 @@ define dso_local i32 @f1(i32 returned) local_unnamed_addr #0 !dbg !7 {
 ; CHECK-NEXT:        .long   8                       # FuncInfo
 ; CHECK-NEXT:        .long   11                      # FuncInfo section string offset=11
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Lfunc_begin0
+; CHECK-NEXT:        .long   Lfunc_begin0
 ; CHECK-NEXT:        .long   3
 ; CHECK-NEXT:        .long   16                      # LineInfo
 ; CHECK-NEXT:        .long   11                      # LineInfo section string offset=11
 ; CHECK-NEXT:        .long   2
-; CHECK-NEXT:        .long   .Lfunc_begin0
+; CHECK-NEXT:        .long   Lfunc_begin0
 ; CHECK-NEXT:        .long   17
 ; CHECK-NEXT:        .long   0
 ; CHECK-NEXT:        .long   1024                    # Line 1 Col 0
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   17
 ; CHECK-NEXT:        .long   0
 ; CHECK-NEXT:        .long   1042                    # Line 1 Col 18
diff --git a/llvm/test/CodeGen/BPF/BTF/func-source.ll b/llvm/test/CodeGen/BPF/BTF/func-source.ll
index 63814d766e6a2..868aa826da277 100644
--- a/llvm/test/CodeGen/BPF/BTF/func-source.ll
+++ b/llvm/test/CodeGen/BPF/BTF/func-source.ll
@@ -53,12 +53,12 @@ entry:
 ; CHECK-NEXT:        .long   8                       # FuncInfo
 ; CHECK-NEXT:        .long   3                       # FuncInfo section string offset=3
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Lfunc_begin0
+; CHECK-NEXT:        .long   Lfunc_begin0
 ; CHECK-NEXT:        .long   2
 ; CHECK-NEXT:        .long   16                      # LineInfo
 ; CHECK-NEXT:        .long   3                       # LineInfo section string offset=3
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   9
 ; CHECK-NEXT:        .long   18
 ; CHECK-NEXT:        .long   1040                    # Line 1 Col 16
diff --git a/llvm/test/CodeGen/BPF/BTF/func-typedef.ll b/llvm/test/CodeGen/BPF/BTF/func-typedef.ll
index ac0813f8a342f..bda2f27255d32 100644
--- a/llvm/test/CodeGen/BPF/BTF/func-typedef.ll
+++ b/llvm/test/CodeGen/BPF/BTF/func-typedef.ll
@@ -71,16 +71,16 @@ entry:
 ; CHECK-NEXT:        .long   8                       # FuncInfo
 ; CHECK-NEXT:        .long   20                      # FuncInfo section string offset=20
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Lfunc_begin0
+; CHECK-NEXT:        .long   Lfunc_begin0
 ; CHECK-NEXT:        .long   5
 ; CHECK-NEXT:        .long   16                      # LineInfo
 ; CHECK-NEXT:        .long   20                      # LineInfo section string offset=20
 ; CHECK-NEXT:        .long   2
-; CHECK-NEXT:        .long   .Lfunc_begin0
+; CHECK-NEXT:        .long   Lfunc_begin0
 ; CHECK-NEXT:        .long   26
 ; CHECK-NEXT:        .long   0
 ; CHECK-NEXT:        .long   3072                    # Line 3 Col 0
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   26
 ; CHECK-NEXT:        .long   0
 ; CHECK-NEXT:        .long   3092                    # Line 3 Col 20
diff --git a/llvm/test/CodeGen/BPF/BTF/func-unused-arg.ll b/llvm/test/CodeGen/BPF/BTF/func-unused-arg.ll
index 95c2f9d4e1c8f..243f2ba770372 100644
--- a/llvm/test/CodeGen/BPF/BTF/func-unused-arg.ll
+++ b/llvm/test/CodeGen/BPF/BTF/func-unused-arg.ll
@@ -58,12 +58,12 @@ define dso_local i32 @f1(i32) local_unnamed_addr #0 !dbg !7 {
 ; CHECK-NEXT:        .long   8                       # FuncInfo
 ; CHECK-NEXT:        .long   11                      # FuncInfo section string offset=11
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Lfunc_begin0
+; CHECK-NEXT:        .long   Lfunc_begin0
 ; CHECK-NEXT:        .long   3
 ; CHECK-NEXT:        .long   16                      # LineInfo
 ; CHECK-NEXT:        .long   11                      # LineInfo section string offset=11
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   17
 ; CHECK-NEXT:        .long   0
 ; CHECK-NEXT:        .long   1042                    # Line 1 Col 18
diff --git a/llvm/test/CodeGen/BPF/BTF/func-void.ll b/llvm/test/CodeGen/BPF/BTF/func-void.ll
index 94770dba455e0..c4638533dfcfd 100644
--- a/llvm/test/CodeGen/BPF/BTF/func-void.ll
+++ b/llvm/test/CodeGen/BPF/BTF/func-void.ll
@@ -47,12 +47,12 @@ define dso_local void @f1() local_unnamed_addr #0 !dbg !7 {
 ; CHECK-NEXT:        .long   8                       # FuncInfo
 ; CHECK-NEXT:        .long   4                       # FuncInfo section string offset=4
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Lfunc_begin0
+; CHECK-NEXT:        .long   Lfunc_begin0
 ; CHECK-NEXT:        .long   2
 ; CHECK-NEXT:        .long   16                      # LineInfo
 ; CHECK-NEXT:        .long   4                       # LineInfo section string offset=4
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   10
 ; CHECK-NEXT:        .long   0
 ; CHECK-NEXT:        .long   1040                    # Line 1 Col 16
diff --git a/llvm/test/CodeGen/BPF/CORE/anon-struct-argument-pragma.ll b/llvm/test/CodeGen/BPF/CORE/anon-struct-argument-pragma.ll
index 46507e3338211..bf12d91267ace 100644
--- a/llvm/test/CodeGen/BPF/CORE/anon-struct-argument-pragma.ll
+++ b/llvm/test/CodeGen/BPF/CORE/anon-struct-argument-pragma.ll
@@ -46,7 +46,7 @@ entry:
 ; CHECK:             .long   16                              # FieldReloc
 ; CHECK-NEXT:        .long   59                              # Field reloc section string offset=59
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Ltmp[[#]]
+; CHECK-NEXT:        .long   Ltmp[[#]]
 ; CHECK-NEXT:        .long   2
 ; CHECK-NEXT:        .long   65
 ; CHECK-NEXT:        .long   0
diff --git a/llvm/test/CodeGen/BPF/CORE/anon-union-localvar-attr.ll b/llvm/test/CodeGen/BPF/CORE/anon-union-localvar-attr.ll
index dd8c86897275e..7df73114b2fc8 100644
--- a/llvm/test/CodeGen/BPF/CORE/anon-union-localvar-attr.ll
+++ b/llvm/test/CodeGen/BPF/CORE/anon-union-localvar-attr.ll
@@ -47,7 +47,7 @@ entry:
 ; CHECK:             .long   16                              # FieldReloc
 ; CHECK-NEXT:        .long   10                              # Field reloc section string offset=10
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Ltmp[[#]]
+; CHECK-NEXT:        .long   Ltmp[[#]]
 ; CHECK-NEXT:        .long   7
 ; CHECK-NEXT:        .long   101
 ; CHECK-NEXT:        .long   1
diff --git a/llvm/test/CodeGen/BPF/CORE/anon-union-localvar-pragma.ll b/llvm/test/CodeGen/BPF/CORE/anon-union-localvar-pragma.ll
index d2fee10d27710..11ae69dc3d1b0 100644
--- a/llvm/test/CodeGen/BPF/CORE/anon-union-localvar-pragma.ll
+++ b/llvm/test/CodeGen/BPF/CORE/anon-union-localvar-pragma.ll
@@ -50,7 +50,7 @@ entry:
 ; CHECK:             .long   16                              # FieldReloc
 ; CHECK-NEXT:        .long   10                              # Field reloc section string offset=10
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Ltmp[[#]]
+; CHECK-NEXT:        .long   Ltmp[[#]]
 ; CHECK-NEXT:        .long   7
 ; CHECK-NEXT:        .long   101
 ; CHECK-NEXT:        .long   1
diff --git a/llvm/test/CodeGen/BPF/CORE/btf-id-duplicate.ll b/llvm/test/CodeGen/BPF/CORE/btf-id-duplicate.ll
index 23a461752e1b8..c23cab4ba8850 100644
--- a/llvm/test/CodeGen/BPF/CORE/btf-id-duplicate.ll
+++ b/llvm/test/CodeGen/BPF/CORE/btf-id-duplicate.ll
@@ -49,11 +49,11 @@ entry:
 ; CHECK:             .long   16                              # FieldReloc
 ; CHECK-NEXT:        .long   20                              # Field reloc section string offset=20
 ; CHECK-NEXT:        .long   2
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   2
 ; CHECK-NEXT:        .long   26
 ; CHECK-NEXT:        .long   6
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   2
 ; CHECK-NEXT:        .long   26
 ; CHECK-NEXT:        .long   6
diff --git a/llvm/test/CodeGen/BPF/CORE/field-reloc-alu32.ll b/llvm/test/CodeGen/BPF/CORE/field-reloc-alu32.ll
index 40a2432d67850..43f494e10bc81 100644
--- a/llvm/test/CodeGen/BPF/CORE/field-reloc-alu32.ll
+++ b/llvm/test/CodeGen/BPF/CORE/field-reloc-alu32.ll
@@ -34,7 +34,7 @@ entry:
 ; CHECK:             .long   16                      # FieldReloc
 ; CHECK-NEXT:        .long   7                       # Field reloc section string offset=7
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   4
 ; CHECK-NEXT:        .long   19
 ; CHECK-NEXT:        .long   0
diff --git a/llvm/test/CodeGen/BPF/CORE/field-reloc-bitfield-1-bpfeb.ll b/llvm/test/CodeGen/BPF/CORE/field-reloc-bitfield-1-bpfeb.ll
index 63a0945edf152..fb5c1db0080c3 100644
--- a/llvm/test/CodeGen/BPF/CORE/field-reloc-bitfield-1-bpfeb.ll
+++ b/llvm/test/CodeGen/BPF/CORE/field-reloc-bitfield-1-bpfeb.ll
@@ -27,67 +27,67 @@ target triple = "bpfeb"
 ; Function Attrs: nounwind readnone
 define dso_local i32 @test(ptr %arg) local_unnamed_addr #0 !dbg !13 {
 ; CHECK-ALU64-LABEL: test:
-; CHECK-ALU64:       .Ltest$local:
-; CHECK-ALU64-NEXT:    .type .Ltest$local,@function
-; CHECK-ALU64-NEXT:  .Lfunc_begin0:
+; CHECK-ALU64:       Ltest$local:
+; CHECK-ALU64-NEXT:    .type Ltest$local,@function
+; CHECK-ALU64-NEXT:  Lfunc_begin0:
 ; CHECK-ALU64-NEXT:    .loc 1 11 0 # test.c:11:0
 ; CHECK-ALU64-NEXT:    .cfi_sections .debug_frame
 ; CHECK-ALU64-NEXT:    .cfi_startproc
 ; CHECK-ALU64-NEXT:  # %bb.0: # %entry
 ; CHECK-ALU64-NEXT:    #DEBUG_VALUE: test:arg <- $r1
-; CHECK-ALU64-NEXT:  .Ltmp0:
+; CHECK-ALU64-NEXT:  Ltmp0:
 ; CHECK-ALU64-NEXT:    r1 = 16
-; CHECK-ALU64-NEXT:  .Ltmp1:
-; CHECK-ALU64-NEXT:  .Ltmp2:
-; CHECK-ALU64-NEXT:  .Ltmp3:
+; CHECK-ALU64-NEXT:  Ltmp1:
+; CHECK-ALU64-NEXT:  Ltmp2:
+; CHECK-ALU64-NEXT:  Ltmp3:
 ; CHECK-ALU64-NEXT:    r0 = 8
-; CHECK-ALU64-NEXT:  .Ltmp4:
+; CHECK-ALU64-NEXT:  Ltmp4:
 ; CHECK-ALU64-NEXT:    .loc 1 12 69 prologue_end # test.c:12:69
-; CHECK-ALU64-NEXT:  .Ltmp5:
-; CHECK-ALU64-NEXT:  .Ltmp6:
+; CHECK-ALU64-NEXT:  Ltmp5:
+; CHECK-ALU64-NEXT:  Ltmp6:
 ; CHECK-ALU64-NEXT:    r0 += r1
-; CHECK-ALU64-NEXT:  .Ltmp7:
+; CHECK-ALU64-NEXT:  Ltmp7:
 ; CHECK-ALU64-NEXT:    r1 = 45
 ; CHECK-ALU64-NEXT:    .loc 1 13 67 # test.c:13:67
-; CHECK-ALU64-NEXT:  .Ltmp8:
+; CHECK-ALU64-NEXT:  Ltmp8:
 ; CHECK-ALU64-NEXT:    r0 += r1
 ; CHECK-ALU64-NEXT:    .loc 1 12 3 # test.c:12:3
-; CHECK-ALU64-NEXT:  .Ltmp9:
+; CHECK-ALU64-NEXT:  Ltmp9:
 ; CHECK-ALU64-NEXT:    exit
-; CHECK-ALU64-NEXT:  .Ltmp10:
-; CHECK-ALU64-NEXT:  .Ltmp11:
+; CHECK-ALU64-NEXT:  Ltmp10:
+; CHECK-ALU64-NEXT:  Ltmp11:
 ;
 ; CHECK-ALU32-LABEL: test:
-; CHECK-ALU32:       .Ltest$local:
-; CHECK-ALU32-NEXT:    .type .Ltest$local,@function
-; CHECK-ALU32-NEXT:  .Lfunc_begin0:
+; CHECK-ALU32:       Ltest$local:
+; CHECK-ALU32-NEXT:    .type Ltest$local,@function
+; CHECK-ALU32-NEXT:  Lfunc_begin0:
 ; CHECK-ALU32-NEXT:    .loc 1 11 0 # test.c:11:0
 ; CHECK-ALU32-NEXT:    .cfi_sections .debug_frame
 ; CHECK-ALU32-NEXT:    .cfi_startproc
 ; CHECK-ALU32-NEXT:  # %bb.0: # %entry
 ; CHECK-ALU32-NEXT:    #DEBUG_VALUE: test:arg <- $r1
-; CHECK-ALU32-NEXT:  .Ltmp0:
+; CHECK-ALU32-NEXT:  Ltmp0:
 ; CHECK-ALU32-NEXT:    r1 = 16
-; CHECK-ALU32-NEXT:  .Ltmp1:
-; CHECK-ALU32-NEXT:  .Ltmp2:
-; CHECK-ALU32-NEXT:  .Ltmp3:
+; CHECK-ALU32-NEXT:  Ltmp1:
+; CHECK-ALU32-NEXT:  Ltmp2:
+; CHECK-ALU32-NEXT:  Ltmp3:
 ; CHECK-ALU32-NEXT:    r0 = 8
-; CHECK-ALU32-NEXT:  .Ltmp4:
+; CHECK-ALU32-NEXT:  Ltmp4:
 ; CHECK-ALU32-NEXT:    .loc 1 12 69 prologue_end # test.c:12:69
-; CHECK-ALU32-NEXT:  .Ltmp5:
-; CHECK-ALU32-NEXT:  .Ltmp6:
+; CHECK-ALU32-NEXT:  Ltmp5:
+; CHECK-ALU32-NEXT:  Ltmp6:
 ; CHECK-ALU32-NEXT:    w0 += w1
-; CHECK-ALU32-NEXT:  .Ltmp7:
+; CHECK-ALU32-NEXT:  Ltmp7:
 ; CHECK-ALU32-NEXT:    r1 = 45
 ; CHECK-ALU32-NEXT:    .loc 1 13 67 # test.c:13:67
-; CHECK-ALU32-NEXT:  .Ltmp8:
+; CHECK-ALU32-NEXT:  Ltmp8:
 ; CHECK-ALU32-NEXT:    w0 += w1
 ; CHECK-ALU32-NEXT:    # kill: def $w0 killed $w0 killed $r0
 ; CHECK-ALU32-NEXT:    .loc 1 12 3 # test.c:12:3
-; CHECK-ALU32-NEXT:  .Ltmp9:
+; CHECK-ALU32-NEXT:  Ltmp9:
 ; CHECK-ALU32-NEXT:    exit
-; CHECK-ALU32-NEXT:  .Ltmp10:
-; CHECK-ALU32-NEXT:  .Ltmp11:
+; CHECK-ALU32-NEXT:  Ltmp10:
+; CHECK-ALU32-NEXT:  Ltmp11:
 entry:
   call void @llvm.dbg.value(metadata ptr %arg, metadata !30, metadata !DIExpression()), !dbg !31
   %0 = tail call ptr @llvm.preserve.struct.access.index.p0.p0.ss(ptr elementtype(%struct.s) %arg, i32 5, i32 6), !dbg !32, !llvm.preserve.access.index !18
@@ -108,15 +108,15 @@ entry:
 ; CHECK:             .long   16                      # FieldReloc
 ; CHECK-NEXT:        .long   89                      # Field reloc section string offset=89
 ; CHECK-NEXT:        .long   3
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   2
 ; CHECK-NEXT:        .long   95
 ; CHECK-NEXT:        .long   0
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   2
 ; CHECK-NEXT:        .long   95
 ; CHECK-NEXT:        .long   1
-; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   Ltmp{{[0-9]+}}
 ; CHECK-NEXT:        .long   2
 ; CHECK-NEXT:        .long   95
 ; CHECK-NEXT:        .long   4
diff --git a/llvm/test/CodeGen/BPF/CORE/field-reloc-bitfield-1.ll b/llvm/test/CodeGen/BPF/CORE/field-reloc-bitfield-1.ll
index 33462198eab15..b88cd9223006a 100644
--- a/llvm/test/CodeGen/BPF/CORE/field-reloc-bitfield-1.ll
+++ b/llvm/test/CodeGen/BPF/CORE/field-reloc-bitfield-1.ll
@@ -27,67 +27,67 @@ target triple = "bpfel"
 ; Function Attrs: nounwind readnone
 define dso_local i32 @test(ptr %arg) local_unnamed_addr #0 !dbg !13 {
 ; CHECK-ALU64-LABEL: test:
-; CHECK-ALU64:       .Ltest$local:
-; CHECK-ALU64-NEXT:    .type .Ltest$local,@function
-; CHECK-ALU64-NEXT:  .Lfunc_begin0:
+; CHECK-ALU64:       Ltest$local:
+; CHECK-ALU64-NEXT:    .type Ltest$local,@function
+; CHECK-ALU64-NEXT:  Lfunc_begin0:
 ; CHECK-ALU64-NEXT:    .loc 1 11 0 # test.c:11:0
 ; CHECK-ALU64-NEXT:    .cfi_sections .debug_frame
 ; CHECK-ALU64-NEXT:    .cfi_startproc
 ; CHECK-ALU64-NEXT:  # %bb.0: # %entry
 ;...
[truncated]

aengelke · 2024-06-11T19:57:30Z

PrivateGlobalPrefix = PrivateLabelPrefix = "L" is certainly better.

I now changed the prefix so that for BPF all local labels start with "L" without dot.

aengelke · 2024-06-12T09:29:09Z

The "quick fix" broke AMDGPU, so I had to add a new BPF mangling mode that uses "L" for private labels.

4ast · 2024-06-12T14:49:02Z

The "quick fix" broke AMDGPU, so I had to add a new BPF mangling mode that uses "L" for private labels.

wait what ?
Why change ManglingMode ? This doesn't look right. It's still an ELF.
It's really puzzling why default "L" prefix needs this special logic.

@eddyz87 @yonghong-song pls take a look.

MaskRay · 2024-06-12T22:02:57Z

While I'm not involved with BPF development, I favor using PrivateGlobalPrefix = PrivateLabelPrefix = ".L" like regular ELF targets.
This is better from an assembler's perspective: .L prevents name collision with user-defined symbols.
Object file formats that use L typically decorate regular symbols with _ to avoid name collision, which causes uglier names.

If .L symbols in the symbol table are useful, users can specify -Wa,-L.

4ast · 2024-06-13T01:31:57Z

I'm starting to lean toward ".L" as well, but first would like to understand why "L" is causing issues.

MaskRay · 2024-06-24T04:30:58Z

Ping:)

MaskRay · 2024-07-10T05:57:30Z

Ping:)

4ast

lgtm. sorry for delay.

yonghong-song · 2024-07-24T18:39:09Z

Okay, looks like consensus is to use '.L' prefix for private global prefix and private label prefix. @aengelke could you make the change again? The below should work:

diff --git a/llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h b/llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h
index 7b2168458c93..1173fe2f7590 100644
--- a/llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h
+++ b/llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h
@@ -25,6 +25,7 @@ public:
       IsLittleEndian = false;
 
     PrivateGlobalPrefix = ".L";
+    PrivateLabelPrefix = ".L";
     WeakRefDirective = "\t.weak\t";

4ast · 2024-07-24T19:04:54Z

Okay, looks like consensus is to use '.L' prefix for private global prefix and private label prefix. @aengelke could you make the change again? The below should work:

diff --git a/llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h b/llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h
index 7b2168458c93..1173fe2f7590 100644
--- a/llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h
+++ b/llvm/lib/Target/BPF/MCTargetDesc/BPFMCAsmInfo.h
@@ -25,6 +25,7 @@ public:
       IsLittleEndian = false;
 
     PrivateGlobalPrefix = ".L";
+    PrivateLabelPrefix = ".L";
     WeakRefDirective = "\t.weak\t";

No. The current diff changed things to work properly with "L".

yonghong-song · 2024-07-27T01:03:14Z

Just want to confirm. @4ast You perfer to use prefix 'L' for all temp labels?

4ast · 2024-07-27T02:42:46Z

Just want to confirm. @4ast You perfer to use prefix 'L' for all temp labels?

Yes. As this patch implements it. The issue we had is that those L labels were in symbol table. This patch is fixing that.

yonghong-song · 2024-07-27T04:09:28Z

@aengelke This patch does not really work. I downloaded aengelke:bpf-private-label-prefix and build the clang/llvm tools. I hit the following errors:

$ cat test.c
int test(int a, int b, int *p1, int *p2) {
   if (a < b) *p1 = a * b;
   else if (a > b) *p2 = a * b * a;
   return a * a;
}
$ ~/tmp3/llvm-project/llvm/build/install/bin/clang --target=bpf -O2 -c -g test.c
error: backend data layout 'e-m:b-p:64:64-i64:64-i128:128-n32:64-S128' does not match expected target description 'e-m:e-p:64:64-i64:64-i128:128-n32:64-S128'
1 error generated.
$

I then applied the following change,

diff --git a/clang/lib/Basic/Targets/BPF.h b/clang/lib/Basic/Targets/BPF.h
index d19b37dd4df7..00909afb8093 100644
--- a/clang/lib/Basic/Targets/BPF.h
+++ b/clang/lib/Basic/Targets/BPF.h
@@ -35,9 +35,9 @@ public:
     Int64Type = SignedLong;
     RegParmMax = 5;
     if (Triple.getArch() == llvm::Triple::bpfeb) {
-      resetDataLayout("E-m:e-p:64:64-i64:64-i128:128-n32:64-S128");
+      resetDataLayout("E-m:b-p:64:64-i64:64-i128:128-n32:64-S128");
     } else {
-      resetDataLayout("e-m:e-p:64:64-i64:64-i128:128-n32:64-S128");
+      resetDataLayout("e-m:b-p:64:64-i64:64-i128:128-n32:64-S128");
     }
     MaxAtomicPromoteWidth = 64;
     MaxAtomicInlineWidth = 64;

and compilation can now pass. However, I didn't see L* lables in the symbol table:

$ ~/tmp3/llvm-project/llvm/build/install/bin/clang --target=bpf -O2 -c -g test.c
$ ~/tmp3/llvm-project/llvm/build/install/bin/llvm-readelf -s test.o

Symbol table '.symtab' contains 12 entries:
   Num:    Value          Size Type    Bind   Vis       Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT   UND 
     1: 0000000000000000     0 FILE    LOCAL  DEFAULT   ABS test.c
     2: 0000000000000000     0 SECTION LOCAL  DEFAULT     2 .text
     3: 0000000000000000     0 SECTION LOCAL  DEFAULT     3 .debug_loclists
     4: 0000000000000000     0 SECTION LOCAL  DEFAULT     4 .debug_abbrev
     5: 0000000000000000     0 SECTION LOCAL  DEFAULT     7 .debug_str_offsets
     6: 0000000000000000     0 SECTION LOCAL  DEFAULT     9 .debug_str
     7: 0000000000000000     0 SECTION LOCAL  DEFAULT    10 .debug_addr
     8: 0000000000000000     0 SECTION LOCAL  DEFAULT    15 .debug_frame
     9: 0000000000000000     0 SECTION LOCAL  DEFAULT    17 .debug_line
    10: 0000000000000000     0 SECTION LOCAL  DEFAULT    19 .debug_line_str
    11: 0000000000000000   136 FUNC    GLOBAL DEFAULT     2 test
$ ~/tmp3/llvm-project/llvm/build/install/bin/llvm-objdump -d test.o

test.o: file format elf64-bpf

Disassembly of section .text:

0000000000000000 <test>:
       0:       bf 25 00 00 00 00 00 00 r5 = r2
       1:       67 05 00 00 20 00 00 00 r5 <<= 0x20
       2:       c7 05 00 00 20 00 00 00 r5 s>>= 0x20
       3:       bf 16 00 00 00 00 00 00 r6 = r1
       4:       67 06 00 00 20 00 00 00 r6 <<= 0x20
       5:       c7 06 00 00 20 00 00 00 r6 s>>= 0x20
       6:       bf 10 00 00 00 00 00 00 r0 = r1
       7:       2f 00 00 00 00 00 00 00 r0 *= r0
       8:       7d 56 03 00 00 00 00 00 if r6 s>= r5 goto +0x3 <test+0x60>
       9:       2f 12 00 00 00 00 00 00 r2 *= r1
      10:       63 23 00 00 00 00 00 00 *(u32 *)(r3 + 0x0) = r2
      11:       05 00 04 00 00 00 00 00 goto +0x4 <test+0x80>
      12:       7d 65 03 00 00 00 00 00 if r5 s>= r6 goto +0x3 <test+0x80>
      13:       bf 01 00 00 00 00 00 00 r1 = r0
      14:       2f 21 00 00 00 00 00 00 r1 *= r2
      15:       63 14 00 00 00 00 00 00 *(u32 *)(r4 + 0x0) = r1
      16:       95 00 00 00 00 00 00 00 exit

If I add '-Wa,-L' in the command line below, the L* labels will be in symbol
and those labels are in llvm-objdump as well.

$ ~/tmp3/llvm-project/llvm/build/install/bin/clang --target=bpf -O2 -c -g test.c -Wa,-L                                     
$ ~/tmp3/llvm-project/llvm/build/install/bin/llvm-readelf -s test.o                                                         
                                                                                                                                                       
Symbol table '.symtab' contains 15 entries:                                                                                                            
   Num:    Value          Size Type    Bind   Vis       Ndx Name                                                                                       
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT   UND                                                                                            
     1: 0000000000000000     0 FILE    LOCAL  DEFAULT   ABS test.c                                                                                     
     2: 0000000000000000     0 SECTION LOCAL  DEFAULT     2 .text                                                                                      
     3: 0000000000000060     0 NOTYPE  LOCAL  DEFAULT     2 LBB0_2                                                                                     
     4: 0000000000000080     0 NOTYPE  LOCAL  DEFAULT     2 LBB0_4                                                                                     
     5: 0000000000000000     0 SECTION LOCAL  DEFAULT     3 .debug_loclists                                                                            
     6: 0000000000000000     0 SECTION LOCAL  DEFAULT     4 .debug_abbrev
     7: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT    17 Lline_table_start0
     8: 0000000000000000     0 SECTION LOCAL  DEFAULT     7 .debug_str_offsets
     9: 0000000000000000     0 SECTION LOCAL  DEFAULT     9 .debug_str
    10: 0000000000000000     0 SECTION LOCAL  DEFAULT    10 .debug_addr
    11: 0000000000000000     0 SECTION LOCAL  DEFAULT    15 .debug_frame
    12: 0000000000000000     0 SECTION LOCAL  DEFAULT    17 .debug_line
    13: 0000000000000000     0 SECTION LOCAL  DEFAULT    19 .debug_line_str 
    14: 0000000000000000   136 FUNC    GLOBAL DEFAULT     2 test
$ ~/tmp3/llvm-project/llvm/build/install/bin/llvm-objdump -d test.o

test.o: file format elf64-bpf

Disassembly of section .text:

0000000000000000 <test>:
       0:       bf 25 00 00 00 00 00 00 r5 = r2
       1:       67 05 00 00 20 00 00 00 r5 <<= 0x20
       2:       c7 05 00 00 20 00 00 00 r5 s>>= 0x20
       3:       bf 16 00 00 00 00 00 00 r6 = r1
       4:       67 06 00 00 20 00 00 00 r6 <<= 0x20
       5:       c7 06 00 00 20 00 00 00 r6 s>>= 0x20
       6:       bf 10 00 00 00 00 00 00 r0 = r1
       7:       2f 00 00 00 00 00 00 00 r0 *= r0
       8:       7d 56 03 00 00 00 00 00 if r6 s>= r5 goto +0x3 <LBB0_2>
       9:       2f 12 00 00 00 00 00 00 r2 *= r1
      10:       63 23 00 00 00 00 00 00 *(u32 *)(r3 + 0x0) = r2
      11:       05 00 04 00 00 00 00 00 goto +0x4 <LBB0_4>

0000000000000060 <LBB0_2>:
      12:       7d 65 03 00 00 00 00 00 if r5 s>= r6 goto +0x3 <LBB0_4>
      13:       bf 01 00 00 00 00 00 00 r1 = r0
      14:       2f 21 00 00 00 00 00 00 r1 *= r2
      15:       63 14 00 00 00 00 00 00 *(u32 *)(r4 + 0x0) = r1

0000000000000080 <LBB0_4>:
      16:       95 00 00 00 00 00 00 00 exit

So to ensure L* labels in symbol table, one way is to add -Wa,-L in compilation flags. But that requires additional compilation options. So maybe we can find some llvm internal setting which allows bpf target to overwrite in order to enable L* labels in symbol table. I have not poked into llvm internal yet.

4ast · 2024-07-27T04:22:24Z

@yonghong-song we're talking past each other. L labels should NOT be in the symbol table. They are private. We had a bug before.

yonghong-song · 2024-07-27T04:39:41Z

Okay, looks like current llvm19/llvm20 already do not have L* labels in symbol table.
@eddyz87 has another pull request (#100550) to address llvm-objdump without-label issue. There are a couple of options to resolve this. Since we do not want L* in the symbol table,
then the only option may be tweak llvm-objdump.

MaskRay · 2024-07-27T05:09:51Z

Okay, looks like current llvm19/llvm20 already do not have L* labels in symbol table. @eddyz87 has another pull request (#100550) to address llvm-objdump without-label issue. There are a couple of options to resolve this. Since we do not want L* in the symbol table, then the only option may be tweak llvm-objdump.

#100550 is for llvm-objdump --symbolize-operands, which creates fake labels for the disassembly output.

For this patch, I hope we use .L to be similar to other ELF architectures and not to create a special case:)

4ast · 2024-07-27T15:58:45Z

Okay, looks like current llvm19/llvm20 already do not have L* labels in symbol table. @eddyz87 has another pull request (#100550) to address llvm-objdump without-label issue. There are a couple of options to resolve this. Since we do not want L* in the symbol table, then the only option may be tweak llvm-objdump.

#100550 is for llvm-objdump --symbolize-operands, which creates fake labels for the disassembly output.

For this patch, I hope we use .L to be similar to other ELF architectures and not to create a special case:)

are you saying that if we use ".L" then private labels will automatically appear in disasm output and we don't need this extra flag?
If so let's switch to ".L".

MaskRay · 2024-07-27T18:18:49Z

Okay, looks like current llvm19/llvm20 already do not have L* labels in symbol table. @eddyz87 has another pull request (#100550) to address llvm-objdump without-label issue. There are a couple of options to resolve this. Since we do not want L* in the symbol table, then the only option may be tweak llvm-objdump.

#100550 is for llvm-objdump --symbolize-operands, which creates fake labels for the disassembly output.
For this patch, I hope we use .L to be similar to other ELF architectures and not to create a special case:)

are you saying that if we use ".L" then private labels will automatically appear in disasm output and we don't need this extra flag? If so let's switch to ".L".

llvm-objdump -d --symbolize-operands is a way to improve readability of the disassembly, which doesn't need symbol table entries.
With that feature, basic block labels are likely less relevant. In that case, we should favor more .L to match regular ELF targets.

There is a question whether --symbolize-operands should be the default for llvm-objdump BPF. While I don't favor that inconsistency, the discussion can be postponed to a difference place and not block the PR that switches to .L

4ast · 2024-07-27T23:30:50Z

llvm-objdump -d --symbolize-operands is a way to improve readability of the disassembly, which doesn't need symbol table entries. With that feature, basic block labels are likely less relevant. In that case, we should favor more .L to match regular ELF targets.

There is a question whether --symbolize-operands should be the default for llvm-objdump BPF. While I don't favor that inconsistency, the discussion can be postponed to a difference place and not block the PR that switches to .L

well, this two PRs are related. Currently bpf users used to see L* labels in disasm and we have to preserve this behavior.
Turned out that they were in disasm by default because of bug/feature that L* labels were in symbol table.
Now if we remove them from symbol table either via this patch or via switching to ".L" we will regress disasm output, right?
I was hoping that switch to ".L" will make labels to appear in disasm automatically without extra flag.
If that is not the case we should treat both PRs as one and make sure disasm is still readable regardless of whether we pick L or .L

yonghong-song · 2024-07-28T00:12:22Z

@4ast For this ' I was hoping that switch to ".L" will make labels to appear in disasm automatically without extra flag.', the switch to '.L' won't change anything to disasm since with '.L' the labels still not in symbol table.

#100550 is the right direction as at least with option --symbolize-operands, the disasm will break basic blocks with labels properly. Without #100550, even with --symbolize-operands, the disasm will display without any labels.

But at you mentioned, this is a regression. Previous, 'llvm-objdump -d ...' will be able to disasm with proper labels. Now we need 'llvm-objdump --symbolize-operands -d ...'. That is why I suggest to make '--symbolize-operands' as the default at least for BPF. An option '--no-symbolize-operands' can be added to disable labels.

4ast

We need to keep L symbols in symbol table to preserve current disasm behavior.
Once --symbolize-operands becomes a default for objdump we can revisit this diff.

[BPF] Use local prefix label for basic block

a0446a3

Previously, PrivateLabelPrefix was default-initialized to "L", so basic block labels were added to the symbol table. This seems like an oversight, so use ".L" for all private labels.

aengelke requested review from eddyz87, 4ast and yonghong-song June 11, 2024 12:30

4ast requested changes Jun 11, 2024

View reviewed changes

Change private global to "L" (the default of MCAsmInfo)

f4545a7

llvmbot added mc Machine (object) code llvm:globalisel labels Jun 11, 2024

Add BPF mangling mode that uses "L" prefix

0f72c6d

llvmbot added llvm:ir llvm:binary-utilities labels Jun 12, 2024

MaskRay mentioned this pull request Jun 12, 2024

[CodeGen] Use temp symbol for MBBs #95031

Merged

4ast approved these changes Jul 10, 2024

View reviewed changes

eddyz87 mentioned this pull request Jul 28, 2024

[llvm-objdump][BPF] --symbolize-operands: infer local labels for BPF #100550

Merged

4ast requested changes Jul 31, 2024

View reviewed changes

MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Aug 23, 2024

WIP remove getPrivateLabelPrefix after llvm#95103

72d193c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BPF] Use ".L" local prefix label for basic blocks #95103

[BPF] Use ".L" local prefix label for basic blocks #95103

aengelke commented Jun 11, 2024

4ast left a comment

aengelke commented Jun 11, 2024

4ast commented Jun 11, 2024

llvmbot commented Jun 11, 2024 •

edited

Loading

aengelke commented Jun 11, 2024

aengelke commented Jun 12, 2024

4ast commented Jun 12, 2024

MaskRay commented Jun 12, 2024 •

edited

Loading

4ast commented Jun 13, 2024

MaskRay commented Jun 24, 2024

MaskRay commented Jul 10, 2024

4ast left a comment

yonghong-song commented Jul 24, 2024

4ast commented Jul 24, 2024

yonghong-song commented Jul 27, 2024

4ast commented Jul 27, 2024

yonghong-song commented Jul 27, 2024 •

edited

Loading

4ast commented Jul 27, 2024

yonghong-song commented Jul 27, 2024

MaskRay commented Jul 27, 2024

4ast commented Jul 27, 2024

MaskRay commented Jul 27, 2024

4ast commented Jul 27, 2024

yonghong-song commented Jul 28, 2024

4ast left a comment

[BPF] Use ".L" local prefix label for basic blocks #95103

Are you sure you want to change the base?

[BPF] Use ".L" local prefix label for basic blocks #95103

Conversation

aengelke commented Jun 11, 2024

4ast left a comment

Choose a reason for hiding this comment

aengelke commented Jun 11, 2024

4ast commented Jun 11, 2024

llvmbot commented Jun 11, 2024 • edited Loading

aengelke commented Jun 11, 2024

aengelke commented Jun 12, 2024

4ast commented Jun 12, 2024

MaskRay commented Jun 12, 2024 • edited Loading

4ast commented Jun 13, 2024

MaskRay commented Jun 24, 2024

MaskRay commented Jul 10, 2024

4ast left a comment

Choose a reason for hiding this comment

yonghong-song commented Jul 24, 2024

4ast commented Jul 24, 2024

yonghong-song commented Jul 27, 2024

4ast commented Jul 27, 2024

yonghong-song commented Jul 27, 2024 • edited Loading

4ast commented Jul 27, 2024

yonghong-song commented Jul 27, 2024

MaskRay commented Jul 27, 2024

4ast commented Jul 27, 2024

MaskRay commented Jul 27, 2024

4ast commented Jul 27, 2024

yonghong-song commented Jul 28, 2024

4ast left a comment

Choose a reason for hiding this comment

llvmbot commented Jun 11, 2024 •

edited

Loading

MaskRay commented Jun 12, 2024 •

edited

Loading

yonghong-song commented Jul 27, 2024 •

edited

Loading