[PATCH] D36117: [DebugInfo] Don't turn dbg.declare into DBG_VALUE for static allocas

Adrian Prantl via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 31 16:05:20 PDT 2017


aprantl added a comment.

Code LGTM, I have some comments for the testcases inline.



================
Comment at: llvm/test/DebugInfo/WebAssembly/dbg-declare.ll:3
 ; RUN: llc < %s -verify-machineinstrs -mtriple=wasm32-unknown-unknown-wasm -fast-isel | FileCheck --check-prefix=CHECK-FAST %s
-; CHECK: #DEBUG_VALUE: decode:i <- [%vreg
-; CHECK: #DEBUG_VALUE: decode:v <- [%vreg
 ; CHECK: DW_TAG_variable
 ; CHECK-FAST: DW_TAG_variable
----------------
When removing the check for the DEBUG_VALUE, could you add a check for the actual variable location being emitted via the MMI side table? `llc -filetype=obj | llvm-dwarfdump -` and a check for a DW_AT_name / DW_AT_location pair should do the trick.


================
Comment at: llvm/test/DebugInfo/X86/dbg-declare-alloca.ll:7
+
+; CHECK-LABEL: use_dbg_declare:
+; CHECK-NOT: #DEBUG_VALUE
----------------
I would prefer something that checks the dwarf or assembler output for an actual debug location to avoid future bitrot.


================
Comment at: llvm/test/DebugInfo/X86/dbg-declare-alloca.ll:31
+
+attributes #0 = { noinline nounwind uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone speculatable }
----------------
We usually strip out all non-essential attributes. (Usually at least everything in quotes :-)


================
Comment at: llvm/test/DebugInfo/X86/dbg-declare.ll:5
+; CHECK-LABEL: _foo:
+; CHECK-NOT: #DEBUG_VALUE
+
----------------
ditto here, checking that we still emit the location would be even better


https://reviews.llvm.org/D36117





More information about the llvm-commits mailing list