llvm dev - Jan 2018 - Change memcpy/memmove/memset to have dest and source alignment attributes

Good day all,
 I’ve spent a few days resurrecting the circa-2015 work on removing the explicit
alignment argument (4th arg) from the @llvm.memcpy/memmove/memset intrinsics in
favour of using the alignment attribute on the pointer args of calls to the
intrinsic. This work was first proposed back in August 2015 by Lang Hames:
http://lists.llvm.org/pipermail/llvm-dev/2015-August/089384.html   (item 2)
 and an attempt at landing the work was made by Pete Cooper in November 2015,
but then backed out due to unspecified bot failures:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20151109/312083.html

 I’ve prepared changes for LLVM, Clang, and Polly that are now up for review:
* https://reviews.llvm.org/D41675   (LLVM part)
* https://reviews.llvm.org/D41676   (polly part)
* https://reviews.llvm.org/D41677   (Clang part)

 Importantly for those maintaining downstream users of the LLVM API, this
changes the prototypes for the @llvm.memcpy/memmove/memset intrinsics and
changes the IRBuilder API for creating memcpy and memmove calls.

For example, IR 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)

 The LLVM change includes auto upgrade of the old IR. However, match expressions
in IR tests and calls to IRBuilder’s CreateMemCpy & CreateMemMove will need
to be updated.

 My plan is to post another note to the list when the change is landed, and
stable.

 Comments? Concerns?

-Daniel

---
Daniel Neilson, Ph.D.
Azul Systems




-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180102/f44b211d/attachment.html>

On 01/02/2018 02:11 PM, Daniel Neilson via llvm-dev
wrote:
> Good day all,
>  I’ve spent a few days resurrecting the circa-2015 work on removing 
> the explicit alignment argument (4th arg) from the 
> @llvm.memcpy/memmove/memset intrinsics in favour of using the 
> alignment attribute on the pointer args of calls to the intrinsic. 
> This work was first proposed back in August 2015 by Lang Hames:
> http://lists.llvm.org/pipermail/llvm-dev/2015-August/089384.html  (item 2)
>  and an attempt at landing the work was made by Pete Cooper in 
> November 2015, but then backed out due to unspecified bot failures:
>
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20151109/312083.html
>
>  I’ve prepared changes for LLVM, Clang, and Polly that are now up for 
> review:
> * https://reviews.llvm.org/D41675  (LLVM part)
> * https://reviews.llvm.org/D41676  (polly part)
> * https://reviews.llvm.org/D41677  (Clang part)
>
>  Importantly for those maintaining downstream users of the LLVM API, 
> this changes the prototypes for the @llvm.memcpy/memmove/memset 
> intrinsics and changes the IRBuilder API for creating memcpy and 
> memmove calls.
>
> For example, IR 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)
>
>  The LLVM change includes auto upgrade of the old IR. However, match 
> expressions in IR tests and calls to IRBuilder’s CreateMemCpy & 
> CreateMemMove will need to be updated.
>
>  My plan is to post another note to the list when the change is 
> landed, and stable.
>
>  Comments? Concerns?
Thanks for working on this. This still seems like the right thing to do 
(and will let us represent separate source and destination alignments).

  -Hal
>
> -Daniel
>
> ---
> Daniel Neilson, Ph.D.
> Azul Systems
>
>
>
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180102/16ee51e7/attachment.html>

Hi all,
 This change has been reviewed, and appears to be ready to land (review
available here if anyone still wants to chime in:
https://reviews.llvm.org/D41675 ). The process that we’re going to use for
landing this will take a few steps. To wit:

Step 1) Remove align argument, and add align attribute to pointer args. Require
that src & dest have the same alignment via verifier rule.  Also update
Clang & Polly tests to pattern-match the new form.
Step 2) Expand the IRBuilder API to allow creation of memcpy/memmove with
differing source and dest alignments.
Step 3) Update Clang to use the new IRBuilder API.
Step 4) Update Polly to use the new IRBuilder API.
Step 5) Update LLVM passes that create memcpy/memmove calls to use the new
IRBuilder API, and those that use use MemIntrinsicInst::[get|set]Alignment() to
use  getDestAlignment() and getSourceAlignment() instead.
Step 6) Remove the single-alignment IRBuilder API for memcpy/memmove, and the
MemIntrinsicInst::[get|set]Alignment() methods.

 Unless there are objections, I would like to land step 1 tomorrow (Friday Jan
19) morning. My hope is to be completing up to step 4 by the middle of next
week, and then steps 5 & 6 in the coming weeks.

 Landing step 1 will potentially have a large impact on downstream consumer’s
tests. For example, a front-end that checks IR generation of constructs
implemented using memcpy/memmove/memset; the Clang tests for OpenMP
firstprivate/lastprivate clauses, for instance, required a lot of manual
updating. These sorts of tests will need to be updated to match the new pattern
printed for the memory intrinsics. I’ve included an extended sed script in the
commit log that can update many patterns, but definitely does not catch them all
so some manual updating will be required.

-Daniel

---
Daniel Neilson, Ph.D.
Azul Systems

On Jan 2, 2018, at 2:11 PM, Daniel Neilson via llvm-dev <llvm-dev at
lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:

Good day all,
 I’ve spent a few days resurrecting the circa-2015 work on removing the explicit
alignment argument (4th arg) from the @llvm.memcpy/memmove/memset intrinsics in
favour of using the alignment attribute on the pointer args of calls to the
intrinsic. This work was first proposed back in August 2015 by Lang Hames:
http://lists.llvm.org/pipermail/llvm-dev/2015-August/089384.html   (item 2)
 and an attempt at landing the work was made by Pete Cooper in November 2015,
but then backed out due to unspecified bot failures:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20151109/312083.html

 I’ve prepared changes for LLVM, Clang, and Polly that are now up for review:
* https://reviews.llvm.org/D41675   (LLVM part)
* https://reviews.llvm.org/D41676   (polly part)
* https://reviews.llvm.org/D41677   (Clang part)

 Importantly for those maintaining downstream users of the LLVM API, this
changes the prototypes for the @llvm.memcpy/memmove/memset intrinsics and
changes the IRBuilder API for creating memcpy and memmove calls.

For example, IR 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)

 The LLVM change includes auto upgrade of the old IR. However, match expressions
in IR tests and calls to IRBuilder’s CreateMemCpy & CreateMemMove will need
to be updated.

 My plan is to post another note to the list when the change is landed, and
stable.

 Comments? Concerns?

-Daniel

---
Daniel Neilson, Ph.D.
Azul Systems




_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180118/157cd12e/attachment.html>

> On Jan 18, 2018, at 7:45 AM, Daniel Neilson via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> 
> Hi all,
>  This change has been reviewed, and appears to be ready to land (review
available here if anyone still wants to chime in:
https://reviews.llvm.org/D41675 <https://reviews.llvm.org/D41675> ). The
process that we’re going to use for landing this will take a few steps. To wit:
> 
> Step 1) Remove align argument, and add align attribute to pointer args.
What does this mean? Are you suggesting that alignment become part of
PointerType?  That would introduce lots of problems, because unaligned and
aligned pointers would require casts, which then interfere with optimizations.

-Chris


> Require that src & dest have the same alignment via verifier rule. 
Also update Clang & Polly tests to pattern-match the new form.
> Step 2) Expand the IRBuilder API to allow creation of memcpy/memmove with
differing source and dest alignments.
> Step 3) Update Clang to use the new IRBuilder API.
> Step 4) Update Polly to use the new IRBuilder API.
> Step 5) Update LLVM passes that create memcpy/memmove calls to use the new
IRBuilder API, and those that use use MemIntrinsicInst::[get|set]Alignment() to
use  getDestAlignment() and getSourceAlignment() instead.
> Step 6) Remove the single-alignment IRBuilder API for memcpy/memmove, and
the MemIntrinsicInst::[get|set]Alignment() methods.
> 
>  Unless there are objections, I would like to land step 1 tomorrow (Friday
Jan 19) morning. My hope is to be completing up to step 4 by the middle of next
week, and then steps 5 & 6 in the coming weeks.
> 
>  Landing step 1 will potentially have a large impact on downstream
consumer’s tests. For example, a front-end that checks IR generation of
constructs implemented using memcpy/memmove/memset; the Clang tests for OpenMP
firstprivate/lastprivate clauses, for instance, required a lot of manual
updating. These sorts of tests will need to be updated to match the new pattern
printed for the memory intrinsics. I’ve included an extended sed script in the
commit log that can update many patterns, but definitely does not catch them all
so some manual updating will be required.
> 
> -Daniel
> 
> ---
> Daniel Neilson, Ph.D.
> Azul Systems
> 
>> On Jan 2, 2018, at 2:11 PM, Daniel Neilson via llvm-dev <llvm-dev at
lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>> 
>> Good day all,
>>  I’ve spent a few days resurrecting the circa-2015 work on removing the
explicit alignment argument (4th arg) from the @llvm.memcpy/memmove/memset
intrinsics in favour of using the alignment attribute on the pointer args of
calls to the intrinsic. This work was first proposed back in August 2015 by Lang
Hames:
>> http://lists.llvm.org/pipermail/llvm-dev/2015-August/089384.html
<http://lists.llvm.org/pipermail/llvm-dev/2015-August/089384.html>   (item
2)
>>  and an attempt at landing the work was made by Pete Cooper in November
2015, but then backed out due to unspecified bot failures:
>>
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20151109/312083.html
<http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20151109/312083.html>
>> 
>>  I’ve prepared changes for LLVM, Clang, and Polly that are now up for
review:
>> * https://reviews.llvm.org/D41675
<https://reviews.llvm.org/D41675>   (LLVM part)
>> * https://reviews.llvm.org/D41676
<https://reviews.llvm.org/D41676>   (polly part)
>> * https://reviews.llvm.org/D41677
<https://reviews.llvm.org/D41677>   (Clang part)
>> 
>>  Importantly for those maintaining downstream users of the LLVM API,
this changes the prototypes for the @llvm.memcpy/memmove/memset intrinsics and
changes the IRBuilder API for creating memcpy and memmove calls.
>> 
>> For example, IR 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)
>> 
>>  The LLVM change includes auto upgrade of the old IR. However, match
expressions in IR tests and calls to IRBuilder’s CreateMemCpy &
CreateMemMove will need to be updated.
>> 
>>  My plan is to post another note to the list when the change is landed,
and stable.
>> 
>>  Comments? Concerns?
>> 
>> -Daniel
>> 
>> ---
>> Daniel Neilson, Ph.D.
>> Azul Systems
>> 
>> 
>> 
>> 
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180118/b4a9b1c1/attachment.html>

Hi all,
 A quick note just to let people know that as of this past Friday my go at this
work has been fully landed. It ended up being a back-burner item, so it took
longer than I would have liked to get completed. None the less, the changes made
were:
1) The IRBuilders in LLVM, Clang, and Polly were all updated to create only the
new form of the memory intrinsics.
2) All LLVM passes to understand the new forms of the intrinsics; modulo the
notes below.
3) The IRBuilder APIs for creating the old-style memcpy/memmove/memset forms and
the MemIntrinsicInst API for getting/setting the old generic/conservative
alignment have all been eliminated.

 There are a few places where the LLVM pass updates were conservative, and could
potentially be made more aggressive by a motivated individual — list below.

 All that remains is for interested folk to enhance lowering of small
memcpy/memmove into load/store sequences. The lowering in SelectionDAG currently
just conservatively uses the MinAlign() of the source & dest alignments as
the alignment for the loads and stores. I suspect that we could do better by
teaching the lowering that the source & dest can have different alignments.

 Note that, also, the createMem[Cpy|Move]Loop type functions used in the
LowerMemIntrinsics pass are also now invoked with different source & dest
alignments by LowerMemIntrinsics, rather than the same alignment for both, so
these helpers will now be invoked with more information than they have in the
past; I’m guessing that it’s possible they could do better with this
information. For example, createMemMoveLoop() doesn’t even use the alignments
it’s given at all right now, and the neither of the createMemCpyLoop*()
functions try to set the alignments on the loads & stores it creates.

Passes that have conservative alignments after updating:
- SelectionDAG
- AArch64FastISel
- ARMFastISel
- MemorySanitizer
- MemCpyOpt : Call slot optimization
- InlineFunction :  HandleByValArgumentInit
- LowerMemIntrinsics (see note above)

Cheers,
 Daniel

---
Daniel Neilson, Ph.D.
Azul Systems


On Jan 2, 2018, at 2:11 PM, Daniel Neilson via llvm-dev <llvm-dev at
lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:

Good day all,
 I’ve spent a few days resurrecting the circa-2015 work on removing the explicit
alignment argument (4th arg) from the @llvm.memcpy/memmove/memset intrinsics in
favour of using the alignment attribute on the pointer args of calls to the
intrinsic. This work was first proposed back in August 2015 by Lang Hames:
http://lists.llvm.org/pipermail/llvm-dev/2015-August/089384.html   (item 2)
 and an attempt at landing the work was made by Pete Cooper in November 2015,
but then backed out due to unspecified bot failures:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20151109/312083.html

 I’ve prepared changes for LLVM, Clang, and Polly that are now up for review:
* https://reviews.llvm.org/D41675   (LLVM part)
* https://reviews.llvm.org/D41676   (polly part)
* https://reviews.llvm.org/D41677   (Clang part)

 Importantly for those maintaining downstream users of the LLVM API, this
changes the prototypes for the @llvm.memcpy/memmove/memset intrinsics and
changes the IRBuilder API for creating memcpy and memmove calls.

For example, IR 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)

 The LLVM change includes auto upgrade of the old IR. However, match expressions
in IR tests and calls to IRBuilder’s CreateMemCpy & CreateMemMove will need
to be updated.

 My plan is to post another note to the list when the change is landed, and
stable.

 Comments? Concerns?

-Daniel

---
Daniel Neilson, Ph.D.
Azul Systems




_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180326/32d23b5b/attachment.html>

Hi Daniel,

a quick question (and kind-of a follow-up to
<https://lists.llvm.org/pipermail/llvm-dev/2017-July/115665.html>):  Do
the
pointers have to be aligned even if the size is 0?  It would be nice to have
this stated explicitly in the LangRef.

Kind regards,
Ralf

On 26.03.2018 22:43, Daniel Neilson via llvm-dev wrote:
> Hi all,
>  A quick note just to let people know that as of this past Friday my go at
this
> work has been fully landed. It ended up being a back-burner item, so it
took
> longer than I would have liked to get completed. None the less, the changes
made
> were:
> 1) The IRBuilders in LLVM, Clang, and Polly were all updated to create only
the
> new form of the memory intrinsics.
> 2) All LLVM passes to understand the new forms of the intrinsics; modulo
the
> notes below.
> 3) The IRBuilder APIs for creating the old-style memcpy/memmove/memset
forms and
> the MemIntrinsicInst API for getting/setting the old generic/conservative
> alignment have all been eliminated.
> 
>  There are a few places where the LLVM pass updates were conservative, and
could
> potentially be made more aggressive by a motivated individual — list
below. 
> 
>  All that remains is for interested folk to enhance lowering of small
> memcpy/memmove into load/store sequences. The lowering in SelectionDAG
currently
> just conservatively uses the MinAlign() of the source & dest alignments
as the
> alignment for the loads and stores. I suspect that we could do better by
> teaching the lowering that the source & dest can have different
alignments.
> 
>  Note that, also, the createMem[Cpy|Move]Loop type functions used in the
> LowerMemIntrinsics pass are also now invoked with different source &
dest
> alignments by LowerMemIntrinsics, rather than the same alignment for both,
so
> these helpers will now be invoked with more information than they have in
the
> past; I’m guessing that it’s possible they could do better with this
> information. For example, createMemMoveLoop() doesn’t even use the
alignments
> it’s given at all right now, and the neither of the createMemCpyLoop*()
> functions try to set the alignments on the loads & stores it creates.
> 
> Passes that have conservative alignments after updating:
> - SelectionDAG
> - AArch64FastISel
> - ARMFastISel
> - MemorySanitizer
> - MemCpyOpt : Call slot optimization
> - InlineFunction :  HandleByValArgumentInit
> - LowerMemIntrinsics (see note above)
> 
> Cheers,
>  Daniel
> 
> ---
> Daniel Neilson, Ph.D.
> Azul Systems
> 
> 
>> On Jan 2, 2018, at 2:11 PM, Daniel Neilson via llvm-dev
>> <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>> wrote:
>>
>> Good day all,
>>  I’ve spent a few days resurrecting the circa-2015 work on removing the
>> explicit alignment argument (4th arg) from the
@llvm.memcpy/memmove/memset
>> intrinsics in favour of using the alignment attribute on the pointer
args of
>> calls to the intrinsic. This work was first proposed back in August
2015 by
>> Lang Hames:
>> http://lists.llvm.org/pipermail/llvm-dev/2015-August/089384.html 
 (item 2)
>>  and an attempt at landing the work was made by Pete Cooper in November
2015,
>> but then backed out due to unspecified bot failures:
>>
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20151109/312083.html
>>
>>  I’ve prepared changes for LLVM, Clang, and Polly that are now up for
review:
>> * https://reviews.llvm.org/D41675   (LLVM part)
>> * https://reviews.llvm.org/D41676   (polly part)
>> * https://reviews.llvm.org/D41677   (Clang part)
>>
>>  Importantly for those maintaining downstream users of the LLVM API,
this
>> changes the prototypes for the @llvm.memcpy/memmove/memset intrinsics
and
>> changes the IRBuilder API for creating memcpy and memmove calls.
>>
>> For example, IR 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)
>>
>>  The LLVM change includes auto upgrade of the old IR. However, match
>> expressions in IR tests and calls to IRBuilder’s CreateMemCpy &
CreateMemMove
>> will need to be updated.
>>
>>  My plan is to post another note to the list when the change is landed,
and
>> stable.
>>
>>  Comments? Concerns?
>>
>> -Daniel
>>
>> ---
>> Daniel Neilson, Ph.D.
>> Azul Systems
>>
>>
>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> 
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>