Issue 14355 in skia: Simplify SkOpts compilation

1 view
Skip to first unread message

brian… via monorail

unread,
May 31, 2023, 1:46:29 PM5/31/23
to bu...@skia.org
Status: Accepted
Owner: brian...@google.com
CC: kjlu...@google.com
Priority: Medium
Type: Defect

New issue 14355 by brian...@google.com: Simplify SkOpts compilation
https://bugs.chromium.org/p/skia/issues/detail?id=14355

Move from using command-line flags to build SkOpts code, to controlling the target architecture within the translation unit. Following that, investigate slicing the logic on the other (functionality) axis, rather than the architecture axis. This would let us build (for example), all of the "swizzler" code as a single TU, which directly enables better modularity of Skia.

Full design / rationale is here: https://docs.google.com/document/d/1J61ovS0pibxANhjaZMrmhy7mu2XkE0ofLZPRYEtoqCY/edit?usp=sharing&resourcekey=0--0drqBG6-WJRTqh9OLUTNA

--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

Git Watcher via monorail

unread,
Jun 1, 2023, 1:00:09 PM6/1/23
to bu...@skia.org

Comment #1 on issue 14355 by Git Watcher: Simplify SkOpts compilation
https://bugs.chromium.org/p/skia/issues/detail?id=14355#c1

The following revision refers to this bug:
https://skia.googlesource.com/skia/+/caefc81b300e2ab8996357e80b97240ab4d6b122

commit caefc81b300e2ab8996357e80b97240ab4d6b122
Author: Brian Osman <brian...@google.com>
Date: Thu Jun 01 14:10:49 2023

Workaround clang/GCC bug (attributes on re-declared constructors)

Attributes are omitted from constructors that are re-declared
(synthesized) via `using`, so if we wrap this code in something that
(for example) changes the target architecture, we get mismatch.

Explicitly re-declaring the constructors we want fixes the issue.
However, once we do that, there's really no benefit to VecStorage
(actually, there wasn't much benefit anyway). So instead, just remove
it entirely. This effectively reverts:
https://skia-review.googlesource.com/c/skia/+/457477

Details: https://groups.google.com/a/google.com/g/clang-discuss/c/_F7kjbDGt6o/m/vuAlu53oAQAJ

Bug: skia:14355
Change-Id: Ia37d0eb36e44fa92da26fccaa0907a7392672138
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/705896
Commit-Queue: Brian Osman <brian...@google.com>
Reviewed-by: John Stiles <johns...@google.com>

[modify] https://crrev.com/caefc81b300e2ab8996357e80b97240ab4d6b122/src/base/SkVx.h

brian… via monorail

unread,
Jun 15, 2023, 4:40:01 PM6/15/23
to bu...@skia.org
Updates:
Status: WontFix

Comment #2 on issue 14355 by brian...@google.com: Simplify SkOpts compilation
https://bugs.chromium.org/p/skia/issues/detail?id=14355#c2

Alas, clang's behavior makes this not work. Details in the addendum of the linked design doc.

brian… via monorail

unread,
Jul 21, 2023, 2:08:07 PM7/21/23
to bu...@skia.org
Updates:
Status: Accepted

Comment #3 on issue 14355 by brian...@google.com: Simplify SkOpts compilation
https://bugs.chromium.org/p/skia/issues/detail?id=14355#c3

(No comment was entered for this change.)
Reply all
Reply to author
Forward
0 new messages