This is an archive of the discontinued LLVM Phabricator instance.

[VP] Propagate align parameter attr on VP load/store to ISel
ClosedPublic

Authored by frasercrmck on Nov 23 2021, 2:05 AM.

Details

Summary

This patch fixes a case where the 'align' parameter attribute on the
pointer operands to llvm.vp.load and llvm.vp.store was being dropped
during the conversion to the SelectionDAG. The default alignment
equal to the ABI type alignment of the vector type was kept. It also
updates the documentation to reflect the fact that the parameter
attribute is now properly supported.

Diff Detail

Event Timeline

frasercrmck created this revision.Nov 23 2021, 2:05 AM
frasercrmck requested review of this revision.Nov 23 2021, 2:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 23 2021, 2:05 AM
craig.topper added inline comments.Nov 24 2021, 11:32 AM
llvm/docs/LangRef.rst
19645

The previous paragraph called this "first operand" now we're using "first argument".

craig.topper added inline comments.Nov 24 2021, 2:27 PM
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
7346

Not related to this patch, but should we be using UnknownSize here after D113888. At least if the mask isn't all ones.

frasercrmck marked 2 inline comments as done.
  • rebase and address feedback
llvm/docs/LangRef.rst
19645

You make a good argument (sorry!). I've fixed that up.

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
7346

Yeah thanks for reminding me. I saw that patch go through and was going to check what was needed for VP. I'll create a note for myself to come back to this.

simoll added inline comments.Dec 3 2021, 1:34 AM
llvm/docs/LangRef.rst
19655

"is taken as the align parameter attribute"? To use the same wording as below for vp.store. Also, to make clear that alignment is specified as an attribute of the pointer and not another parameter.

frasercrmck marked an inline comment as done.
  • rebase and add missing word
llvm/docs/LangRef.rst
19655

Oh crumbs yeah I accidentally a word. Fixed!

simoll accepted this revision.Dec 7 2021, 1:30 AM

LGTM

This revision is now accepted and ready to land.Dec 7 2021, 1:30 AM