[PATCH] D59744: Fix i386 ABI "__m64" type bug

[Wei Xiao via Phabricator via cfe-commits]

wxiao3 marked 2 inline comments as done.
wxiao3 added inline comments.


================
Comment at: lib/CodeGen/TargetInfo.cpp:919
 /// IsX86_MMXType - Return true if this is an MMX type.
 bool IsX86_MMXType(llvm::Type *IRType) {
-  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x i8>.
----------------
rnk wrote:
> I think looking at the LLVM type to decide how something should be passed is a bad pattern to follow. We should look at the clang AST to decide how things will be passed, not LLVM types. Would that be complicated? Are there aggregate types that end up getting passed directly in MMX registers?
For x86 32 bit target, no aggregate types end up getting passed in MMX register.
The only type passed by MMX is 

> __m64

 which is defined in header file (mmintrin.h):


```
typedef long long __m64 __attribute__((__vector_size__(8), __aligned__(8)));
```

Yes, it would be good if we define _m64 as a builtin type and handle it in AST level. But I'm afraid that it won't be a trivial work. Since GCC also handles __m64 in the same way as Clang currently does, can we just keep current implementation as it is?



================
Comment at: lib/CodeGen/TargetInfo.cpp:9489
+      // System V i386 ABI requires __m64 value passing by MMX registers.
+      bool EnableMMX = getContext().getTargetInfo().getABI() != "no-mmx";
       return SetCGInfo(new X86_32TargetCodeGenInfo(
----------------
rnk wrote:
> I think this needs to preserve existing behavior for Darwin and PS4 based on comments from @rjmccall and @dexonsmith in D60748.
ok, I will follow it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59744/new/

https://reviews.llvm.org/D59744