[llvm-branch-commits] [llvm] bceec8e - [BPF] Reset machine register kill mark in BPFMISimplifyPatchable
Tobias Hieta via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Aug 21 22:51:31 PDT 2023
Author: Eduard Zingerman
Date: 2023-08-22T07:48:00+02:00
New Revision: bceec8e801cfd359b6de86237e8cb7e2f8f8efa7
URL: https://github.com/llvm/llvm-project/commit/bceec8e801cfd359b6de86237e8cb7e2f8f8efa7
DIFF: https://github.com/llvm/llvm-project/commit/bceec8e801cfd359b6de86237e8cb7e2f8f8efa7.diff
LOG: [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
(cherry picked from commit 27026fe5633b546ed647efd99eccdfc598686535)
Added:
llvm/test/CodeGen/BPF/CORE/simplify-patchable-liveness-bug.ll
Modified:
llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp b/llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp
index 088195994edd61..67574403ca8328 100644
--- a/llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp
+++ b/llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp
@@ -207,8 +207,32 @@ void BPFMISimplifyPatchable::processDstReg(MachineRegisterInfo *MRI,
decltype(End) NextI;
for (auto I = Begin; I != End; I = NextI) {
NextI = std::next(I);
- if (doSrcRegProp)
+ if (doSrcRegProp) {
+ // In situations like below it is not known if usage is a kill
+ // after setReg():
+ //
+ // .-> %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 I
+ // `----------------.
+ // %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 I
+ //
+ // Instructions (1) and (2) would be updated by setReg() to:
+ //
+ // ADD_rr %0:gpr(tied-def 0), %2:gpr
+ //
+ // %2:gpr is not killed at (1), so it is necessary to remove kill flag
+ // from I.
I->setReg(SrcReg);
+ I->setIsKill(false);
+ }
// The candidate needs to have a unique definition.
if (IsAma && MRI->getUniqueVRegDef(I->getReg()))
diff --git a/llvm/test/CodeGen/BPF/CORE/simplify-patchable-liveness-bug.ll b/llvm/test/CodeGen/BPF/CORE/simplify-patchable-liveness-bug.ll
new file mode 100644
index 00000000000000..d8fd7cce193b96
--- /dev/null
+++ b/llvm/test/CodeGen/BPF/CORE/simplify-patchable-liveness-bug.ll
@@ -0,0 +1,121 @@
+; RUN: llc -mtriple=bpf -mcpu=v2 < %s | FileCheck -check-prefixes=CHECK %s
+
+; Test for machine register liveness update bug in
+; BPFMISimplifyPatchable::processDstReg.
+;
+; Generated from the following source code:
+; 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;
+; }
+;
+; Using the following command:
+; clang -g -O2 -S -emit-llvm --target=bpf t.c -o t.ll
+
+@"llvm.t:0:0$0:0" = external global i64, !llvm.preserve.access.index !0 #0
+
+; Function Attrs: nofree nounwind
+define dso_local void @foo(ptr noundef %t, ptr noundef %p) local_unnamed_addr #1 !dbg !12 {
+entry:
+ call void @llvm.dbg.value(metadata ptr %t, metadata !20, metadata !DIExpression()), !dbg !22
+ call void @llvm.dbg.value(metadata ptr %p, metadata !21, metadata !DIExpression()), !dbg !22
+ %0 = load i64, ptr @"llvm.t:0:0$0:0", align 8
+ %1 = getelementptr i8, ptr %t, i64 %0
+ %2 = tail call ptr @llvm.bpf.passthrough.p0.p0(i32 0, ptr %1)
+ %3 = load volatile i64, ptr %2, align 8, !dbg !23, !tbaa !24
+ store volatile i64 %3, ptr %p, align 8, !dbg !29, !tbaa !30
+ %4 = load i64, ptr @"llvm.t:0:0$0:0", align 8
+ %5 = getelementptr i8, ptr %t, i64 %4
+ %6 = tail call ptr @llvm.bpf.passthrough.p0.p0(i32 1, ptr %5)
+ %7 = load volatile i64, ptr %6, align 8, !dbg !31, !tbaa !24
+ store volatile i64 %7, ptr %p, align 8, !dbg !32, !tbaa !30
+ ret void, !dbg !33
+}
+
+; CHECK: foo:
+; CHECK: prologue_end
+; CHECK-NEXT: .Ltmp[[LABEL1:.*]]:
+; CHECK-NEXT: .Ltmp
+; CHECK-NEXT: r[[#A:]] = *(u64 *)(r1 + 0)
+; CHECK-NEXT: .loc
+; CHECK-NEXT: .Ltmp
+; CHECK-NEXT: *(u64 *)(r2 + 0) = r[[#A]]
+; CHECK-NEXT: .loc
+; CHECK-NEXT: .Ltmp[[LABEL2:.*]]:
+; CHECK-NEXT: .Ltmp
+; CHECK-NEXT: r[[#B:]] = *(u64 *)(r1 + 0)
+; CHECK-NEXT: .Ltmp
+; CHECK-NEXT: .Ltmp
+; CHECK-NEXT: .loc
+; CHECK-NEXT: .Ltmp
+; CHECK-NEXT: *(u64 *)(r2 + 0) = r[[#B]]
+
+; CHECK: .section .BTF
+; CHECK: .long [[STR_T:.*]] # BTF_KIND_STRUCT(id = [[ID:.*]])
+
+; CHECK: .byte 116 # string offset=[[STR_T]]
+; CHECK: .ascii "0:0" # string offset=[[STR_A:.*]]
+
+; CHECK: # FieldReloc
+; CHECK: .long .Ltmp[[LABEL1]]
+; CHECK-NEXT: .long [[ID]]
+; CHECK-NEXT: .long [[STR_A]]
+; CHECK-NEXT: .long 0
+; CHECK: .long .Ltmp[[LABEL2]]
+; CHECK-NEXT: .long [[ID]]
+; CHECK-NEXT: .long [[STR_A]]
+; CHECK-NEXT: .long 0
+
+; Function Attrs: nofree nosync nounwind memory(none)
+declare ptr @llvm.bpf.passthrough.p0.p0(i32, ptr) #2
+
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare void @llvm.dbg.value(metadata, metadata, metadata) #3
+
+attributes #0 = { "btf_ama" }
+attributes #1 = { nofree nounwind "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
+attributes #2 = { nofree nosync nounwind memory(none) }
+attributes #3 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
+
+!llvm.dbg.cu = !{!5}
+!llvm.module.flags = !{!6, !7, !8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "t", file: !1, line: 1, size: 64, elements: !2)
+!1 = !DIFile(filename: "some.file", directory: "/some/dir", checksumkind: CSK_MD5, checksum: "a149cfaf65a83125e7f2b2f47e5c7287")
+!2 = !{!3}
+!3 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !0, file: !1, line: 2, baseType: !4, size: 64)
+!4 = !DIBasicType(name: "unsigned long", size: 64, encoding: DW_ATE_unsigned)
+!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)
+!6 = !{i32 7, !"Dwarf Version", i32 5}
+!7 = !{i32 2, !"Debug Info Version", i32 3}
+!8 = !{i32 1, !"wchar_size", i32 4}
+!9 = !{i32 7, !"frame-pointer", i32 2}
+!10 = !{i32 7, !"debug-info-assignment-tracking", i1 true}
+!11 = !{!"clang version 18.0.0 (/home/eddy/work/llvm-project/clang 3810f2eb4382d5e2090ce5cd47f45379cb453c35)"}
+!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)
+!13 = !DISubroutineType(types: !14)
+!14 = !{null, !15, !17}
+!15 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !16, size: 64)
+!16 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !0)
+!17 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !18, size: 64)
+!18 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !4)
+!19 = !{!20, !21}
+!20 = !DILocalVariable(name: "t", arg: 1, scope: !12, file: !1, line: 4, type: !15)
+!21 = !DILocalVariable(name: "p", arg: 2, scope: !12, file: !1, line: 4, type: !17)
+!22 = !DILocation(line: 0, scope: !12)
+!23 = !DILocation(line: 5, column: 11, scope: !12)
+!24 = !{!25, !26, i64 0}
+!25 = !{!"t", !26, i64 0}
+!26 = !{!"long", !27, i64 0}
+!27 = !{!"omnipotent char", !28, i64 0}
+!28 = !{!"Simple C/C++ TBAA"}
+!29 = !DILocation(line: 5, column: 6, scope: !12)
+!30 = !{!26, !26, i64 0}
+!31 = !DILocation(line: 6, column: 11, scope: !12)
+!32 = !DILocation(line: 6, column: 6, scope: !12)
+!33 = !DILocation(line: 7, column: 1, scope: !12)
More information about the llvm-branch-commits
mailing list