Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1170)

Issue 2144233003: [gn] Restore strict-aliasing for gn on mac

Created:
4 years, 5 months ago by Michael Achenbach
Modified:
4 years, 5 months ago
CC:
v8-reviews_googlegroups.com, Dirk Pranke, Robert Sesek
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[gn] Restore strict-aliasing for gn on mac BUG=chromium:474921, chromium:626064 NOTRY=true

Patch Set 1 #

Patch Set 2 : [gn] Restore strict-aliasing for gn on mac #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -0 lines) Patch
M BUILD.gn View 1 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (4 generated)
Michael Achenbach
PTAL. I'm not so sure about this change. This would restore what we had in ...
4 years, 5 months ago (2016-07-14 12:43:42 UTC) #4
Nico
Adding this just on mac looks weird to me. I think we generally should try ...
4 years, 5 months ago (2016-07-14 13:37:30 UTC) #6
Michael Achenbach
> From what I understand, v8 previously only used the flag in standalone builds, > ...
4 years, 5 months ago (2016-07-14 13:44:24 UTC) #7
Michael Achenbach
4 years, 5 months ago (2016-07-15 08:34:21 UTC) #8
Correction: -fno-strict-aliasing was also set for mac before:
https://cs.chromium.org/chromium/src/build/common.gypi?q=strict-aliasing&sq=p...

It looks like in gyp, for v8 objects on mac we passed -fno-strict-aliasing and
-fstrict-aliasing. We'd do the same with this change. I'm hesitating to remove
-fno-strict-aliasing for v8 as this might really change behavior.

I suggest we keep the chromium wide default -fno-strict-aliasing and submit a
bug for analyzing if
1) -fstrict-aliasing gives a performance improvement for v8 and
2) -fstrict-aliasing doesn't expose any undefined behavior in v8 code.

Powered by Google App Engine
This is Rietveld 408576698