Ignore metainstructions during the shrink wrap analysis

[aprantl]

Summary

Shrink wrapping should ignore DBG_VALUEs referring to frame indices, since the presence of debug information must not affect code generation.

[qcolombet]

LGTM

[thegameg]

llvm/trunk/lib/CodeGen/ShrinkWrap.cpp

244 ↗

(On Diff #126794)

Do we also want to ignore

%csr = IMPLICIT_DEF

?

I remember @MatzeB suggested this approach to me to write cleaner tests.

[MatzeB]

llvm/trunk/lib/CodeGen/ShrinkWrap.cpp

244 ↗

(On Diff #126794)

Should probably do MO.isDef() || MO.readsReg() below instead of this check.

[thegameg]

llvm/trunk/lib/CodeGen/ShrinkWrap.cpp

244 ↗

(On Diff #126794)

That would do it for register operands, but seems that DBG_VALUE instructions also have FrameIndex operands. Is MI.isDebugValue() not enough here?

[MatzeB]

llvm/trunk/lib/CodeGen/ShrinkWrap.cpp

244 ↗

(On Diff #126794)

the operands of DBG_VALUE have MachineOperand::isDebug() set, so readsReg() will always return false for them. The good thing about MachineOperand::readsReg() is that it also checks some other conditions that are often forgotten (such as undef operands or internals reads in bundles, which both are forgotten here as well it seems).

[MatzeB]

llvm/trunk/lib/CodeGen/ShrinkWrap.cpp

244 ↗

(On Diff #126794)

And you are right you probably need an extra MO.isDebugValue() check for the frame index case, didn't realize the code was checking MachineOperand::isFI() here. But in this case I'd move the isDebugValue() check closer to the isFI() check.

lib/CodeGen/ShrinkWrap.cpp

 INITIALIZE_PASS_DEPENDENCY(MachineBlockFrequencyInfo)
 INITIALIZE_PASS_DEPENDENCY(MachineDominatorTree)
 INITIALIZE_PASS_DEPENDENCY(MachinePostDominatorTree)
 INITIALIZE_PASS_DEPENDENCY(MachineLoopInfo)
 INITIALIZE_PASS_END(ShrinkWrap, DEBUG_TYPE, "Shrink Wrap Pass", false, false)
 
 bool ShrinkWrap::useOrDefCSROrFI(const MachineInstr &MI,
                                  RegScavenger *RS) const {
+  // Ignore DBG_VALUE and other meta instructions that must not affect codegen.
+  if (MI.isMetaInstruction())
+    return false;
+
   if (MI.getOpcode() == FrameSetupOpcode ||
       MI.getOpcode() == FrameDestroyOpcode) {
     DEBUG(dbgs() << "Frame instruction: " << MI << '\n');
     return true;
   }
   for (const MachineOperand &MO : MI.operands()) {
     bool UseOrDefCSR = false;
     if (MO.isReg()) {

test/CodeGen/MIR/X86/shrink_wrap_dbg_value.mir

+# RUN: llc -o - %s -run-pass=shrink-wrap | FileCheck %s
+--- |
+  ; ModuleID = '<stdin>'
+  source_filename = "t.c"
+  target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
+  target triple = "i386-pc-windows-msvc19.11.25508"
+  
+  ; Function Attrs: nounwind
+  define x86_fastcallcc i32 @"@shrink_wrap_basic@16"(i32 inreg %a, i32 inreg %b, i32 %c, i32 %d) local_unnamed_addr #0 !dbg !8 {
+  entry:
+    %c.addr = alloca i32, align 4
+    tail call void @llvm.dbg.value(metadata i32 %d, metadata !13, metadata !DIExpression()), !dbg !19
+    tail call void @llvm.dbg.value(metadata i32 %c, metadata !14, metadata !DIExpression()), !dbg !20
+    store i32 %c, i32* %c.addr, align 4, !tbaa !21
+    tail call void @llvm.dbg.value(metadata i32 %b, metadata !15, metadata !DIExpression()), !dbg !25
+    tail call void @llvm.dbg.value(metadata i32 %a, metadata !16, metadata !DIExpression()), !dbg !26
+    %cmp = icmp slt i32 %a, %b, !dbg !27
+    br i1 %cmp, label %return, label %for.cond.preheader, !dbg !29
+  
+  for.cond.preheader:                               ; preds = %entry
+    %0 = add i32 %c, -1, !dbg !30
+    br label %for.cond, !dbg !30
+  
+  for.cond:                                         ; preds = %for.cond, %for.cond.preheader
+    %lsr.iv = phi i32 [ %lsr.iv.next, %for.cond ], [ %0, %for.cond.preheader ]
+    call void @llvm.dbg.value(metadata i32 undef, metadata !17, metadata !DIExpression()), !dbg !32
+    call void @llvm.dbg.value(metadata i32* %c.addr, metadata !14, metadata !DIExpression()), !dbg !20
+    %call = call i32 @doSomething(i32* nonnull %c.addr) #3, !dbg !33
+    call void @llvm.dbg.value(metadata !2, metadata !17, metadata !DIExpression()), !dbg !32
+    %lsr.iv.next = add i32 %lsr.iv, 1, !dbg !30
+    %cmp1 = icmp slt i32 %lsr.iv.next, %d, !dbg !30
+    br i1 %cmp1, label %for.cond, label %return, !dbg !34, !llvm.loop !35
+  
+  return:                                           ; preds = %for.cond, %entry
+    %retval.0 = phi i32 [ %a, %entry ], [ %call, %for.cond ]
+    ret i32 %retval.0, !dbg !37
+  }
+  
+  declare i32 @doSomething(i32*) local_unnamed_addr
+  
+  ; Function Attrs: nounwind readnone speculatable
+  declare void @llvm.dbg.value(metadata, metadata, metadata) #2
+  
+  ; Function Attrs: nounwind
+  declare void @llvm.stackprotector(i8*, i8**) #3
+  
+  attributes #0 = { nounwind }
+  attributes #2 = { nounwind readnone speculatable }
+  attributes #3 = { nounwind }
+  
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!3, !4, !5, !6}
+  !llvm.ident = !{!7}
+  
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 6.0.0 ", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+  !1 = !DIFile(filename: "t.c", directory: "C:\5Csrc\5Cllvm-project\5Cbuild", checksumkind: CSK_MD5, checksum: "32f118fd5dd7e65ff7733c49b2f804ef")
+  !2 = !{}
+  !3 = !{i32 1, !"NumRegisterParameters", i32 0}
+  !4 = !{i32 2, !"CodeView", i32 1}
+  !5 = !{i32 2, !"Debug Info Version", i32 3}
+  !6 = !{i32 1, !"wchar_size", i32 2}
+  !7 = !{!"clang version 6.0.0 "}
+  !8 = distinct !DISubprogram(name: "shrink_wrap_basic", linkageName: "\01@shrink_wrap_basic@16", scope: !1, file: !1, line: 2, type: !9, isLocal: false, isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !12)
+  !9 = !DISubroutineType(cc: DW_CC_BORLAND_msfastcall, types: !10)
+  !10 = !{!11, !11, !11, !11, !11}
+  !11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !12 = !{!13, !14, !15, !16, !17}
+  !13 = !DILocalVariable(name: "d", arg: 4, scope: !8, file: !1, line: 2, type: !11)
+  !14 = !DILocalVariable(name: "c", arg: 3, scope: !8, file: !1, line: 2, type: !11)
+  !15 = !DILocalVariable(name: "b", arg: 2, scope: !8, file: !1, line: 2, type: !11)
+  !16 = !DILocalVariable(name: "a", arg: 1, scope: !8, file: !1, line: 2, type: !11)
+  !17 = !DILocalVariable(name: "i", scope: !18, file: !1, line: 5, type: !11)
+  !18 = distinct !DILexicalBlock(scope: !8, file: !1, line: 5, column: 3)
+  !19 = !DILocation(line: 2, column: 59, scope: !8)
+  !20 = !DILocation(line: 2, column: 52, scope: !8)
+  !21 = !{!22, !22, i64 0}
+  !22 = !{!"int", !23, i64 0}
+  !23 = !{!"omnipotent char", !24, i64 0}
+  !24 = !{!"Simple C/C++ TBAA"}
+  !25 = !DILocation(line: 2, column: 45, scope: !8)
+  !26 = !DILocation(line: 2, column: 38, scope: !8)
+  !27 = !DILocation(line: 3, column: 9, scope: !28)
+  !28 = distinct !DILexicalBlock(scope: !8, file: !1, line: 3, column: 7)
+  !29 = !DILocation(line: 3, column: 7, scope: !8)
+  !30 = !DILocation(line: 5, column: 21, scope: !31)
+  !31 = distinct !DILexicalBlock(scope: !18, file: !1, line: 5, column: 3)
+  !32 = !DILocation(line: 5, column: 12, scope: !18)
+  !33 = !DILocation(line: 0, scope: !8)
+  !34 = !DILocation(line: 5, column: 3, scope: !18)
+  !35 = distinct !{!35, !34, !36}
+  !36 = !DILocation(line: 6, column: 19, scope: !18)
+  !37 = !DILocation(line: 8, column: 1, scope: !8)
+
+...
+---
+name:            '@shrink_wrap_basic@16'
+alignment:       4
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+registers:       
+liveins:         
+  - { reg: '%ecx', virtual-reg: '' }
+  - { reg: '%edx', virtual-reg: '' }
+frameInfo:       
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    4
+  adjustsStack:    false
+  hasCalls:        true
+  stackProtector:  ''
+  maxCallFrameSize: 4294967295
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  # CHECK: savePoint:       '%bb.1'
+  # CHECK: restorePoint:    '%bb.3'
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:      
+  - { id: 0, type: default, offset: 4, size: 4, alignment: 4, stack-id: 0, 
+      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true }
+  - { id: 1, type: default, offset: 0, size: 4, alignment: 4, stack-id: 0, 
+      isImmutable: false, isAliased: false, callee-saved-register: '', 
+      callee-saved-restored: true }
+stack:           
+constants:       
+body:             |
+  bb.0.entry:
+    successors: %bb.4(0x40000000), %bb.1(0x40000000)
+    liveins: %ecx, %edx
+  
+    DBG_VALUE debug-use %edx, debug-use %noreg, !15, !DIExpression(), debug-location !25
+    DBG_VALUE debug-use %ecx, debug-use %noreg, !16, !DIExpression(), debug-location !26
+    %eax = COPY %ecx
+    DBG_VALUE %fixed-stack.0, 0, !16, !DIExpression(), debug-location !26
+    DBG_VALUE %fixed-stack.1, 0, !15, !DIExpression(), debug-location !25
+    CMP32rr %eax, killed %edx, implicit-def %eflags, debug-location !27
+    JL_1 %bb.4, implicit killed %eflags, debug-location !29
+    JMP_1 %bb.1, debug-location !29
+  
+  bb.1.for.cond.preheader:
+    successors: %bb.2(0x80000000)
+  
+    %esi = MOV32rm %fixed-stack.0, 1, %noreg, 0, %noreg :: (load 4 from %fixed-stack.0)
+    DBG_VALUE debug-use %esi, debug-use %noreg, !13, !DIExpression(), debug-location !19
+    %edi = MOV32rm %fixed-stack.1, 1, %noreg, 0, %noreg :: (load 4 from %fixed-stack.1)
+    DBG_VALUE debug-use %edi, debug-use %noreg, !14, !DIExpression(), debug-location !20
+    %edi = DEC32r killed %edi, implicit-def dead %eflags, debug-location !30
+    %ebx = LEA32r %fixed-stack.1, 1, %noreg, 0, %noreg
+  
+  bb.2.for.cond:
+    successors: %bb.2(0x7c000000), %bb.3(0x04000000)
+    liveins: %ebx, %edi, %esi
+  
+    ADJCALLSTACKDOWN32 4, 0, 4, implicit-def dead %esp, implicit-def dead %eflags, implicit-def dead %ssp, implicit %esp, implicit %ssp, debug-location !33
+    DBG_VALUE %fixed-stack.1, 0, !14, !DIExpression(), debug-location !20
+    PUSH32r %ebx, implicit-def %esp, implicit %esp, debug-location !33
+    ;CFI_INSTRUCTION <unserializable cfi operation>, debug-location !33
+    CALLpcrel32 @doSomething, csr_32, implicit %esp, implicit %ssp, implicit-def %esp, implicit-def %ssp, implicit-def %eax, debug-location !33
+    ADJCALLSTACKUP32 4, 0, implicit-def dead %esp, implicit-def dead %eflags, implicit-def dead %ssp, implicit %esp, implicit %ssp, debug-location !33
+    %edi = INC32r killed %edi, implicit-def dead %eflags, debug-location !30
+    CMP32rr %edi, %esi, implicit-def %eflags, debug-location !30
+    JL_1 %bb.2, implicit killed %eflags, debug-location !34
+  
+  bb.3:
+    successors: %bb.4(0x80000000)
+    liveins: %eax
+  
+  
+  bb.4.return:
+    liveins: %eax
+  
+    RET 8, %eax, debug-location !37
+
+...