From 992e21eeeef101b4fd68053d5f6a19d21b655288 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse@sony.com>
Date: Tue, 24 Aug 2021 10:14:37 +0100
Subject: [PATCH] [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
---
 llvm/lib/Target/X86/X86FloatingPoint.cpp                 |  7 ++++---
 .../MIR/InstrRef/x86-fp-stackifier-drop-locations.mir    | 16 ++++++++++++----
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/llvm/lib/Target/X86/X86FloatingPoint.cpp b/llvm/lib/Target/X86/X86FloatingPoint.cpp
index e0f30f0..67314b1 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 a3560bb..d030f7b 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
-- 
2.7.4