Don't define MMX define in SSE.h since Microsoft compiler for x64 doesn't support MMX

RESOLVED FIXED in mozilla2.0b10

Status

()

Core
XPCOM
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

Trunk
mozilla2.0b10
x86_64
Windows Vista
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 years ago
http://msdn.microsoft.com/ja-jp/library/698bxz2w.aspx

"MMX intrinsics use the __m64 data type, which is not supported on x64 processors. "
(Assignee)

Comment 1

7 years ago
Also, bustage by bug 

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1294920732.1294924843.19536.gz#err0
Blocks: 616778
(Assignee)

Comment 2

7 years ago
Created attachment 503739 [details] [diff] [review]
fix
Assignee: nobody → m_kato
Status: NEW → ASSIGNED
Attachment #503739 - Flags: review?(justin.lebar+bug)
(Assignee)

Comment 3

7 years ago
Comment on attachment 503739 [details] [diff] [review]
fix

canel. I will refresh this.
Attachment #503739 - Flags: review?(justin.lebar+bug)
(Assignee)

Comment 4

7 years ago
Created attachment 503789 [details] [diff] [review]
fix v2
Attachment #503739 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Attachment #503789 - Flags: review?(justin.lebar+bug)
Comment on attachment 503789 [details] [diff] [review]
fix v2

Just a few nits on the comments:

>+# Microsoft Compiler doesn't support MMX
>+ifdef _MSC_VER
>+ifneq ($(OS_TEST),x86_64)
>+CPPSRCS += yuv_convert_mmx.cpp
>+endif

Suggest

> # MSVC doesn't support MMX when targeting AMD64.

> #if defined(_M_AMD64)
>-  // MMX is always available on AMD64.
>-  #define MOZILLA_PRESUME_MMX
>+  // Don't define MMX on Microsoft Visual C++ x64.
>+  // Microsoft Visual C++ doesn't support MMX although OS supports it

Suggest

> // MSVC for AMD64 doesn't support MMX, so don't presume it here.

>+#if !(defined(_MSC_VER) && defined(_M_AMD64))
>+  // Don't define MMX since compiler doesn't support it .
> #define MOZILLA_MAY_SUPPORT_MMX 1
>+#endif

Suggest

> // Define MOZILLA_MAY_SUPPORT_MMX only if we're not on MSVC for 
> // AMD64, since that compiler doesn't support MMX.

r=me with those changes.
Attachment #503789 - Flags: review?(justin.lebar+bug) → review+
(Assignee)

Comment 6

7 years ago
Created attachment 504403 [details] [diff] [review]
for checkin
(Assignee)

Updated

7 years ago
Attachment #504403 - Attachment is patch: true
Attachment #504403 - Attachment mime type: application/octet-stream → text/plain
(Assignee)

Comment 7

7 years ago
Comment on attachment 504403 [details] [diff] [review]
for checkin

Although this is bustage fix for Win64 build, do I need approval 2.0?
Attachment #504403 - Flags: approval2.0?
(In reply to comment 7)

khuey says no.  Go for it.
Attachment #503789 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Attachment #504403 - Flags: approval2.0?
(Assignee)

Updated

7 years ago
Blocks: 626480
(Assignee)

Comment 9

7 years ago
http://hg.mozilla.org/mozilla-central/rev/36bee2169e32
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b10
You need to log in before you can comment on or make changes to this bug.