[DebugInfo] Enable the call site parameter feature by default

[alok]

Summary

This works with debug entry value feature, which works by default.
Without call site parameter feature, debug entry value feature is of no use.
With current changes, debuggers should be able to show optimized out parameters by default.

[alok]

Re-based after setting the parent patch.

[djtodoro]

clang/lib/Frontend/CompilerInvocation.cpp

1648

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.

llvm/test/DebugInfo/X86/callsitepar-fastisel.ll

12 ↗

(On Diff #332870)

I don't think this should be tested this way. We should be testing Driver itself.

[alok]

clang/lib/Frontend/CompilerInvocation.cpp

1648

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_64amd64__ -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 ?

llvm/test/DebugInfo/X86/callsitepar-fastisel.ll

12 ↗

(On Diff #332870)

Sure, I shall remove this test from here.

[alok]

Updated to address comments from @djtodoro

[probinson]

clang/lib/Frontend/CompilerInvocation.cpp

1648

I think we don't want this enabled by default at -O0. It doesn't run any optimizations that should eliminate parameters, so this is going to increase debug info size for no real benefit to the debugging experience.

[dblaikie]

clang/lib/Frontend/CompilerInvocation.cpp

1648

+1 to @probinson here. That's generally been the thinking behind why this was only enabled when optimizations are enabled. (there's some minor tradeoff here: call site descriptions in unoptimized code can still be useful if they call into optimized code, so this is a bit heuristical - assuming that all code will be compiled with the same optimization level)

If flang is in a different situation and enables some optimizations at -O0 that make call site parameter descriptions beneficial even there, then maybe we'll need some different way to differentiate optimizations V no optimizations for the purpose of this feature.

[alok]

clang/lib/Frontend/CompilerInvocation.cpp

1648

Thanks Paul for the comment. I shall probably add an option to enable in such cases which should remain off by default, Flang driver can pass that to compiler to turn it on.

1648

Thanks David for your explanation and thoughts. Please also look at trigger to this https://reviews.llvm.org/D99160

It is needed in a Fortran case. I shall probably drop to enable it by default and add an option which by default is disabled and Fortran driver can pass that to make use of it.

[dblaikie]

clang/lib/Frontend/CompilerInvocation.cpp

1648

Sounds like @djtodoro is suggesting in https://reviews.llvm.org/D99160#2666106 that the "Bug" to fix here is the presence of DW_OP_call_value at -O0, it seems that could be an issue for the existing/original use of call_site_parameter/call_value (in C++/clang) as it would be for flang - so the fix should probably not be flang-specific if the issue isn't flang-specific. Understanding why we produce DW_OP_call_values at -O0, whether we can/should avoid that, what it costs (how much larger the debug info gets by adding DW_TAG_call_sites at -O0) would be necessary to determine the right path forward here.

[alok]

clang/lib/Frontend/CompilerInvocation.cpp

1648

Thanks David for clarification. I shall look at it and get back.

clang/include/clang/Basic/CodeGenOptions.def

 CODEGENOPT(DisableO0ImplyOptNone , 1, 0) ///< Don't annonate function with optnone at O0
 CODEGENOPT(ExperimentalStrictFloatingPoint, 1, 0) ///< Enables the new, experimental
                                                   ///< strict floating point.
 CODEGENOPT(EnableNoundefAttrs, 1, 0) ///< Enable emitting `noundef` attributes on IR call arguments and return values
 CODEGENOPT(LegacyPassManager, 1, 0) ///< Use the legacy pass manager.
 CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
                                    ///< pass manager.
 CODEGENOPT(DisableRedZone    , 1, 0) ///< Set when -mno-red-zone is enabled.
-CODEGENOPT(EmitCallSiteInfo, 1, 0) ///< Emit call site info only in the case of
-                                   ///< '-g' + 'O>0' level.
+CODEGENOPT(EmitCallSiteInfo, 1, 0) ///< Emit call site info.
 CODEGENOPT(EnableDIPreservationVerify, 1, 0) ///< Enable di preservation verify
                                              ///< each (it means check
                                              ///< the original debug info
                                              ///< metadata preservation).
 CODEGENOPT(IndirectTlsSegRefs, 1, 0) ///< Set when -mno-tls-direct-seg-refs
                                      ///< is specified.
 CODEGENOPT(DisableTailCalls  , 1, 0) ///< Do not emit tail calls.
 CODEGENOPT(NoEscapingBlockTailCalls, 1, 0) ///< Do not emit tail calls from

clang/lib/Frontend/CompilerInvocation.cpp

Opts.CoveragePrefixMap.insert(
         {std::string(Split.first), std::string(Split.second)});
   }
 
   const llvm::Triple::ArchType DebugEntryValueArchs[] = {
       llvm::Triple::x86, llvm::Triple::x86_64, llvm::Triple::aarch64,
       llvm::Triple::arm, llvm::Triple::armeb, llvm::Triple::mips,
       llvm::Triple::mipsel, llvm::Triple::mips64, llvm::Triple::mips64el};
 
-  if (Opts.OptimizationLevel > 0 && Opts.hasReducedDebugInfo() &&
+  if (Opts.hasReducedDebugInfo() &&
       llvm::is_contained(DebugEntryValueArchs, T.getArch()))
     Opts.EmitCallSiteInfo = true;
 
   if (!Opts.EnableDIPreservationVerify && Opts.DIBugsReportFilePath.size()) {
     Diags.Report(diag::warn_ignoring_verify_debuginfo_preserve_export)
         << Opts.DIBugsReportFilePath;
     Opts.DIBugsReportFilePath = "";
   }

llvm/include/llvm/Target/TargetOptions.h

public:
     /// Emit basic blocks into separate sections.
     BasicBlockSection BBSections = BasicBlockSection::None;
 
     /// Memory Buffer that contains information on sampled basic blocks and used
     /// to selectively generate basic block sections.
     std::shared_ptr<MemoryBuffer> BBSectionsFuncListBuf;
 
     /// The flag enables call site info production. It is used only for debug
-    /// info, and it is restricted only to optimized code. This can be used for
-    /// something else, so that should be controlled in the frontend.
+    /// info. This can be used for something else, so that should be controlled
+    /// in the frontend.
     unsigned EmitCallSiteInfo : 1;
     /// Set if the target supports the debug entry values by default.
     unsigned SupportsDebugEntryValues : 1;
     /// When set to true, the EnableDebugEntryValues option forces production
     /// of debug entry values even if the target does not officially support
     /// it. Useful for testing purposes only. This flag should never be checked
     /// directly, always use \ref ShouldEmitDebugEntryValues instead.
      unsigned EnableDebugEntryValues : 1;