Page MenuHomePhabricator

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

Authored by frasercrmck on Tue, Nov 23, 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.Tue, Nov 23, 2:05 AM
frasercrmck requested review of this revision.Tue, Nov 23, 2:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptTue, Nov 23, 2:05 AM
craig.topper added inline comments.Wed, Nov 24, 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.Wed, Nov 24, 2:27 PM
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
7345

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
7345

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.Fri, Dec 3, 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!