rjmccall added inline comments.

================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:4938
+    if (EIT->getNumBits() > 128)
+      return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+
----------------
rjmccall wrote:
> erichkeane wrote:
> > rjmccall wrote:
> > > Does this need to consider the aggregate-as-array logic below?
> > I'm not sure what you mean by this?  Are you suggesting I could/should pass 
> > this as an array instead of indirectly?
> My interpretation is that in general you're lowering large a `_ExtInt` like a 
> struct with a bunch of integer members in it.  My understanding is that, for 
> this target, that would make it a homogeneous aggregate eligible for the 
> special treatment given to certain aggregate types below.  I don't know what 
> that corresponds to at the code-emission level.
This seems to work, although it's unfortunate that it duplicates some of the 
existing behavior.  Do we also need to modify `isHomogeneousAggregate` to 
consider `ExtInts`?  And if we do, does that avoid any of the duplicate logic 
here?


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:6658
 /// Checks if the type is unsupported directly by the current target.
 static bool isUnsupportedType(ASTContext &Context, QualType T) {
   if (!Context.getTargetInfo().hasFloat16Type() && T->isFloat16Type())
----------------
erichkeane wrote:
> rjmccall wrote:
> > Ugh.  Please go ahead and rename this to make it clear that it's an NVPTX 
> > helper function.
> :)  Moved to a member function.  Also, coerceToIntArrayWithLimit made sense 
> to do so as well, since it was the caller of this.
Thanks.


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

https://reviews.llvm.org/D79118



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to