[PATCH] D134282: [CGP] Add generic TargetLowering::shouldAlignPointerArgs() implementation

[Jiejie Rong via Phabricator via llvm-commits]

JojoR added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetLoweringBase.cpp:967
+  if (Arg->getPointerAlignment(DL) > PrefAlign)
+    return false; // already aligned, not need to update it.
+
----------------
efriedma wrote:
> The question is what size load/store ops would we prefer to use for the memcpy, and whether those ops require alignment.  Using the alignment of a pointer seems arbitrary; we aren't loading or storing a pointer.
> 
> I'd prefer not to call getPointerAlignment() here if we can avoid it; the caller already does the math to figure out the current alignment and the increase.  shouldUpdatePointerArgAlignment just needs to know what alignment it wants, not whether the call currently satisfies that alignment.
As @efriedma said, we can set default PrefAlign according to PointerSize,
but we should return final PrefAlign from backend du to backend's requirement,
different ISAs maybe have different alignment :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134282/new/

https://reviews.llvm.org/D134282