[PATCH] Change memcpy/memmove/memset to have dest and source alignment

Pete Cooper via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 28 15:59:18 PDT 2015


Hi Hal/Lang

This came out of a discussion here: http://lists.llvm.org/pipermail/llvm-dev/2015-August/089384.html

We want to be able to provide alignment for both the source and dest of mem intrinsics, instead of the alignment argument which only provides the minimum of dest/src.

Instead of adding another argument, I removed the alignment argument and added the align attributes to the src/dest pointer arguments.

I’ve updated the MemIntrinsic definition to handle this, and all of the code to now call getDestAlignment/getSrcAlignment instead of getAlignment.  For the few places where it wasn’t clear whether dest/src was the right choice, i’ve left a FIXME and I take the minimum of dest/src so as to match the current behavior.

I’ve also updated the create methods in the IR builder.  There is a (temporary) class there to handle the new source alignment parameter, as otherwise existing callers of this code could end up having the isVolatile bool implicitly converted to the source alignment.  I’ll remove this once out-of-tree users have had a chance to update to this patch.

Tests were updated to automatically strip out the alignment argument (the regex find/replace is in the patch).  I then ran make check and explicitly added back in alignments to all tests which needed it.  I tried to automatically update tests to transfer alignment to the attributes, but that wasn’t feasible due to constant expressions confusing regex (turns out regex doesn’t like recursion, which is more or less what you need to get balanced braces in gep(bit cast(inttoptr())) type expressions which we have in our tests).

There’s also a commit which shows how auto upgrading bitcode is handled.  There was already a test for llvm 3.2 which called memcpy so is used for this. I’ll add tests to upgrade memmove and memset prior to committing but just wanted to get the review process started.  memmove/memset tests will be almost identical to the memcpy test we already have.  I will use 3.7 as the bitcode generator for those tests though.

Finally, for LLVM, i’ve got a patch to show a cleanup of AlignmentFromAssumptions now that dest and source alignments can be different.  This is the only patch which can be committed on its own, and after the rest.  Everything else here is broken out in to separate commits just to try ease the review process.

For clang, the majority of changes are to pass both source and dest alignments to the IR Builder.  Again I tried to get the right ones where I could, but if I didn’t know the code well enough I just pass the same value to dest/src to match the current behaviour.  clang tests were all updated by hand because they all needed to check for the alignment attribute being set correctly coming out of codegen.

Finally, this is really just the minimum changes needed to get this in tree.  I haven’t made any effort to teach LLVM codegen about this.  That can luckily be done later, and probably independently for most backends by their respective owners.

Apologies for the scale of this patch.  As you can imagine, there wasn’t really a nice way to land this without excessive churn of the test cases themselves.

Cheers,
Pete

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Change-memcpy-memset-memmove-to-have-dest-and-source.patch
Type: application/octet-stream
Size: 62161 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150828/416a8aba/attachment-0007.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Auto-upgrade-bitcode-containing-calls-to-mem-intrins.patch
Type: application/octet-stream
Size: 4200 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150828/416a8aba/attachment-0008.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Automatically-strip-alignment-from-a-whole-bunch-of-.patch
Type: application/octet-stream
Size: 558675 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150828/416a8aba/attachment-0009.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Add-alignment-attributes-to-all-the-tests-which-need.patch
Type: application/octet-stream
Size: 106982 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150828/416a8aba/attachment-0010.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-Remove-a-bunch-of-unnecessary-code-from-AlignmentFro.patch
Type: application/octet-stream
Size: 5086 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150828/416a8aba/attachment-0011.obj>
-------------- next part --------------


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Update-clang-codegen-to-pass-both-dest-and-src-align.patch
Type: application/octet-stream
Size: 11435 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150828/416a8aba/attachment-0012.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Update-tests-to-check-for-the-alignment-attributes-i.patch
Type: application/octet-stream
Size: 138534 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150828/416a8aba/attachment-0013.obj>
-------------- next part --------------




More information about the cfe-commits mailing list