[PATCH v2] Respect alignment when loading up a coerced function argument

[Hal Finkel]

[+John]

Hi Uli,

-    CGF.Builder.CreateStore(Src, DstPtr, DstIsVolatile);
+    llvm::StoreInst *SI = CGF.Builder.CreateStore(Src, DstPtr, DstIsVolatile);
+    SI->setAlignment(DstAlign.getQuantity());
     return;

You can use CreateAlignedStore instead of CreateStore here (and in several other places).

+    llvm::AllocaInst *Tmp = CGF.CreateTempAlloca(SrcTy);
+    Tmp->setAlignment(DstAlign.getQuantity());

As follow-up, we should make CreateTempAlloca take an optional alignment. Running:

  grep -r -A 2 CreateTempAlloca * | grep setAlign

reveals that this patten already occurs in several places.

Thanks again,
Hal

----- Original Message -----
> From: "Ulrich Weigand" <Ulrich.Weigand at de.ibm.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "cfe commits" <cfe-commits at cs.uiuc.edu>
> Sent: Friday, June 19, 2015 11:16:43 AM
> Subject: [PATCH v2] Respect alignment when loading up a coerced function argument
> 
> Hal Finkel <hfinkel at anl.gov> wrote on 06.05.2015 00:09:45:
> 
> > >> This patch adds a setAlignment call in one place in
> > >> CreateCoercedLoad
> > >> where it was missing so far.  Note that as this location, we do
> > >> not
> > >> actually know what alignment of the source location we can rely
> > >> on;
> > >> the callers do not pass anything to this routine.
> >
> > Can we actually fix this problem so that the correct alignment can
> > be
> used?
> 
> Here's a new version of the patch that changes CreateCoercedLoad to
> take
> an alignment operand and changes the callers to pass in the best data
> they
> have.  For consistency, I've done the same to CreateCoercedStore as
> well
> (although isn't required for correctness reasons on SystemZ).
> 
> This does indeed reduce the number of misaligned loads and store
> quite
> at bit, as can be seen in the test cases I've had to update ...
> 
> (See attached file: clang-align-coerced)
> 
> Does this look OK?
> 
> Bye,
> Ulrich

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory