[clang][CodeGen] Do not emit NVRO debug helper when not emitting debug info

[nick]

Summary

When no debug information is emitted there is no point in emitting a hack introduced in D63361.

[rnk]

We try to uphold the invariant that -g flags do not affect generated code, so I don't think we should do this.

[nick]

In D99107#2642451, @rnk wrote:

We try to uphold the invariant that -g flags do not affect generated code, so I don't think we should do this.

Even under -O0? I could change the check to always emit on -O0. With optimizations enabled there is no change in generated code, DSE+DCE removes it anyway.

[nick]

Is this hack actually needed? I could not reproduce a problem with https://bugs.chromium.org/p/chromium/issues/detail?id=860398#c13 repro, the breakpoint fires for me and I see the variable.

The difference with the hack and without, using clang.exe src.cpp -S -masm=intel -O2 -g -o out.asm is

diff
---
+++
@@ -26,10 +26,10 @@
        .seh_stackalloc 32
        .seh_endprologue
 .Ltmp0:
-       #DEBUG_VALUE: test0:x <- [DW_OP_deref] $rcx
+       #DEBUG_VALUE: test0:x <- [$rcx+0]
        mov     rsi, rcx
 .Ltmp1:
-       #DEBUG_VALUE: test0:x <- [DW_OP_deref] $rsi
+       #DEBUG_VALUE: test0:x <- [$rsi+0]
        .cv_loc 0 1 9 0                         # nrvox.cpp:9:0
        call    "??0X@@QEAA@XZ"
        .cv_loc 0 1 10 0                        # nrvox.cpp:10:0

[dblaikie]

In D99107#2642451, @rnk wrote:

We try to uphold the invariant that -g flags do not affect generated code, so I don't think we should do this.

+1

[rnk]

In D99107#2642599, @nick wrote:

Is this hack actually needed? I could not reproduce a problem with https://bugs.chromium.org/p/chromium/issues/detail?id=860398#c13 repro, the breakpoint fires for me and I see the variable.

The difference with the hack and without, using clang.exe src.cpp -S -masm=intel -O2 -g -o out.asm is

You enabled optimizations, so I wouldn't expect there to be any difference. The main purpose of taking the sret pointer and storing it into a local alloca is to make it easier to find in unoptimized builds. Otherwise we have this variable value living in the first register parameter, and at the time, the backend would quickly lose track of it. It's always safer to anchor your variable location info in memory.

clang/lib/CodeGen/CodeGenFunction.cpp

if (!endsWithReturn(D))
       ++NumReturnExprs;
   } else if (CurFnInfo->getReturnInfo().getKind() == ABIArgInfo::Indirect) {
     // Indirect return; emit returned value directly into sret slot.
     // This reduces code size, and affects correctness in C++.
     auto AI = CurFn->arg_begin();
     if (CurFnInfo->getReturnInfo().isSRetAfterThis())
       ++AI;
     ReturnValue = Address(&*AI, CurFnInfo->getReturnInfo().getIndirectAlign());
-    if (!CurFnInfo->getReturnInfo().getIndirectByVal()) {
+    if (getDebugInfo() && CGM.getCodeGenOpts().hasReducedDebugInfo() &&
+        !CurFnInfo->getReturnInfo().getIndirectByVal()) {
       ReturnValuePointer =
           CreateDefaultAlignTempAlloca(Int8PtrTy, "result.ptr");
       Builder.CreateStore(Builder.CreatePointerBitCastOrAddrSpaceCast(
                               ReturnValue.getPointer(), Int8PtrTy),
                           ReturnValuePointer);
     }
   } else if (CurFnInfo->getReturnInfo().getKind() == ABIArgInfo::InAlloca &&
              !hasScalarEvaluationKind(CurFnInfo->getReturnType())) {

clang/test/CodeGen/arm64-microsoft-arguments.cpp

 S3 f3() {
   S3 x;
   return func3(x);
 }
 
 // Pass and return aggregate (of size < 16 bytes) with non-trivial destructor.
 // Passed directly but returned indirectly.
 // CHECK: define {{.*}} void {{.*}}f4{{.*}}(%struct.S4* inreg noalias sret(%struct.S4) align 4 %agg.result)
-// CHECK: call void {{.*}}func4{{.*}}(%struct.S4* inreg sret(%struct.S4) align 4 %agg.result, [2 x i64] %5)
+// CHECK: call void {{.*}}func4{{.*}}(%struct.S4* inreg sret(%struct.S4) align 4 %agg.result, [2 x i64] %4)
 struct S4 {
   int a[3];
   ~S4();
 };
 
 S4 func4(S4 x);
 S4 f4() {
   S4 x;
   return func4(x);
 }
 
 // Pass and return from instance method called from instance method.
 // CHECK: define {{.*}} void @{{.*}}bar@Q1{{.*}}(%class.Q1* {{[^,]*}} %this, %class.P1* inreg noalias sret(%class.P1) align 1 %agg.result)
-// CHECK: call void {{.*}}foo@P1{{.*}}(%class.P1* {{[^,]*}} %ref.tmp, %class.P1* inreg sret(%class.P1) align 1 %agg.result, i8 %1)
+// CHECK: call void {{.*}}foo@P1{{.*}}(%class.P1* {{[^,]*}} %ref.tmp, %class.P1* inreg sret(%class.P1) align 1 %agg.result, i8 %0)
 
 class P1 {
 public:
   P1 foo(P1 x);
 };
 
 class Q1 {
 public: