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

[Reid Kleckner via Phabricator via cfe-commits]

rnk 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>.
----------------
wxiao3 wrote:
> 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?
> 
That's not quite what I'm suggesting. I'm saying that IsX86_MMXType should take a QualType parameter, and it should check if that qualtype looks like the __m64 vector type, instead of converting the QualType to llvm::Type and then checking if the llvm::Type is a 64-bit vector. Does that seem reasonable? See the code near the call site conditionalized on IsDarwinVectorABI which already has similar logic.


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

https://reviews.llvm.org/D59744