rjmccall added inline comments.

================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:4938
+    if (EIT->getNumBits() > 128)
+      return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+
----------------
erichkeane wrote:
> rjmccall wrote:
> > 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?
> I think doing that would result in altering a number of other targets as well 
> (thats not a PPC specific function I think?). 
> 
> While there IS a little duplication, it is just the 5 lines that calculates 
> the array and coerces it. Otherwise the logic is just slightly different 
> enough I don't think it would save much.
> 
> That said, perhaps there is value in extracting those 5 lines or so into a 
> function.  I'll give that a shot.
Yeah, the function isn't PPC-specific.  The question is what the right rules 
are for homogeneous aggregates when aggregates contain ExtInt types.  For PPC64 
ELFv2, that... oh right, it only applies to floating-point and vector types.  
So I'm sorry, I led you on a false path here; we should not be trying to apply 
this logic to ExtInt types at all.


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