Page MenuHomePhabricator

Change memcpy/memove/memset to have dest and source alignment attributes.
ClosedPublic

Authored by dneilson on Jan 2 2018, 12:06 PM.

Details

Summary

Upstream LLVM is changing the the prototypes of the @llvm.memcpy/memmove/memset
intrinsics. This change updates the Clang CGBuilder and the Clang tests
for this change.

The @llvm.memcpy/memmove/memset intrinsics currently have an explicit argument
which is required to be a constant integer. It represents the alignment of the
dest (and source), and so must be the minimum of the actual alignment of the
two.

This change allows source and dest to each have their own alignments by
using the alignment attribute on their arguments. The alignment argument
is removed.

For example, code which used to read:
call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* %src, i32 100, i32 4, i1 false)
will now read
call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %dest, i8* align 4 %src, i32 100, i1 false)

Diff Detail

Repository
rL LLVM

Event Timeline

dneilson created this revision.Jan 2 2018, 12:06 PM
rjmccall accepted this revision.Jan 2 2018, 12:47 PM

I'm glad to hear that progress is finally happening on this.

The change to CGBuilder looks good to me. I'm going to take your word for it that the test changes are all just obvious updates; if there's one in particular that you'd like me to look at, I'd be happy to.

This revision is now accepted and ready to land.Jan 2 2018, 12:47 PM

I'm glad to hear that progress is finally happening on this.

The change to CGBuilder looks good to me. I'm going to take your word for it that the test changes are all just obvious updates; if there's one in particular that you'd like me to look at, I'd be happy to.

Thanks! Yeah, the CGBuilder change is the meat of it. The test changes are all either sed-script changes, or manual fixes. For the manual fixes I made an effort to make sure that if the test wasn't checking for a specific alignment value before this change then it won't be checking for a specific alignment value after it either (i.e. matches like align {{[0-9]+}} when the alignment value doesn't matter).

dneilson updated this revision to Diff 131685.Jan 27 2018, 10:04 AM

Rebaseline

rjmccall accepted this revision.Jan 27 2018, 9:23 PM

Still LGTM.

This revision was automatically updated to reflect the committed changes.