[llvm] 3cb7e7b - BPF: fix a CORE optimization bug
Tom Stellard via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 20 20:05:48 PDT 2020
On 04/20/2020 08:04 PM, Yonghong Song via llvm-commits wrote:
>
> Author: Yonghong Song
> Date: 2020-04-20T19:54:51-07:00
> New Revision: 3cb7e7bf959dcd3b8080986c62e10a75c7af43f0
>
Should we backport this to the release/10.x branch?
-Tom
> URL: https://github.com/llvm/llvm-project/commit/3cb7e7bf959dcd3b8080986c62e10a75c7af43f0
> DIFF: https://github.com/llvm/llvm-project/commit/3cb7e7bf959dcd3b8080986c62e10a75c7af43f0.diff
>
> LOG: BPF: fix a CORE optimization bug
>
> For the test case in this patch like below
> struct t { int a; } __attribute__((preserve_access_index));
> int foo(void *);
> int test(struct t *arg) {
> long param[1];
> param[0] = (long)&arg->a;
> return foo(param);
> }
>
> The IR right before BPF SimplifyPatchable phase:
> %1:gpr = LD_imm64 @"llvm.t:0:0$0:0"
> %2:gpr = LDD killed %1:gpr, 0
> %3:gpr = ADD_rr %0:gpr(tied-def 0), killed %2:gpr
> STD killed %3:gpr, %stack.0.param, 0
> After SimplifyPatchable phase, the incorrect IR is generated:
> %1:gpr = LD_imm64 @"llvm.t:0:0$0:0"
> %3:gpr = ADD_rr %0:gpr(tied-def 0), killed %1:gpr
> CORE_MEM killed %3:gpr, 306, %0:gpr, @"llvm.t:0:0$0:0"
>
> Note that CORE_MEM pseudo op is introduced to encode
> memory operations related to CORE. In the above, we intend
> to check whether we have a store like
> *(%3:gpr + 0) = ...
> and if this is the case, we could replace it with
> *(%0:gpr + @"llvm.t:0:0$0:0"_ = ...
>
> Unfortunately, in the above, IR for the store is
> *(%stack.0.param + 0) = %3:gpr
> and transformation should not happen.
>
> Note that we won't have problem if the actual CORE
> dereference (arg->a) happens.
>
> This patch fixed the problem by skip CORE optimization if
> the use of ADD_rr result is not the base address of the store
> operation.
>
> Differential Revision: https://reviews.llvm.org/D78466
>
> Added:
> llvm/test/CodeGen/BPF/CORE/store-addr.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 29abc9303a62..b2ecb531db9d 100644
> --- a/llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp
> +++ b/llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp
> @@ -116,11 +116,22 @@ void BPFMISimplifyPatchable::checkADDrr(MachineRegisterInfo *MRI,
> else
> continue;
>
> - // It must be a form of %1 = *(type *)(%2 + 0) or *(type *)(%2 + 0) = %1.
> + // It must be a form of %2 = *(type *)(%1 + 0) or *(type *)(%1 + 0) = %2.
> const MachineOperand &ImmOp = DefInst->getOperand(2);
> if (!ImmOp.isImm() || ImmOp.getImm() != 0)
> continue;
>
> + // Reject the form:
> + // %1 = ADD_rr %2, %3
> + // *(type *)(%2 + 0) = %1
> + if (Opcode == BPF::STB || Opcode == BPF::STH || Opcode == BPF::STW ||
> + Opcode == BPF::STD || Opcode == BPF::STB32 || Opcode == BPF::STH32 ||
> + Opcode == BPF::STW32) {
> + const MachineOperand &Opnd = DefInst->getOperand(0);
> + if (Opnd.isReg() && Opnd.getReg() == I->getReg())
> + continue;
> + }
> +
> BuildMI(*DefInst->getParent(), *DefInst, DefInst->getDebugLoc(), TII->get(COREOp))
> .add(DefInst->getOperand(0)).addImm(Opcode).add(*BaseOp)
> .addGlobalAddress(GVal);
>
> diff --git a/llvm/test/CodeGen/BPF/CORE/store-addr.ll b/llvm/test/CodeGen/BPF/CORE/store-addr.ll
> new file mode 100644
> index 000000000000..4b19c33e6665
> --- /dev/null
> +++ b/llvm/test/CodeGen/BPF/CORE/store-addr.ll
> @@ -0,0 +1,107 @@
> +; RUN: llc -march=bpfel -filetype=asm -o - %s | FileCheck %s
> +; RUN: llc -march=bpfel -mattr=+alu32 -filetype=asm -o - %s | FileCheck %s
> +; Source code:
> +; struct t {
> +; int a;
> +; } __attribute__((preserve_access_index));
> +; int foo(void *);
> +; int test(struct t *arg) {
> +; long param[1];
> +; param[0] = (long)&arg->a;
> +; return foo(param);
> +; }
> +; Compiler flag to generate IR:
> +; clang -target bpf -S -O2 -g -emit-llvm test.c
> +
> +%struct.t = type { i32 }
> +
> +; Function Attrs: nounwind
> +define dso_local i32 @test(%struct.t* %arg) local_unnamed_addr #0 !dbg !14 {
> +entry:
> + %param = alloca [1 x i64], align 8
> + call void @llvm.dbg.value(metadata %struct.t* %arg, metadata !22, metadata !DIExpression()), !dbg !27
> + %0 = bitcast [1 x i64]* %param to i8*, !dbg !28
> + call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %0) #5, !dbg !28
> + call void @llvm.dbg.declare(metadata [1 x i64]* %param, metadata !23, metadata !DIExpression()), !dbg !29
> + %1 = tail call i32* @llvm.preserve.struct.access.index.p0i32.p0s_struct.ts(%struct.t* %arg, i32 0, i32 0), !dbg !30, !llvm.preserve.access.index !18
> + %2 = ptrtoint i32* %1 to i64, !dbg !31
> + %arrayidx = getelementptr inbounds [1 x i64], [1 x i64]* %param, i64 0, i64 0, !dbg !32
> + store i64 %2, i64* %arrayidx, align 8, !dbg !33, !tbaa !34
> + %call = call i32 @foo(i8* nonnull %0) #5, !dbg !38
> + call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %0) #5, !dbg !39
> + ret i32 %call, !dbg !40
> +}
> +
> +; CHECK: r[[OFFSET:[0-9]+]] = 0
> +; CHECK: r1 += r[[OFFSET]]
> +; CHECK: *(u64 *)(r10 - 8) = r1
> +
> +; Function Attrs: nounwind readnone speculatable
> +declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
> +
> +; Function Attrs: argmemonly nounwind
> +declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture) #2
> +
> +; Function Attrs: nounwind readnone
> +declare i32* @llvm.preserve.struct.access.index.p0i32.p0s_struct.ts(%struct.t*, i32, i32) #3
> +
> +declare !dbg !5 dso_local i32 @foo(i8*) local_unnamed_addr #4
> +
> +; Function Attrs: argmemonly nounwind
> +declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture) #2
> +
> +; Function Attrs: nounwind readnone speculatable
> +declare void @llvm.dbg.value(metadata, metadata, metadata) #1
> +
> +attributes #0 = { nounwind "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
> +attributes #1 = { nounwind readnone speculatable }
> +attributes #2 = { argmemonly nounwind }
> +attributes #3 = { nounwind readnone }
> +attributes #4 = { "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
> +attributes #5 = { nounwind }
> +
> +!llvm.dbg.cu = !{!0}
> +!llvm.module.flags = !{!10, !11, !12}
> +!llvm.ident = !{!13}
> +
> +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 11.0.0 (https://github.com/llvm/llvm-project.git 4f995959a05ae94cc4f9cc80035f7e4b3ecd2d88)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, splitDebugInlining: false, nameTableKind: None)
> +!1 = !DIFile(filename: "test.c", directory: "/tmp/home/yhs/work/tests/core")
> +!2 = !{}
> +!3 = !{!4, !5}
> +!4 = !DIBasicType(name: "long int", size: 64, encoding: DW_ATE_signed)
> +!5 = !DISubprogram(name: "foo", scope: !1, file: !1, line: 4, type: !6, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
> +!6 = !DISubroutineType(types: !7)
> +!7 = !{!8, !9}
> +!8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
> +!9 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 64)
> +!10 = !{i32 7, !"Dwarf Version", i32 4}
> +!11 = !{i32 2, !"Debug Info Version", i32 3}
> +!12 = !{i32 1, !"wchar_size", i32 4}
> +!13 = !{!"clang version 11.0.0 (https://github.com/llvm/llvm-project.git 4f995959a05ae94cc4f9cc80035f7e4b3ecd2d88)"}
> +!14 = distinct !DISubprogram(name: "test", scope: !1, file: !1, line: 5, type: !15, scopeLine: 5, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !21)
> +!15 = !DISubroutineType(types: !16)
> +!16 = !{!8, !17}
> +!17 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !18, size: 64)
> +!18 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "t", file: !1, line: 1, size: 32, elements: !19)
> +!19 = !{!20}
> +!20 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !18, file: !1, line: 2, baseType: !8, size: 32)
> +!21 = !{!22, !23}
> +!22 = !DILocalVariable(name: "arg", arg: 1, scope: !14, file: !1, line: 5, type: !17)
> +!23 = !DILocalVariable(name: "param", scope: !14, file: !1, line: 6, type: !24)
> +!24 = !DICompositeType(tag: DW_TAG_array_type, baseType: !4, size: 64, elements: !25)
> +!25 = !{!26}
> +!26 = !DISubrange(count: 1)
> +!27 = !DILocation(line: 0, scope: !14)
> +!28 = !DILocation(line: 6, column: 5, scope: !14)
> +!29 = !DILocation(line: 6, column: 10, scope: !14)
> +!30 = !DILocation(line: 7, column: 28, scope: !14)
> +!31 = !DILocation(line: 7, column: 16, scope: !14)
> +!32 = !DILocation(line: 7, column: 5, scope: !14)
> +!33 = !DILocation(line: 7, column: 14, scope: !14)
> +!34 = !{!35, !35, i64 0}
> +!35 = !{!"long", !36, i64 0}
> +!36 = !{!"omnipotent char", !37, i64 0}
> +!37 = !{!"Simple C/C++ TBAA"}
> +!38 = !DILocation(line: 8, column: 12, scope: !14)
> +!39 = !DILocation(line: 9, column: 1, scope: !14)
> +!40 = !DILocation(line: 8, column: 5, scope: !14)
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list