Closed Bug 643144 Opened 9 years ago Closed 9 years ago

GCC internal compiler error

Categories

(Tamarin Graveyard :: Virtual Machine, defect, P2)

x86
macOS
defect

Tracking

(Not tracked)

RESOLVED FIXED
Q3 11 - Serrano

People

(Reporter: treilly, Assigned: treilly)

Details

Attachments

(2 files)

No description provided.
Attachment #520452 - Flags: review?(edwsmith)
Attachment #520452 - Flags: review?(edwsmith) → review+
Just a quick issue: the code here was using a union, and this patch changes it to a cast.

Way way back in changeset 619, it was changed from a cast to a union, at least for allocDouble: see http://hg.mozilla.org/tamarin-redux/rev/619#l39.115

From past discussions with Lars and Steven, I believe the change to use a union form was intended to avoid strict-aliasing issues.

See: http://cellperformance.beyond3d.com/articles/2006/06/understanding-strict-aliasing.html

I have not reviewed this particular case carefully enough to tell whether it is safe under strict-aliasing or not, so maybe all is well.  But I'd want to see a comment justifying why a cast is okay in this case, just so someone does not go in and change it back to a union later. . .
FWIW, I'm sort of questioning whether trying to be aliasing safe is buying us anything; so far as I know, we're not able to compile either the VM or the Player with strict aliasing enabled.  I also don't believe that the code that we have that tries to avoid aliasing failures actually does that, in all cases.
Also, on the flip side of the coin, if these unions are causing gcc to
internal-compiler-error, we might need a work-item to review all of the
existing reinterpret-bits-via-union uses and see if they can be re-expressed as
casts, to future-proof against other ICE's that have not arisen yet . . .
the union stuff was both (1) misguided, as it's not clear that it actually helps the aliasing issue, and (2) premature, as we don't attempt to compile with strict aliasing.

It's desirable to do someday, as some architectures reportedly get substantial benefit from this, but it needs to be done systematically.

so just change to a cast for now to deal with the dumb compiler.
changeset: 6118:79e463a7dbfc
user:      Tommy Reilly <treilly@adobe.com>
summary:   Bug 643144 - GCC internal compiler error (r=edwsmith)

http://hg.mozilla.org/tamarin-redux/rev/79e463a7dbfc
The only way I think we can safe guard against this is to a) figure out why the xp build doesn't crash like the xcode one does (when both are using 4.2)  or b) have the deep testing build tweak the xcode project with this patch and try to build tamarin.   I'm resolving this bug but please post links to any new bugs if folks think there's more we should do here.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.