Closed Bug 625629 Opened 14 years ago Closed 14 years ago

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

Categories

(Core :: XPCOM, defect)

x86_64
Windows Vista
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b10

People

(Reporter: m_kato, Assigned: m_kato)

References

Details

Attachments

(1 file, 2 obsolete files)

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

"MMX intrinsics use the __m64 data type, which is not supported on x64 processors. "
Attached patch fix (obsolete) — Splinter Review
Assignee: nobody → m_kato
Status: NEW → ASSIGNED
Attachment #503739 - Flags: review?(justin.lebar+bug)
Comment on attachment 503739 [details] [diff] [review]
fix

canel. I will refresh this.
Attachment #503739 - Flags: review?(justin.lebar+bug)
Attached patch fix v2 (obsolete) — Splinter Review
Attachment #503739 - Attachment is obsolete: true
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+
Attached patch for checkinSplinter Review
Attachment #504403 - Attachment is patch: true
Attachment #504403 - Attachment mime type: application/octet-stream → text/plain
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
Attachment #504403 - Flags: approval2.0?
Blocks: 626480
http://hg.mozilla.org/mozilla-central/rev/36bee2169e32
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b10
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: