Use compiler alignof support

RESOLVED WONTFIX

Status

()

Firefox for Android
General
RESOLVED WONTFIX
6 years ago
4 years ago

People

(Reporter: Chris Dearman, Unassigned)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0
Build ID: 20120310193349

Steps to reproduce:

Tried to use MOZ_ALIGNOF() as a parameter to MOZ_ALIGNED_DECL()

See https://bugzilla.mozilla.org/show_bug.cgi?id=779013
for the history


Actual results:

gcc generates an error: "requested alignment is not  a constant"


Expected results:

This seems to be a deliberate design decision for gcc. See
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10479#c4

Gcc and MSC have an alignof keyword which could be used instead of the template code that is currently used.
(Reporter)

Comment 1

6 years ago
Created attachment 648187 [details] [diff] [review]
Use alignof keyword for MSC/gcc

I've only tested the gcc version of this
Attachment #648187 - Flags: review?(jones.chris.g)
Comment on attachment 648187 [details] [diff] [review]
Use alignof keyword for MSC/gcc

I don't think this patch is a very good idea, but jlebar should weigh in.

The problem is that for gcc and msvc, we'll be using magical compiler helpers that statically evaluate properly.  Everywhere else, we'll be using a "fallback" template solution that may or may not evaluate the same way as the magical compiler helpers.  So the semantics of MOZ_ALIGNOF() won't be consistent across platforms.

If the template solution isn't sufficient, we'll need to find another fallback that works everywhere.
Attachment #648187 - Flags: review?(jones.chris.g) → review?(justin.lebar+bug)
> The problem is that for gcc and msvc, we'll be using magical compiler helpers that statically 
> evaluate properly.  Everywhere else, we'll be using a "fallback" template solution that may or may 
> not evaluate the same way as the magical compiler helpers.  So the semantics of MOZ_ALIGNOF() 
> won't be consistent across platforms.

We can also use alignof on any C++11-compliant compiler.

I'm /kind of/ OK with this change, inasmuch as alignof/__alignof/__alignof__ is strictly more powerful than the templated class, so by using it, we can only break tier-3 compilers.

But maybe we're answering the wrong question here.  It seems that what we really want is NaturallyAlignedStorage<T>, which gives you sizeof(T) space, aligned to alignof(T).  You'd implement this however you can/want, on your given compiler.
Comment on attachment 648187 [details] [diff] [review]
Use alignof keyword for MSC/gcc

Closing this bug because it doesn't look like we're doing it this way.  Although I do think there's room for improvement in how we declare aligned buffers (comment 3).
Attachment #648187 - Flags: review?(justin.lebar+bug)
Status: UNCONFIRMED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WONTFIX

Comment 5

4 years ago
Created attachment 8444020 [details] [diff] [review]
Use alignof/alignas keywords for C++11/MSC/gcc, v2

__alignof__ may be added again per bug 1026499 comment 9. Updating the patch as an alternative to fixing AlignmentFinder.

https://tbpl.mozilla.org/?tree=Try&rev=04b05669b31f
https://tbpl.mozilla.org/?tree=Try&rev=392daba7c7c1
Attachment #648187 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.