New issue
Advanced search Search tips
Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 636111



Sign in to add a comment

Move away from Clang's -instcombine-lower-dbg-declare=0

Project Member Reported by h...@chromium.org, Sep 15 2017

Issue description

Filing this to track the work of making the default Clang passes generate good enough debug info that we don't need this flag.

See e.g.  Issue 753736 , Issue 753934,  Issue 761633  for the problems we were seeing.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/32750c1251012624a87fb97098bd0f6a60ffa8ed

commit 32750c1251012624a87fb97098bd0f6a60ffa8ed
Author: Hans Wennborg <hans@chromium.org>
Date: Fri Sep 15 23:10:14 2017

Clang: enable -instcombine-lower-dbg-declare=1

This enables better optimized debug info for address-taken values
at the cost of potentially inaccurate debug info in some situations.

We believe turning this on is the right trade-off for developers
debugging Chromium on all platforms, but especially on Windows where
values that are available in MSVC's debug info would otherwise show
as unavailable when using Clang.

The goal (crbug.com/765793) is to make Clang's optimized
debug info good enough in all cases that this compromise is not
necessary.

Note: This requires Clang r313108 or later.

Bug:  761633 ,  753736 , 765793
Change-Id: Ia0ed38c499fac282a3c8ab27c5fd5571fdfab84d
Reviewed-on: https://chromium-review.googlesource.com/669314
Commit-Queue: Hans Wennborg <hans@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502428}
[modify] https://crrev.com/32750c1251012624a87fb97098bd0f6a60ffa8ed/build/config/compiler/BUILD.gn

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5487d51f9175a69613e7b715be437ece3c1d88e5

commit 5487d51f9175a69613e7b715be437ece3c1d88e5
Author: Xiaoqian Dai <xdai@chromium.org>
Date: Sat Sep 16 03:54:06 2017

Revert "Clang: enable -instcombine-lower-dbg-declare=1"

This reverts commit 32750c1251012624a87fb97098bd0f6a60ffa8ed.

Reason for revert: This CL breaks most of the informational builds https://uberchromegw.corp.google.com/i/chromeos.chrome/waterfall?reload=300. See an example here: https://uberchromegw.corp.google.com/i/chromeos.chrome/builders/veyron_minnie-tot-chrome-pfq-informational/builds/6039

Original change's description:
> Clang: enable -instcombine-lower-dbg-declare=1
> 
> This enables better optimized debug info for address-taken values
> at the cost of potentially inaccurate debug info in some situations.
> 
> We believe turning this on is the right trade-off for developers
> debugging Chromium on all platforms, but especially on Windows where
> values that are available in MSVC's debug info would otherwise show
> as unavailable when using Clang.
> 
> The goal (crbug.com/765793) is to make Clang's optimized
> debug info good enough in all cases that this compromise is not
> necessary.
> 
> Note: This requires Clang r313108 or later.
> 
> Bug:  761633 ,  753736 , 765793
> Change-Id: Ia0ed38c499fac282a3c8ab27c5fd5571fdfab84d
> Reviewed-on: https://chromium-review.googlesource.com/669314
> Commit-Queue: Hans Wennborg <hans@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#502428}

TBR=thakis@chromium.org,hans@chromium.org,rnk@chromium.org

Change-Id: I4fea2ab8ec9571d4afb66cd3912c5975e0b9395c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  761633 ,  753736 , 765793
Reviewed-on: https://chromium-review.googlesource.com/669825
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502484}
[modify] https://crrev.com/5487d51f9175a69613e7b715be437ece3c1d88e5/build/config/compiler/BUILD.gn

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/074fa51f85b15ad2e2d6c804a03825d48dbeb063

commit 074fa51f85b15ad2e2d6c804a03825d48dbeb063
Author: Hans Wennborg <hans@chromium.org>
Date: Mon Sep 18 19:43:09 2017

Reland "Clang: enable -instcombine-lower-dbg-declare=1"

Trying harder to exclude CrOS build configs that don't use
Chromium's Clang.

Note to CrOS folks: If this breaks build configs that are not
tested upstream, please ping me and/or llozano before reverting.

This is a reland of 32750c1251012624a87fb97098bd0f6a60ffa8ed
Original change's description:
> Clang: enable -instcombine-lower-dbg-declare=1
>
> This enables better optimized debug info for address-taken values
> at the cost of potentially inaccurate debug info in some situations.
>
> We believe turning this on is the right trade-off for developers
> debugging Chromium on all platforms, but especially on Windows where
> values that are available in MSVC's debug info would otherwise show
> as unavailable when using Clang.
>
> The goal (crbug.com/765793) is to make Clang's optimized
> debug info good enough in all cases that this compromise is not
> necessary.
>
> Note: This requires Clang r313108 or later.
>
> Bug:  761633 ,  753736 , 765793
> Change-Id: Ia0ed38c499fac282a3c8ab27c5fd5571fdfab84d
> Reviewed-on: https://chromium-review.googlesource.com/669314
> Commit-Queue: Hans Wennborg <hans@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#502428}

TBR=thakis

Bug:  761633 ,  753736 , 765793
Change-Id: I2c3670873f1bd1d75cf1c41191b170983f41e0d0
Reviewed-on: https://chromium-review.googlesource.com/671523
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502646}
[modify] https://crrev.com/074fa51f85b15ad2e2d6c804a03825d48dbeb063/build/config/compiler/BUILD.gn

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/308178955214516b259876b56fff9bfc78b97e7f

commit 308178955214516b259876b56fff9bfc78b97e7f
Author: Hans Wennborg <hans@chromium.org>
Date: Mon Sep 18 20:49:45 2017

clang: Disable -instcombine-lower-dbg-declare=1 for CrOS builds

It breaks CrOS builds which use a different version of Clang, which
doesn't support this flag.

This is a stop-gap fix; Linux CrOS builds do use the Chromium version of
Clang, and we want them to have this flag, but there doesn't seem to be
a straight-forward way to distinguish between these compiler versions
from the build system.

Landing to unbreak the build for now until a better solution is found.

TBR=llozano

Bug: 766311,765793
Change-Id: I3e040161f6987b502b28e97f41fcde2d464da72c
Reviewed-on: https://chromium-review.googlesource.com/671810
Reviewed-by: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502668}
[modify] https://crrev.com/308178955214516b259876b56fff9bfc78b97e7f/build/config/compiler/BUILD.gn

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e1cd7e67e4f7039c1922ccf11521e36fdbcb85fd

commit e1cd7e67e4f7039c1922ccf11521e36fdbcb85fd
Author: Hans Wennborg <hans@chromium.org>
Date: Mon Sep 18 21:52:24 2017

clang: Disable -instcombine-lower-dbg-declare=1 for CrOS builds, take 2

The previous attempt (#502668) didn't actually work.

TBR=llozano

Bug: 766311,765793
Change-Id: I3ea9c11b9c2f320e2989723f1d22f4eac75e6583
Reviewed-on: https://chromium-review.googlesource.com/671471
Reviewed-by: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502685}
[modify] https://crrev.com/e1cd7e67e4f7039c1922ccf11521e36fdbcb85fd/build/config/compiler/BUILD.gn

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1a6a85d8c64b56eaad45415827a34010326a78f4

commit 1a6a85d8c64b56eaad45415827a34010326a78f4
Author: Hans Wennborg <hans@chromium.org>
Date: Wed Oct 18 01:08:16 2017

Clang: disable the llvm.dbg.declare -> value instcombine for realz

It turns out the first attempt (#502428) got the flag backwards.
*headdesks*

Bug:  761633 ,765793, 775258 
Change-Id: I6ef2203822c4b0a5d04d6c8d564e0d347b2d104d
Reviewed-on: https://chromium-review.googlesource.com/724263
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509640}
[modify] https://crrev.com/1a6a85d8c64b56eaad45415827a34010326a78f4/build/config/compiler/BUILD.gn

Comment 7 by thakis@chromium.org, Oct 18 2017

Summary: Move away from Clang's -instcombine-lower-dbg-declare=0 (was: Move away from Clang's -instcombine-lower-dbg-declare=1)

Sign in to add a comment