[PATCH] D112930: [LangRef][VP] Document vp.load and vp.store intrinsics

Tue Nov 2 10:35:57 PDT 2021

frasercrmck added inline comments.

================
Comment at: llvm/docs/LangRef.rst:19699
+'``alignment``' operand of the '``llvm.masked.store``' intrinsic is not set by
+'``llvm.vp.store``': it is taken as the ABI alignment of the type of the
+'``value``' operand as specified by the :ref:`datalayout
----------------
simoll wrote:
> frasercrmck wrote:
> > simoll wrote:
> > > It should suffice to say that alignment is derived the same way it is for regular load/store instructions (eg https://llvm.org/docs/LangRef.html#load-instruction).
> > > For example, there is also an `align` attribute for specifying the alignment (https://llvm.org/docs/LangRef.html#parameter-attributes).
> > > 
> > From what I can tell our current lowering of `vp.load` and `vp.store` don't check any `align` attributes. In SelectionDAGBuilder.cpp, we just do `MaybeAlign Alignment = DAG.getEVTAlign(VT);` I remember talk of obeying this attribute, but I don't think we should document anything that isn't currently true.
> > 
> > Shall I open a patch to use these attributes first, perhaps? //Then// document what we want to see happen?
> It's reasonable to stick with the current phrasing for this patch. Then, prepare another patch for SelectionDAG builder and LangRef that considers the `align` attribute.
Yep okay, that makes sense. Cheers.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112930