wxiao3 marked an inline comment 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: > 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. Yes, it's unnecessary to convert QualType to llvm::Type just for the _m64 vector type checking. Since It's very simple to check _m64 type based on QualType with pre-conditioned type assertion ``` if (const VectorType *VT = RetTy->getAs<VectorType>()) ``` I just remove the utility function: IsX86_MMXType. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59744/new/ https://reviews.llvm.org/D59744 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits