[llvm] [X86] Recognize POP/ADD/SUB modifying rsp in getSPAdjust. (PR #114265)

Björn Pettersson via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 18 13:32:14 PST 2024


bjope wrote:

We have seen some suspected miscompiles after this patch, and I've tried to reduce it a bit. What happens is that we get different code depending on if there is debug info or not (Which seems a bit weird given the code changes here? But maybe it exposes some older problem or I just haven't analyzed this patch enough?).

Below is my reduced test case. It is a mir test foo.mir like this:
```
--- |
  ; ModuleID = '<stdin>'
  source_filename = "reduced.ll"
  target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
  target triple = "x86_64-unknown-linux-gnu"

  ; Function Attrs: mustprogress nocallback nofree nosync nounwind willreturn memory(argmem: readwrite)
  declare void @llvm.lifetime.start.p0(i64 immarg, ptr nocapture) #0

  declare fastcc i16 @check_bi(i16, i256, i256) local_unnamed_addr

  define noundef i16 @test_one_15(i127 %loadedv3.i88, ptr nocapture writeonly %retval.i85, i256 %storedv322) local_unnamed_addr !dbg !4 {
  entry:
    %call216 = tail call fastcc i16 @check_bi(i16 0, i256 0, i256 -365375409332725729550921208179070754913983135743)
    store volatile i128 0, ptr null, align 4294967296
    %call276 = tail call fastcc i16 @check_bi(i16 0, i256 0, i256 0)
    tail call void @llvm.lifetime.start.p0(i64 0, ptr null), !dbg !7
    store i127 %loadedv3.i88, ptr %retval.i85, align 16
      #dbg_value(i256 0, !12, !DIExpression(), !14)
    %call337 = tail call fastcc i16 @check_bi(i16 0, i256 %storedv322, i256 0)
    %call392 = tail call fastcc i16 @check_bi(i16 0, i256 0, i256 -5575186299632655785383929568162090376495103)
    ret i16 0
  }

  attributes #0 = { mustprogress nocallback nofree nosync nounwind willreturn memory(argmem: readwrite) }

  !llvm.dbg.cu = !{!0}
  !llvm.module.flags = !{!3}

  !0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 20.0.0git.prerel (FlexC)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, retainedTypes: !2, splitDebugInlining: false, nameTableKind: None)
  !1 = !DIFile(filename: "foo.c", directory: "")
  !2 = !{}
  !3 = !{i32 2, !"Debug Info Version", i32 3}
  !4 = distinct !DISubprogram(name: "test_one_15", scope: !5, file: !5, line: 612, type: !6, scopeLine: 612, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
  !5 = !DIFile(filename: "foo.h", directory: "")
  !6 = distinct !DISubroutineType(types: !2)
  !7 = !DILocation(line: 103, column: 116, scope: !8, inlinedAt: !10)
  !8 = distinct !DISubprogram(name: "idbi", scope: !5, file: !5, line: 103, type: !9, scopeLine: 103, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
  !9 = distinct !DISubroutineType(types: !2)
  !10 = distinct !DILocation(line: 612, column: 202, scope: !11)
  !11 = distinct !DILexicalBlock(scope: !4, file: !5, line: 612, column: 112)
  !12 = !DILocalVariable(name: "res", scope: !11, file: !5, line: 612, type: !13)
  !13 = !DIBasicType(name: "_BitInt", size: 256, encoding: DW_ATE_signed)
  !14 = !DILocation(line: 0, scope: !11)

...
---
name:            test_one_15
alignment:       16
exposesReturnsTwice: false
legalized:       false
regBankSelected: false
selected:        false
failedISel:      false
tracksRegLiveness: true
hasWinCFI:       false
noPhis:          true
isSSA:           false
noVRegs:         true
hasFakeUses:     false
callsEHReturn:   false
callsUnwindInit: false
hasEHCatchret:   false
hasEHScopes:     false
hasEHFunclets:   false
isOutlined:      false
debugInstrRef:   true
failsVerification: false
tracksDebugUserValues: true
registers:       []
liveins:
  - { reg: '$rdi', virtual-reg: '' }
  - { reg: '$rsi', virtual-reg: '' }
  - { reg: '$rdx', virtual-reg: '' }
  - { reg: '$rcx', virtual-reg: '' }
  - { reg: '$r8', virtual-reg: '' }
  - { reg: '$r9', virtual-reg: '' }
frameInfo:
  isFrameAddressTaken: false
  isReturnAddressTaken: false
  hasStackMap:     false
  hasPatchPoint:   false
  stackSize:       0
  offsetAdjustment: 0
  maxAlignment:    16
  adjustsStack:    true
  hasCalls:        true
  stackProtector:  ''
  functionContext: ''
  maxCallFrameSize: 4294967295
  cvBytesOfCalleeSavedRegisters: 0
  hasOpaqueSPAdjustment: false
  hasVAStart:      false
  hasMustTailInVarArgFunc: false
  hasTailCall:     false
  isCalleeSavedInfoValid: false
  localFrameSize:  0
  savePoint:       ''
  restorePoint:    ''
fixedStack:
  - { id: 0, type: default, offset: 0, size: 8, alignment: 16, stack-id: default,
      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
stack:           []
entry_values:    []
debugValueSubstitutions: []
constants:       []
machineFunctionInfo:
  amxProgModel:    None
  FPClobberedByCall: false
  hasPushSequences: true
body:             |
  bb.0.entry:
    liveins: $rcx, $rdi, $rdx, $rsi, $r8, $r9

    renamable $rbx = COPY $r9
    renamable $r14 = COPY $r8
    renamable $r15 = COPY $rcx
    renamable $r13 = COPY $rdx
    renamable $r12 = COPY $rsi
    renamable $rbp = COPY $rdi
    ADJCALLSTACKDOWN64 32, 0, 32, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
    $edi = MOV32r0 implicit-def dead $eflags
    dead $esi = MOV32r0 implicit-def dead $eflags, implicit-def $rsi
    dead $edx = MOV32r0 implicit-def dead $eflags, implicit-def $rdx
    dead $ecx = MOV32r0 implicit-def dead $eflags, implicit-def $rcx
    dead $r8d = MOV32r0 implicit-def dead $eflags, implicit-def $r8
    PUSH64i32 -1, implicit-def $rsp, implicit $rsp :: (store (s64) into stack + 24)
    CFI_INSTRUCTION adjust_cfa_offset 8
    PUSH64i32 -1073741824, implicit-def $rsp, implicit $rsp :: (store (s64) into stack + 16)
    CFI_INSTRUCTION adjust_cfa_offset 8
    PUSH64i32 0, implicit-def $rsp, implicit $rsp :: (store (s64) into stack + 8)
    CFI_INSTRUCTION adjust_cfa_offset 8
    PUSH64i32 1, implicit-def $rsp, implicit $rsp :: (store (s64) into stack)
    CFI_INSTRUCTION adjust_cfa_offset 8
    CALL64pcrel32 target-flags(x86-plt) @check_bi, csr_64, implicit $rsp, implicit $ssp, implicit $edi, implicit $rsi, implicit $rdx, implicit $rcx, implicit $r8, implicit-def $rsp, implicit-def $ssp, implicit-def dead $ax
    ADJCALLSTACKUP64 32, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
    MOV64mi32 $noreg, 1, $noreg, 8, $noreg, 0 :: (volatile store (s64) into `ptr null` + 8, basealign 4294967296)
    MOV64mi32 $noreg, 1, $noreg, 0, $noreg, 0 :: (volatile store (s64) into `ptr null`, align 4294967296)
    ADJCALLSTACKDOWN64 32, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
    DBG_VALUE i256 0, $noreg, !12, !DIExpression(), debug-location !14
    renamable $xmm0 = V_SET0
    MOVUPSmr $rsp, 1, $noreg, 16, $noreg, renamable $xmm0 :: (store (s128), align 8)
    MOVUPSmr $rsp, 1, $noreg, 0, $noreg, killed renamable $xmm0 :: (store (s128), align 8)
    $edi = MOV32r0 implicit-def dead $eflags
    dead $esi = MOV32r0 implicit-def dead $eflags, implicit-def $rsi
    dead $edx = MOV32r0 implicit-def dead $eflags, implicit-def $rdx
    dead $ecx = MOV32r0 implicit-def dead $eflags, implicit-def $rcx
    dead $r8d = MOV32r0 implicit-def dead $eflags, implicit-def $r8
    CALL64pcrel32 target-flags(x86-plt) @check_bi, csr_64, implicit $rsp, implicit $ssp, implicit $edi, implicit $rsi, implicit $rdx, implicit $rcx, implicit $r8, implicit-def $rsp, implicit-def $ssp, implicit-def dead $ax
    ADJCALLSTACKUP64 32, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
    renamable $rax = MOV64ri 9223372036854775807
    renamable $rax = AND64rr killed renamable $rax, killed renamable $r12, implicit-def dead $eflags
    MOV64mr renamable $r13, 1, $noreg, 8, $noreg, killed renamable $rax :: (store (s64) into %ir.retval.i85 + 8, basealign 16)
    MOV64mr killed renamable $r13, 1, $noreg, 0, $noreg, killed renamable $rbp :: (store (s64) into %ir.retval.i85, align 16)
    ADJCALLSTACKDOWN64 32, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
    renamable $xmm0 = V_SET0
    MOVUPSmr $rsp, 1, $noreg, 16, $noreg, renamable $xmm0 :: (store (s128), align 8)
    MOVUPSmr $rsp, 1, $noreg, 0, $noreg, killed renamable $xmm0 :: (store (s128), align 8)
    $edi = MOV32r0 implicit-def dead $eflags
    $rsi = COPY killed renamable $r15
    $rdx = COPY killed renamable $r14
    $rcx = COPY killed renamable $rbx
    renamable $r8 = MOV64rm %fixed-stack.0, 1, $noreg, 0, $noreg :: (load (s64) from %fixed-stack.0, align 16)
    CALL64pcrel32 target-flags(x86-plt) @check_bi, csr_64, implicit $rsp, implicit $ssp, implicit $edi, implicit $rsi, implicit $rdx, implicit $rcx, implicit $r8, implicit-def $rsp, implicit-def $ssp, implicit-def dead $ax
    ADJCALLSTACKUP64 32, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
    ADJCALLSTACKDOWN64 32, 0, 32, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
    $edi = MOV32r0 implicit-def dead $eflags
    dead $esi = MOV32r0 implicit-def dead $eflags, implicit-def $rsi
    dead $edx = MOV32r0 implicit-def dead $eflags, implicit-def $rdx
    dead $ecx = MOV32r0 implicit-def dead $eflags, implicit-def $rcx
    dead $r8d = MOV32r0 implicit-def dead $eflags, implicit-def $r8
    PUSH64i32 -1, implicit-def $rsp, implicit $rsp :: (store (s64) into stack + 24)
    CFI_INSTRUCTION adjust_cfa_offset 8
    PUSH64i32 -16384, implicit-def $rsp, implicit $rsp :: (store (s64) into stack + 16)
    CFI_INSTRUCTION adjust_cfa_offset 8
    PUSH64i32 0, implicit-def $rsp, implicit $rsp :: (store (s64) into stack + 8)
    CFI_INSTRUCTION adjust_cfa_offset 8
    PUSH64i32 1, implicit-def $rsp, implicit $rsp :: (store (s64) into stack)
    CFI_INSTRUCTION adjust_cfa_offset 8
    CALL64pcrel32 target-flags(x86-plt) @check_bi, csr_64, implicit $rsp, implicit $ssp, implicit $edi, implicit $rsi, implicit $rdx, implicit $rcx, implicit $r8, implicit-def $rsp, implicit-def $ssp, implicit-def dead $ax
    ADJCALLSTACKUP64 32, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
    dead $eax = MOV32r0 implicit-def dead $eflags, implicit-def $ax
    RET 0, $ax

...

```
If I compile it using `llc foo.mir -run-pass prologepilog -o foo1.mir` and then comment out the line with DBG_VALUE and run `llc foo.mir -run-pass prologepilog -o foo2.mir`, then those foo1.mir and foo2.mir files will diff like this:

```
178a179
>     DBG_VALUE i256 0, $noreg, !12, !DIExpression(), debug-location !14
205c206
<     renamable $r8 = MOV64rm $rsp, 1, $noreg, 96, $noreg :: (load (s64) from %fixed-stack.6, align 16)
---
>     renamable $r8 = MOV64rm $rsp, 1, $noreg, 128, $noreg :: (load (s64) from %fixed-stack.6, align 16)
```

https://github.com/llvm/llvm-project/pull/114265


More information about the llvm-commits mailing list