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

Reply via email to