[llvm] 992e21e - [DebugInfo][InstrRef] Fix over-droppage of locations in X86FloatingPoint

[Jeremy Morse via llvm-commits]

Author: Jeremy Morse
Date: 2021-08-24T10:24:07+01:00
New Revision: 992e21eeeef101b4fd68053d5f6a19d21b655288

URL: https://github.com/llvm/llvm-project/commit/992e21eeeef101b4fd68053d5f6a19d21b655288
DIFF: https://github.com/llvm/llvm-project/commit/992e21eeeef101b4fd68053d5f6a19d21b655288.diff

LOG: [DebugInfo][InstrRef] Fix over-droppage of locations in X86FloatingPoint

Over in D105657, we started dropping instruction numbers (that become
variable locations) from call instructions, as we can't correctly represent
the x87 FP stack. Unfortunately, it turns out that the "special FP
instructions" that this pass transforms includes "every call instruction"
[0]. Thus, we've ended up dropping all return values from all calls. Ouch.

This patch adds a filter: only drop instruction numbers from calls if they
return something on the FP stack. Seeing how LLVM only allows a single
return value, this should drop instruction numbers on anything that returns
a float, and nothing else.

Rather than writing a new test, I've modified the original one to have a
positive and negative case: drop instruction number on a call with an
FP-stack modification, keep it on a plain call.

Differential Revision: https://reviews.llvm.org/D108580

Added: 
    

Modified: 
    llvm/lib/Target/X86/X86FloatingPoint.cpp
    llvm/test/DebugInfo/MIR/InstrRef/x86-fp-stackifier-drop-locations.mir

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/X86/X86FloatingPoint.cpp b/llvm/lib/Target/X86/X86FloatingPoint.cpp
index e0f30f0901710..67314b1dad442 100644
--- a/llvm/lib/Target/X86/X86FloatingPoint.cpp
+++ b/llvm/lib/Target/X86/X86FloatingPoint.cpp
@@ -1038,9 +1038,10 @@ void FPS::handleCall(MachineBasicBlock::iterator &I) {
   for (unsigned I = 0; I < N; ++I)
     pushReg(N - I - 1);
 
-  // Drop all variable values defined by this call -- we can't track them
-  // once they've been stackified.
-  I->dropDebugNumber();
+  // If this call has been modified, drop all variable values defined by it.
+  // We can't track them once they've been stackified.
+  if (STReturns)
+    I->dropDebugNumber();
 }
 
 /// If RET has an FP register use operand, pass the first one in ST(0) and

diff  --git a/llvm/test/DebugInfo/MIR/InstrRef/x86-fp-stackifier-drop-locations.mir b/llvm/test/DebugInfo/MIR/InstrRef/x86-fp-stackifier-drop-locations.mir
index a3560bb3a934e..d030f7b6b0db6 100644
--- a/llvm/test/DebugInfo/MIR/InstrRef/x86-fp-stackifier-drop-locations.mir
+++ b/llvm/test/DebugInfo/MIR/InstrRef/x86-fp-stackifier-drop-locations.mir
@@ -1,12 +1,11 @@
-# RUN: llc %s -run-pass=x86-codegen -o - -experimental-debug-variable-locations | FileCheck %s
+# RUN: llc %s -run-pass=x86-codegen -o - -experimental-debug-variable-locations | FileCheck %s --implicit-check-not=debug-instr-number
 #
 # The x87 FP instructions below have debug instr numbers attached -- but the
 # operands get rewritten when it's converted to stack-form. Rather than trying
 # to recover from this, drop any instruction numbers.
 #
-# CHECK-NOT: debug-instr-number
-# CHECK:     ADD_F64m
-# CHECK-NOT: debug-instr-number
+## We shouldn't drop numbers from _every_ call though:
+# CHECK:  CALLpcrel32 @ext, csr_32, implicit $esp, implicit $ssp, implicit-def $esp, implicit-def $ssp, debug-instr-number 101,
 #
 # Original program, command line 'clang ./test.c -O2 -g -m32 -o out.o -c'
 #
@@ -147,7 +146,16 @@ body:             |
     renamable $fp0 = nofpexcept ADD_Fp80 killed renamable $fp0, killed renamable $fp1, implicit-def dead $fpsw, implicit $fpcw, debug-instr-number 6, debug-location !21
     ST_FpP80m %stack.2, 1, $noreg, 0, $noreg, killed renamable $fp0, implicit-def $fpsw, implicit $fpcw, debug-instr-number 7 :: (store (s80) into %stack.2, align 4)
     ADJCALLSTACKDOWN32 0, 0, 0, implicit-def dead $esp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $esp, implicit $ssp, debug-location !22
+
+    ; This identical call (to the one below) does not touch the FP registers at
+    ; all. Therefore, it shouldn't have any debug instructions dropped.
+    CALLpcrel32 @ext, csr_32, implicit $esp, implicit $ssp, implicit-def $esp, implicit-def $ssp, debug-instr-number 101, debug-location !22
+
+    ; Original call: this should have it's location dropped, as it touches the
+    ; FP stack.
     CALLpcrel32 @ext, csr_32, implicit $esp, implicit $ssp, implicit-def $esp, implicit-def $ssp, implicit-def $fp0, debug-instr-number 100, debug-location !22
+
+
     ADJCALLSTACKUP32 0, 0, implicit-def dead $esp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $esp, implicit $ssp, debug-location !22
     renamable $fp1 = LD_Fp80m %stack.2, 1, $noreg, 0, $noreg, implicit-def $fpsw, implicit $fpcw :: (load (s80) from %stack.2, align 4)
     renamable $fp0 = nofpexcept MUL_Fp80 killed renamable $fp1, killed renamable $fp0, implicit-def dead $fpsw, implicit $fpcw, debug-instr-number 8, debug-location !23