Skip to content

Commit 27026fe

Browse files
eddyz87eddyz87
committed
[BPF] Reset machine register kill mark in BPFMISimplifyPatchable
When LLVM is build with `LLVM_ENABLE_EXPENSIVE_CHECKS=ON` option the following C code snippet: struct t { unsigned long a; } __attribute__((preserve_access_index)); void foo(volatile struct t *t, volatile unsigned long *p) { *p = t->a; *p = t->a; } Causes an assertion: $ clang -g -O2 -c --target=bpf -mcpu=v2 t2.c -o /dev/null # After BPF PreEmit SimplifyPatchable # Machine code for function foo: IsSSA, TracksLiveness Function Live Ins: $r1 in %0, $r2 in %1 bb.0.entry: liveins: $r1, $r2 DBG_VALUE $r1, $noreg, !"t", !DIExpression() DBG_VALUE $r2, $noreg, !"p", !DIExpression() %1:gpr = COPY $r2 DBG_VALUE %1:gpr, $noreg, !"p", !DIExpression() %0:gpr = COPY $r1 DBG_VALUE %0:gpr, $noreg, !"t", !DIExpression() %2:gpr = LD_imm64 @"llvm.t:0:0$0:0" %4:gpr = ADD_rr %0:gpr(tied-def 0), killed %2:gpr %5:gpr = CORE_LD 344, %0:gpr, @"llvm.t:0:0$0:0" STD killed %5:gpr, %1:gpr, 0 %7:gpr = ADD_rr %0:gpr(tied-def 0), killed %2:gpr %8:gpr = CORE_LD 344, %0:gpr, @"llvm.t:0:0$0:0" STD killed %8:gpr, %1:gpr, 0 RET # End machine code for function foo. *** Bad machine code: Using a killed virtual register *** - function: foo - basic block: %bb.0 entry (0x6210000e6690) - instruction: %7:gpr = ADD_rr %0:gpr(tied-def 0), killed %2:gpr - operand 2: killed %2:gpr This happens because of the way BPFMISimplifyPatchable::processDstReg() updates second operand of the `ADD_rr` instruction. Code before `BPFMISimplifyPatchable`: .-> %2:gpr = LD_imm64 @"llvm.t:0:0$0:0" | |`----------------. | %3:gpr = LDD %2:gpr, 0 | %4:gpr = ADD_rr %0:gpr(tied-def 0), killed %3:gpr <--- (1) | %5:gpr = LDD killed %4:gpr, 0 ^^^^^^^^^^^^^ | STD killed %5:gpr, %1:gpr, 0 this is updated `----------------. %6:gpr = LDD %2:gpr, 0 %7:gpr = ADD_rr %0:gpr(tied-def 0), killed %6:gpr <--- (2) %8:gpr = LDD killed %7:gpr, 0 ^^^^^^^^^^^^^ STD killed %8:gpr, %1:gpr, 0 this is updated Instructions (1) and (2) would be updated to: ADD_rr %0:gpr(tied-def 0), killed %2:gpr The `killed` mark is inherited from machine operands `killed %3:gpr` and `killed %6:gpr` which are updated inplace by `processDstReg()`. This commit updates `processDstReg()` reset kill marks for updated machine operands to keep liveness information conservatively correct. Differential Revision: https://reviews.llvm.org/D157805
1 parent 1a135bc commit 27026fe

File tree

2 files changed

+146
-1
lines changed

2 files changed

+146
-1
lines changed

‎llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,32 @@ void BPFMISimplifyPatchable::processDstReg(MachineRegisterInfo *MRI,
209209
decltype(End) NextI;
210210
for (auto I = Begin; I != End; I = NextI) {
211211
NextI = std::next(I);
212-
if (doSrcRegProp)
212+
if (doSrcRegProp) {
213+
// In situations like below it is not known if usage is a kill
214+
// after setReg():
215+
//
216+
// .-> %2:gpr = LD_imm64 @"llvm.t:0:0$0:0"
217+
// |
218+
// |`----------------.
219+
// | %3:gpr = LDD %2:gpr, 0
220+
// | %4:gpr = ADD_rr %0:gpr(tied-def 0), killed %3:gpr <--- (1)
221+
// | %5:gpr = LDD killed %4:gpr, 0 ^^^^^^^^^^^^^
222+
// | STD killed %5:gpr, %1:gpr, 0 this is I
223+
// `----------------.
224+
// %6:gpr = LDD %2:gpr, 0
225+
// %7:gpr = ADD_rr %0:gpr(tied-def 0), killed %6:gpr <--- (2)
226+
// %8:gpr = LDD killed %7:gpr, 0 ^^^^^^^^^^^^^
227+
// STD killed %8:gpr, %1:gpr, 0 this is I
228+
//
229+
// Instructions (1) and (2) would be updated by setReg() to:
230+
//
231+
// ADD_rr %0:gpr(tied-def 0), %2:gpr
232+
//
233+
// %2:gpr is not killed at (1), so it is necessary to remove kill flag
234+
// from I.
213235
I->setReg(SrcReg);
236+
I->setIsKill(false);
237+
}
214238

215239
// The candidate needs to have a unique definition.
216240
if (IsAma && MRI->getUniqueVRegDef(I->getReg()))

‎llvm/test/CodeGen/BPF/CORE/simplify-patchable-liveness-bug.ll

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
; RUN: llc -mtriple=bpf -mcpu=v2 < %s | FileCheck -check-prefixes=CHECK %s
2+
3+
; Test for machine register liveness update bug in
4+
; BPFMISimplifyPatchable::processDstReg.
5+
;
6+
; Generated from the following source code:
7+
; struct t {
8+
; unsigned long a;
9+
; } __attribute__((preserve_access_index));
10+
;
11+
; void foo(volatile struct t *t, volatile unsigned long *p) {
12+
; *p = t->a;
13+
; *p = t->a;
14+
; }
15+
;
16+
; Using the following command:
17+
; clang -g -O2 -S -emit-llvm --target=bpf t.c -o t.ll
18+
19+
@"llvm.t:0:0$0:0" = external global i64, !llvm.preserve.access.index !0 #0
20+
21+
; Function Attrs: nofree nounwind
22+
define dso_local void @foo(ptr noundef %t, ptr noundef %p) local_unnamed_addr #1 !dbg !12 {
23+
entry:
24+
call void @llvm.dbg.value(metadata ptr %t, metadata !20, metadata !DIExpression()), !dbg !22
25+
call void @llvm.dbg.value(metadata ptr %p, metadata !21, metadata !DIExpression()), !dbg !22
26+
%0 = load i64, ptr @"llvm.t:0:0$0:0", align 8
27+
%1 = getelementptr i8, ptr %t, i64 %0
28+
%2 = tail call ptr @llvm.bpf.passthrough.p0.p0(i32 0, ptr %1)
29+
%3 = load volatile i64, ptr %2, align 8, !dbg !23, !tbaa !24
30+
store volatile i64 %3, ptr %p, align 8, !dbg !29, !tbaa !30
31+
%4 = load i64, ptr @"llvm.t:0:0$0:0", align 8
32+
%5 = getelementptr i8, ptr %t, i64 %4
33+
%6 = tail call ptr @llvm.bpf.passthrough.p0.p0(i32 1, ptr %5)
34+
%7 = load volatile i64, ptr %6, align 8, !dbg !31, !tbaa !24
35+
store volatile i64 %7, ptr %p, align 8, !dbg !32, !tbaa !30
36+
ret void, !dbg !33
37+
}
38+
39+
; CHECK: foo:
40+
; CHECK: prologue_end
41+
; CHECK-NEXT: .Ltmp[[LABEL1:.*]]:
42+
; CHECK-NEXT: .Ltmp
43+
; CHECK-NEXT: r[[#A:]] = *(u64 *)(r1 + 0)
44+
; CHECK-NEXT: .loc
45+
; CHECK-NEXT: .Ltmp
46+
; CHECK-NEXT: *(u64 *)(r2 + 0) = r[[#A]]
47+
; CHECK-NEXT: .loc
48+
; CHECK-NEXT: .Ltmp[[LABEL2:.*]]:
49+
; CHECK-NEXT: .Ltmp
50+
; CHECK-NEXT: r[[#B:]] = *(u64 *)(r1 + 0)
51+
; CHECK-NEXT: .Ltmp
52+
; CHECK-NEXT: .Ltmp
53+
; CHECK-NEXT: .loc
54+
; CHECK-NEXT: .Ltmp
55+
; CHECK-NEXT: *(u64 *)(r2 + 0) = r[[#B]]
56+
57+
; CHECK: .section .BTF
58+
; CHECK: .long [[STR_T:.*]] # BTF_KIND_STRUCT(id = [[ID:.*]])
59+
60+
; CHECK: .byte 116 # string offset=[[STR_T]]
61+
; CHECK: .ascii "0:0" # string offset=[[STR_A:.*]]
62+
63+
; CHECK: # FieldReloc
64+
; CHECK: .long .Ltmp[[LABEL1]]
65+
; CHECK-NEXT: .long [[ID]]
66+
; CHECK-NEXT: .long [[STR_A]]
67+
; CHECK-NEXT: .long 0
68+
; CHECK: .long .Ltmp[[LABEL2]]
69+
; CHECK-NEXT: .long [[ID]]
70+
; CHECK-NEXT: .long [[STR_A]]
71+
; CHECK-NEXT: .long 0
72+
73+
; Function Attrs: nofree nosync nounwind memory(none)
74+
declare ptr @llvm.bpf.passthrough.p0.p0(i32, ptr) #2
75+
76+
; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
77+
declare void @llvm.dbg.value(metadata, metadata, metadata) #3
78+
79+
attributes #0 = { "btf_ama" }
80+
attributes #1 = { nofree nounwind "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
81+
attributes #2 = { nofree nosync nounwind memory(none) }
82+
attributes #3 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
83+
84+
!llvm.dbg.cu = !{!5}
85+
!llvm.module.flags = !{!6, !7, !8, !9, !10}
86+
!llvm.ident = !{!11}
87+
88+
!0 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "t", file: !1, line: 1, size: 64, elements: !2)
89+
!1 = !DIFile(filename: "some.file", directory: "/some/dir", checksumkind: CSK_MD5, checksum: "a149cfaf65a83125e7f2b2f47e5c7287")
90+
!2 = !{!3}
91+
!3 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !0, file: !1, line: 2, baseType: !4, size: 64)
92+
!4 = !DIBasicType(name: "unsigned long", size: 64, encoding: DW_ATE_unsigned)
93+
!5 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 18.0.0 (/home/eddy/work/llvm-project/clang 3810f2eb4382d5e2090ce5cd47f45379cb453c35)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
94+
!6 = !{i32 7, !"Dwarf Version", i32 5}
95+
!7 = !{i32 2, !"Debug Info Version", i32 3}
96+
!8 = !{i32 1, !"wchar_size", i32 4}
97+
!9 = !{i32 7, !"frame-pointer", i32 2}
98+
!10 = !{i32 7, !"debug-info-assignment-tracking", i1 true}
99+
!11 = !{!"clang version 18.0.0 (/home/eddy/work/llvm-project/clang 3810f2eb4382d5e2090ce5cd47f45379cb453c35)"}
100+
!12 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 4, type: !13, scopeLine: 4, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !5, retainedNodes: !19)
101+
!13 = !DISubroutineType(types: !14)
102+
!14 = !{null, !15, !17}
103+
!15 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !16, size: 64)
104+
!16 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !0)
105+
!17 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !18, size: 64)
106+
!18 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !4)
107+
!19 = !{!20, !21}
108+
!20 = !DILocalVariable(name: "t", arg: 1, scope: !12, file: !1, line: 4, type: !15)
109+
!21 = !DILocalVariable(name: "p", arg: 2, scope: !12, file: !1, line: 4, type: !17)
110+
!22 = !DILocation(line: 0, scope: !12)
111+
!23 = !DILocation(line: 5, column: 11, scope: !12)
112+
!24 = !{!25, !26, i64 0}
113+
!25 = !{!"t", !26, i64 0}
114+
!26 = !{!"long", !27, i64 0}
115+
!27 = !{!"omnipotent char", !28, i64 0}
116+
!28 = !{!"Simple C/C++ TBAA"}
117+
!29 = !DILocation(line: 5, column: 6, scope: !12)
118+
!30 = !{!26, !26, i64 0}
119+
!31 = !DILocation(line: 6, column: 11, scope: !12)
120+
!32 = !DILocation(line: 6, column: 6, scope: !12)
121+
!33 = !DILocation(line: 7, column: 1, scope: !12)

0 commit comments

Comments
 (0)