efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land.
LGTM ================ Comment at: lib/CodeGen/CGBuilder.h:260 + CharUnits::fromQuantity(Offset.getSExtValue()))); + } + ---------------- jfb wrote: > efriedma wrote: > > Not sure about the new helper. We already have CreateStructGEP and > > CreateConstArrayGEP which do approximately what you want. > It's close to doing what we want, but not quite? It seems like the original > code would have used them otherwise. > > To use them we'd have to: > > - branch on struct / array > - for struct calculate the offset there (which the new helper does) > - for array get the element size > > Seems simpler to use GEP2_32 and more fool-proof to (internal to the helper) > use GEP's own idea of what the offset is, no? Okay. (Actually, I think this ends up conservatively underestimating the alignment in some cases involving nested structs, but it's unlikely to matter.) Repository: rC Clang https://reviews.llvm.org/D49209 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
