[llvm-branch-commits] [llvm] 138451c - [StackColoring] Remap FixedStackPseudoSourceValue frame index referenced by MachineMemOperand

Hans Wennborg via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Wed Jan 22 08:38:44 PST 2020


Thanks! But please check with me before pushing to the release branch.

On Tue, Jan 21, 2020 at 12:22 PM Fangrui Song via llvm-branch-commits
<llvm-branch-commits at lists.llvm.org> wrote:
>
>
> Author: Fangrui Song
> Date: 2020-01-21T12:14:27-08:00
> New Revision: 138451c771abe013b7b99650faeb7ae6010f7a8d
>
> URL: https://github.com/llvm/llvm-project/commit/138451c771abe013b7b99650faeb7ae6010f7a8d
> DIFF: https://github.com/llvm/llvm-project/commit/138451c771abe013b7b99650faeb7ae6010f7a8d.diff
>
> LOG: [StackColoring] Remap FixedStackPseudoSourceValue frame index referenced by MachineMemOperand
>
> StackColoring::remapInstructions() remaps MachineOperand frame index (e.g. %stack.1 -> %stack.0)
> but does not remap FixedStackPseudoSourceValue frame index (e.g. store 4 into %stack.1.ap2.i.i)
> referenced by MachineMemoryOperand.
>
> This can cause an assertion failure when LiveDebugValues references a dead stack object.
>
> It is difficult to craft a test case. -g, va_copy and stack-coloring are required.
> I can only reproduce it on ppc32.
>
> (cherry picked from commit eaab1bf21e1d6803fd217fe6052537fc33b06837)
> (cherry picked from commit 854f7be20a0cb1a95671a16d6cc8200107ee25f4)
> (cherry picked from commit 7a8b0b1595e7dc878b48cf9bbaa652087a6895db)
>
> Added:
>     llvm/test/CodeGen/PowerPC/stack-coloring-vararg.mir
>
> Modified:
>     llvm/lib/CodeGen/StackColoring.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff  --git a/llvm/lib/CodeGen/StackColoring.cpp b/llvm/lib/CodeGen/StackColoring.cpp
> index b6e81116286f..40bc36c3030b 100644
> --- a/llvm/lib/CodeGen/StackColoring.cpp
> +++ b/llvm/lib/CodeGen/StackColoring.cpp
> @@ -960,6 +960,7 @@ void StackColoring::remapInstructions(DenseMap<int, int> &SlotRemap) {
>    }
>
>    // Remap all instructions to the new stack slots.
> +  std::vector<std::vector<MachineMemOperand *>> SSRefs(MFI->getObjectIndexEnd());
>    for (MachineBasicBlock &BB : *MF)
>      for (MachineInstr &I : BB) {
>        // Skip lifetime markers. We'll remove them soon.
> @@ -1025,6 +1026,16 @@ void StackColoring::remapInstructions(DenseMap<int, int> &SlotRemap) {
>        SmallVector<MachineMemOperand *, 2> NewMMOs;
>        bool ReplaceMemOps = false;
>        for (MachineMemOperand *MMO : I.memoperands()) {
> +        // Collect MachineMemOperands which reference
> +        // FixedStackPseudoSourceValues with old frame indices.
> +        if (const auto *FSV = dyn_cast_or_null<FixedStackPseudoSourceValue>(
> +                MMO->getPseudoValue())) {
> +          int FI = FSV->getFrameIndex();
> +          auto To = SlotRemap.find(FI);
> +          if (To != SlotRemap.end())
> +            SSRefs[FI].push_back(MMO);
> +        }
> +
>          // If this memory location can be a slot remapped here,
>          // we remove AA information.
>          bool MayHaveConflictingAAMD = false;
> @@ -1062,6 +1073,14 @@ void StackColoring::remapInstructions(DenseMap<int, int> &SlotRemap) {
>          I.setMemRefs(*MF, NewMMOs);
>      }
>
> +  // Rewrite MachineMemOperands that reference old frame indices.
> +  for (auto E : enumerate(SSRefs)) {
> +    const PseudoSourceValue *NewSV =
> +        MF->getPSVManager().getFixedStack(SlotRemap[E.index()]);
> +    for (MachineMemOperand *Ref : E.value())
> +      Ref->setValue(NewSV);
> +  }
> +
>    // Update the location of C++ catch objects for the MSVC personality routine.
>    if (WinEHFuncInfo *EHInfo = MF->getWinEHFuncInfo())
>      for (WinEHTryBlockMapEntry &TBME : EHInfo->TryBlockMap)
>
> diff  --git a/llvm/test/CodeGen/PowerPC/stack-coloring-vararg.mir b/llvm/test/CodeGen/PowerPC/stack-coloring-vararg.mir
> new file mode 100644
> index 000000000000..0085369e1978
> --- /dev/null
> +++ b/llvm/test/CodeGen/PowerPC/stack-coloring-vararg.mir
> @@ -0,0 +1,171 @@
> +# RUN: llc -run-pass=stack-coloring %s -o - | FileCheck %s
> +
> +## Test %stack.1 is merged into %stack.0 and there is no MemoryMemOperand
> +## referencing %stack.1. This regression test is sensitive to the StackColoring
> +## algorithm. Please adjust or delete this test if the merging strategy
> +## changes.
> +
> +# CHECK:    {{^}}stack:
> +# CHECK-NEXT: - { id: 0,
> +# CHECK-NOT:  - { id: 1,
> +# CHECK:      - { id: 2,
> +# CHECK-NOT: %stack.1
> +
> +--- |
> +  ; ModuleID = '<stdin>'
> +  source_filename = "<stdin>"
> +  target datalayout = "E-m:e-p:32:32-i64:64-n32"
> +  target triple = "powerpc-unknown-freebsd13.0"
> +
> +  %struct.__va_list_tag = type { i8, i8, i16, i8*, i8* }
> +  ; Function Attrs: argmemonly nounwind willreturn
> +  declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #0
> +  define dso_local void @atf_tc_fail_nonfatal(i8* %fmt, ...) !dbg !3 {
> +  entry:
> +    %buf.i.i = alloca [1024 x i8], align 1
> +    %ap2.i.i = alloca [1 x %struct.__va_list_tag], align 4
> +    br i1 undef, label %format_reason_ap.exit.i, label %if.then6.i.i
> +
> +  if.then6.i.i:                                     ; preds = %entry
> +    %0 = bitcast [1 x %struct.__va_list_tag]* %ap2.i.i to i8*
> +    call void @llvm.lifetime.start.p0i8(i64 12, i8* nonnull %0)
> +    call void @llvm.va_copy(i8* nonnull %0, i8* nonnull null)
> +    ret void
> +
> +  format_reason_ap.exit.i:                          ; preds = %entry
> +    %1 = bitcast [1024 x i8]* %buf.i.i to i8*
> +    call void @llvm.lifetime.start.p0i8(i64 1024, i8* nonnull %1)
> +    call void @fprintf(i8* nonnull %1)
> +    ret void
> +  }
> +  declare void @fprintf(i8*)
> +  ; Function Attrs: nounwind
> +  declare void @llvm.va_copy(i8*, i8*) #1
> +
> +  attributes #0 = { argmemonly nounwind willreturn }
> +  attributes #1 = { nounwind }
> +
> +  !llvm.dbg.cu = !{!0}
> +  !llvm.module.flags = !{!2}
> +
> +  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, splitDebugInlining: false, nameTableKind: None)
> +  !1 = !DIFile(filename: "tc.c", directory: "")
> +  !2 = !{i32 2, !"Debug Info Version", i32 3}
> +  !3 = distinct !DISubprogram(name: "atf_tc_fail_nonfatal", scope: !1, file: !1, line: 1067, type: !4, scopeLine: 1068, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
> +  !4 = !DISubroutineType(types: !5)
> +  !5 = !{}
> +
> +...
> +---
> +name:            atf_tc_fail_nonfatal
> +alignment:       4
> +tracksRegLiveness: true
> +registers:
> +  - { id: 0, class: gprc }
> +  - { id: 1, class: gprc }
> +  - { id: 2, class: gprc }
> +  - { id: 3, class: gprc }
> +  - { id: 4, class: gprc }
> +  - { id: 5, class: gprc }
> +  - { id: 6, class: gprc }
> +  - { id: 7, class: gprc }
> +  - { id: 8, class: f8rc }
> +  - { id: 9, class: f8rc }
> +  - { id: 10, class: f8rc }
> +  - { id: 11, class: f8rc }
> +  - { id: 12, class: f8rc }
> +  - { id: 13, class: f8rc }
> +  - { id: 14, class: f8rc }
> +  - { id: 15, class: f8rc }
> +  - { id: 16, class: crbitrc }
> +  - { id: 17, class: gprc }
> +  - { id: 18, class: gprc }
> +  - { id: 19, class: gprc }
> +  - { id: 20, class: gprc }
> +liveins:
> +  - { reg: '$r3', virtual-reg: '%0' }
> +  - { reg: '$r4', virtual-reg: '%1' }
> +  - { reg: '$r5', virtual-reg: '%2' }
> +  - { reg: '$r6', virtual-reg: '%3' }
> +  - { reg: '$r7', virtual-reg: '%4' }
> +  - { reg: '$r8', virtual-reg: '%5' }
> +  - { reg: '$r9', virtual-reg: '%6' }
> +  - { reg: '$r10', virtual-reg: '%7' }
> +  - { reg: '$f1', virtual-reg: '%8' }
> +  - { reg: '$f2', virtual-reg: '%9' }
> +  - { reg: '$f3', virtual-reg: '%10' }
> +  - { reg: '$f4', virtual-reg: '%11' }
> +  - { reg: '$f5', virtual-reg: '%12' }
> +  - { reg: '$f6', virtual-reg: '%13' }
> +  - { reg: '$f7', virtual-reg: '%14' }
> +  - { reg: '$f8', virtual-reg: '%15' }
> +frameInfo:
> +  maxAlignment:    8
> +  hasCalls:        true
> +fixedStack:
> +  - { id: 0, offset: 8, size: 4, alignment: 8, isImmutable: true }
> +stack:
> +  - { id: 0, name: buf.i.i, size: 1024, alignment: 1 }
> +  - { id: 1, name: ap2.i.i, size: 12, alignment: 8 }
> +  - { id: 2, size: 96, alignment: 8 }
> +machineFunctionInfo: {}
> +body:             |
> +  bb.0.entry:
> +    liveins: $r3, $r4, $r5, $r6, $r7, $r8, $r9, $r10, $f1, $f2, $f3, $f4, $f5, $f6, $f7, $f8
> +
> +    %15:f8rc = COPY $f8
> +    %14:f8rc = COPY $f7
> +    %13:f8rc = COPY $f6
> +    %12:f8rc = COPY $f5
> +    %11:f8rc = COPY $f4
> +    %10:f8rc = COPY $f3
> +    %9:f8rc = COPY $f2
> +    %8:f8rc = COPY $f1
> +    %7:gprc = COPY $r10
> +    %6:gprc = COPY $r9
> +    %5:gprc = COPY $r8
> +    %4:gprc = COPY $r7
> +    %3:gprc = COPY $r6
> +    %2:gprc = COPY $r5
> +    %1:gprc = COPY $r4
> +    %0:gprc = COPY $r3
> +    STW %0, 0, %stack.2 :: (store 4 into %stack.2, align 8)
> +    STW %1, 4, %stack.2 :: (store 4 into %stack.2 + 4)
> +    STW %2, 8, %stack.2 :: (store 4 into %stack.2 + 8, align 8)
> +    STW %3, 12, %stack.2 :: (store 4)
> +    STW %4, 16, %stack.2 :: (store 4 into %stack.2 + 16, align 8)
> +    STW %5, 20, %stack.2 :: (store 4)
> +    STW %6, 24, %stack.2 :: (store 4 into %stack.2 + 24, align 8)
> +    STW %7, 28, %stack.2 :: (store 4)
> +    STFD %8, 32, %stack.2 :: (store 8)
> +    STFD %9, 40, %stack.2 :: (store 8)
> +    STFD %10, 48, %stack.2 :: (store 8)
> +    STFD %11, 56, %stack.2 :: (store 8)
> +    STFD %12, 64, %stack.2 :: (store 8)
> +    STFD %13, 72, %stack.2 :: (store 8)
> +    STFD %14, 80, %stack.2 :: (store 8)
> +    STFD %15, 88, %stack.2 :: (store 8)
> +    %16:crbitrc = IMPLICIT_DEF
> +    BC killed %16, %bb.2
> +    B %bb.1
> +
> +  bb.1.if.then6.i.i:
> +    LIFETIME_START %stack.1.ap2.i.i
> +    %17:gprc = LWZ 8, $zero :: (load 4, align 8)
> +    STW killed %17, 8, %stack.1.ap2.i.i :: (store 4 into %stack.1.ap2.i.i + 8, align 8)
> +    %18:gprc = LWZ 4, $zero :: (load 4)
> +    STW killed %18, 4, %stack.1.ap2.i.i :: (store 4 into %stack.1.ap2.i.i + 4, align 8)
> +    %19:gprc = LWZ 0, $zero :: (load 4, align 8)
> +    STW killed %19, 0, %stack.1.ap2.i.i :: (store 4 into %stack.1.ap2.i.i, align 8)
> +    BLR implicit $lr, implicit $rm
> +
> +  bb.2.format_reason_ap.exit.i:
> +    LIFETIME_START %stack.0.buf.i.i
> +    ADJCALLSTACKDOWN 8, 0, implicit-def dead $r1, implicit $r1
> +    %20:gprc = ADDI %stack.0.buf.i.i, 0
> +    $r3 = COPY %20
> +    BL @fprintf, csr_svr432, implicit-def dead $lr, implicit $rm, implicit $r3, implicit-def $r1
> +    ADJCALLSTACKUP 8, 0, implicit-def dead $r1, implicit $r1
> +    BLR implicit $lr, implicit $rm
> +
> +...
>
>
>
> _______________________________________________
> llvm-branch-commits mailing list
> llvm-branch-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


More information about the llvm-branch-commits mailing list