Page MenuHomePhabricator

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

Authored by nick on Mar 22 2021, 12:59 PM.

Details

Summary

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

Diff Detail

Unit TestsFailed

TimeTest
50 msx64 debian > Clang.CodeGenCXX::aix-alignment.cpp
Script: -- : 'RUN: at line 2'; /mnt/disks/ssd0/agent/llvm-project/build/bin/clang -cc1 -internal-isystem /mnt/disks/ssd0/agent/llvm-project/build/lib/clang/13.0.0/include -nostdsysteminc -triple powerpc-unknown-aix -emit-llvm -o - -x c++ /mnt/disks/ssd0/agent/llvm-project/clang/test/CodeGenCXX/aix-alignment.cpp | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/clang/test/CodeGenCXX/aix-alignment.cpp --check-prefixes=AIX,AIX32
80 msx64 windows > Clang.CodeGenCXX::aix-alignment.cpp
Script: -- : 'RUN: at line 2'; c:\ws\w16c2-2\llvm-project\premerge-checks\build\bin\clang.exe -cc1 -internal-isystem c:\ws\w16c2-2\llvm-project\premerge-checks\build\lib\clang\13.0.0\include -nostdsysteminc -triple powerpc-unknown-aix -emit-llvm -o - -x c++ C:\ws\w16c2-2\llvm-project\premerge-checks\clang\test\CodeGenCXX\aix-alignment.cpp | c:\ws\w16c2-2\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w16c2-2\llvm-project\premerge-checks\clang\test\CodeGenCXX\aix-alignment.cpp --check-prefixes=AIX,AIX32

Event Timeline

nick requested review of this revision.Mar 22 2021, 12:59 PM
nick created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMar 22 2021, 12:59 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
rnk added a comment.Mar 22 2021, 1:08 PM

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

nick added a comment.Mar 22 2021, 1:57 PM
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 added a comment.Mar 22 2021, 2:32 PM

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
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 added a comment.Mar 23 2021, 1:26 PM

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.