LukeGeeson marked an inline comment as done.
LukeGeeson added inline comments.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:10366
+ auto Alignment = CGM.getNaturalPointeeTypeAlignment(
+ E->getArg(0)->IgnoreParenCasts()->getType());
Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy));
----------------
pratlucas wrote:
> LukeGeeson wrote:
> > simon_tatham wrote:
> > > What effect is this change of strategy having on the alignment
> > > computation, for the already-supported instances of this builtin?
> > >
> > > It //looks// to me as if `__builtin_neon_vld1_v` with (say) a `uint8_t *`
> > > pointer argument will now compute `Alignment=1` (the natural alignment of
> > > the pointee type), whereas it would previously have computed
> > > `Alignment=8` (the size of the whole vector being loaded or stored).
> > >
> > > Is that intentional? Or accidental? Or have I completely misunderstood
> > > what's going on?
> > >
> > > (Whichever of the three, some discussion in the commit message would be a
> > > good idea, explaining why this change does or does not make a difference,
> > > as appropriate.)
> > Clang was incorrectly assuming that all the pointers from which loads were
> > being generated for vld1 intrinsics were aligned according to according to
> > the intrinsics result type. This causes alignment faults on the code
> > generated by the backend.
> > This fixes the issue so that alignment is based on the type of the pointer
> > provided to as input to the intrinsic.
> >
> > @pratlucas has done some work on this in parallel
> > https://reviews.llvm.org/D79721 which has been approved and may overrule
> > this particular line of code.
> >
> > I shall add a note to the commit message, and tentatively mark this as
> > fixed, given it's liable to adopt the work of Lucas.
> >
> Hi @LukeGeeson ,
> Just as a heads up, some changes to this implementations were requested on
> D79721.
> The usage of `CGM.getNaturalPointeeTypeAlignment` and `IgnoreParenCasts()`
> was causing problems on certain argument types, so the alignment is now
> captured from the expression itself when it is emitted.
Thanks @pratlucas yeah I plan to rebase my code onto upstream when you have
made those changes (and fix whatever still breaks) before I push :)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80716/new/
https://reviews.llvm.org/D80716
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
