erichkeane marked an inline comment as done. erichkeane 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: > > > 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. So I should switch it back to direct/indirect like earlier? That I can do. 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
