[PATCH] D99238: [DebugInfo] Enable the call site parameter feature by default

Thu Apr 1 03:46:09 PDT 2021

alok added inline comments.

================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1645

-  if (Opts.OptimizationLevel > 0 && Opts.hasReducedDebugInfo() &&
+  if (Opts.hasReducedDebugInfo() &&
       llvm::is_contained(DebugEntryValueArchs, T.getArch()))
----------------
djtodoro wrote:
> I am a bit confused now :)
> Is `flang` using this clang's library?
> I think we should be careful here, since I am not sure C/C++ -O0 code has any benefits of using this, but the DWARF size might be increased insanely.
Classic flang calls same library. Below is the verbose output of "flang" command.
**flang** -g -O0 -v vla-sub.f90
 "/tmp/bin/**flang1**" vla-sub.f90 -x 47 0x10000000 -opt 0 -terse 1 -inform warn -nohpf -nostatic -cmdline "'+flang -g -O0 -v vla-sub.f90'" -inform warn -disable-vectorize-pragmas -x 120 0x1000000 -debug -y 129 2 -x 19 0x400000 -quad -x 7 0x100000 -x 68 0x1 -x 59 4 -x 15 2 -x 49 0x400004 -x 51 0x20 -x 57 0x48 -x 58 0x10000 -x 124 0x1000 -tp px -x 57 0xfb0000 -x 58 0x78031040 -x 47 0x08 -x 48 4608 -x 49 0x100 -stdinc /tmp/bin/../include:/usr/local/include:/tmp/d/lib/clang/13.0.0/include:/usr/include/x86_64-linux-gnu:/include:/usr/include -def unix -def __unix -def __unix__ -def linux -def __linux -def __linux__ -def __NO_MATH_INLINES -def __LP64__ -def __LONG_MAX__=9223372036854775807L -def "__SIZE_TYPE__=unsigned long int" -def "__PTRDIFF_TYPE__=long int" -def __x86_64 -def __x86_64__ -def __amd_64__amd64__ -def __k8 -def __k8__ -def __THROW= -def __extension__= -def __PGLLVM__ -freeform -vect 48 -x 54 1 -x 70 0x40000000 -y 163 0xc0000000 -x 189 0x10 -stbfile vla-sub-051e06.stb -modexport vla-sub-051e06.cmod -modindex vla-sub-051e06.cmdx -output vla-sub-051e06.ilm
 "/tmp/bin/**flang2**" vla-sub-051e06.ilm -y 129 2 -x 6 0x100 -x 42 0x400000 -y 129 4 -x 129 0x400 -y 216 1 -ieee 1 -fn vla-sub.f90 -opt 0 -terse 1 -inform warn -cmdline "'+flang -g -O0 -v vla-sub.f90'" -inform warn -disable-vectorize-pragmas -x 120 0x1000000 -debug -y 129 2 -x 68 0x1 -x 51 0x20 -x 119 0xa10000 -x 122 0x40 -x 123 0x1000 -x 127 4 -x 127 17 -x 19 0x400000 -x 28 0x40000 -x 120 0x10000000 -x 70 0x8000 -x 122 1 -x 125 0x20000 -x 164 0x800000 -quad -x 59 4 -tp px -x 120 0x1000 -x 124 0x1400 -y 15 2 -x 57 0x3b0000 -x 58 0x48000000 -x 49 0x100 -astype 0 -x 183 4 -x 121 0x800 -x 54 0x10 -x 70 0x40000000 -x 249 1023 -x 124 1 -y 163 0xc0000000 -x 189 0x10 -y 189 0x4000000 -x 183 0x10 -stbfile vla-sub-051e06.stb -asm /tmp/vla-sub-051e06.ll
 "/tmp/bin/**clang-12**" -cc1 -triple x86_64-unknown-linux-gnu -emit-obj -mrelax-all --mrelax-relocations -disable-free -main-file-name vla-sub.f90 -mrelocation-model static -mframe-pointer=all -fmath-errno -fno-rounding-math -mconstructor-aliases -munwind-tables -target-cpu x86-64 -tune-cpu generic -debug-info-kind=limited -dwarf-version=4 -debugger-tuning=gdb -v -fcoverage-compilation-dir=/tmp/build.shared.d -resource-dir /tmp/lib/clang/13.0.0 -O0 -fdebug-compilation-dir=/tmp/build.shared.d -ferror-limit 19 -fgnuc-version=4.2.1 -fcolor-diagnostics -itodcalls -itodcallsbyclone -faddrsig -D__GCC_HAVE_DWARF2_CFI_ASM=1 -o /tmp/vla-sub-cdf94f.o -x ir /tmp/vla-sub-051e06.ll
clang -cc1 version 13.0.0 based upon LLVM 13.0.0git default target x86_64-unknown-linux-gnu
 "/tmp/bin/**ld.lld**" -z relro --hash-style=gnu --eh-frame-hdr -m elf_x86_64 -dynamic-linker /lib64/ld-linux-x86-64.so.2 -o a.out /usr/lib/x86_64-linux-gnu/crt1.o /usr/lib/x86_64-linux-gnu/crti.o /usr/lib/gcc/x86_64-linux-gnu/7.5.0/crtbegin.o -L/usr/lib/gcc/x86_64-linux-gnu/7.5.0 -L/lib/x86_64-linux-gnu -L/lib/../lib64 -L/usr/lib/x86_64-linux-gnu -L/usr/lib/gcc/x86_64-linux-gnu/7.5.0/../../.. -L/home/alok/amdllvm/install/d/bin/../lib -L/lib -L/usr/lib /tmp/vla-sub-cdf94f.o -lflangmain -lflang -lflangrti -lpgmath -lquadmath -lompstub -lm -lrt -lpthread -rpath /tmp/bin/../lib -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed /usr/lib/gcc/x86_64-linux-gnu/7.5.0/crtend.o /usr/lib/x86_64-linux-gnu/crtn.o

Yes, I agree. I think there should be cases when even in C/C++ code this will be beneficial but not sure how frequent it shall be. We need to think about trade-off. Do you think it should be enabled only for Fortran or a command line option should be introduced to enable this otherwise it should disabled by default ? 

================
Comment at: llvm/test/DebugInfo/X86/callsitepar-fastisel.ll:12

+;; This test checks if DW_TAG_GNU_call_site_parameter is generated for option -O0
+;RUN: clang -cc1 -triple x86_64-unknown-linux-gnu -emit-obj  -main-file-name %s -debug-info-kind=limited -dwarf-version=4 -debugger-tuning=gdb -O0 -x ir %s -o - | llvm-dwarfdump - | FileCheck %s --check-prefix=CLANG
----------------
djtodoro wrote:
> I don't think this should be tested this way. We should be testing `Driver` itself.
Sure, I shall remove this test from here.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99238/new/

https://reviews.llvm.org/D99238