<div dir="ltr">LGTM as well.<div><br></div><div>I'm particularly happy to specify that there is no guarantee about the width of loads and/or stores resulting, as I think that's important for frontends to not depend on (or to request a change if they need to depend on it).</div>
<div><br></div><div>Thanks Andy far the additional clarification, that helps pin down things even more.</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jan 29, 2013 at 10:30 AM, Andrew Trick <span dir="ltr"><<a href="mailto:atrick@apple.com" target="_blank">atrick@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I can't think of a better way to do this, so I think it's ok.<br>
<br>
I also submitted a complementary patch on llvm-commits clarifying volatile semantics.<br>
<br>
-Andy<br>
<div><div class="h5"><br>
On Jan 28, 2013, at 8:54 AM, Arnaud A. de Grandmaison <<a href="mailto:arnaud.allarddegrandmaison@parrot.com">arnaud.allarddegrandmaison@parrot.com</a>> wrote:<br>
<br>
> Hi All,<br>
><br>
> In the language reference manual, the access behavior of the memcpy,<br>
> memmove and memset intrinsics is not well defined with respect to the<br>
> volatile flag. The LRM even states that "it is unwise to depend on it".<br>
> This forces optimization passes to be conservatively correct and prevent<br>
> optimizations.<br>
><br>
> A very simple example of this is :<br>
><br>
> $ cat test.c<br>
><br>
> #include <stdint.h><br>
><br>
> struct R {<br>
> uint16_t a;<br>
> uint16_t b;<br>
> };<br>
><br>
> volatile struct R * const addr = (volatile struct R *) 416;<br>
><br>
> void test(uint16_t a)<br>
> {<br>
> struct R r = { a, 1 };<br>
> *addr = r;<br>
> }<br>
><br>
> $ clang -O2 -o - -emit-llvm -S -c test.c<br>
> ; ModuleID = 'test.c'<br>
> target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"<br>
> target triple = "x86_64-unknown-linux-gnu"<br>
><br>
> %struct.R = type { i16, i16 }<br>
><br>
> @addr = constant %struct.R* inttoptr (i64 416 to %struct.R*), align 8<br>
><br>
> define void @test(i16 zeroext %a) nounwind uwtable {<br>
> %r.sroa.0 = alloca i16, align 2<br>
> %r.sroa.1 = alloca i16, align 2<br>
> store i16 %a, i16* %r.sroa.0, align 2<br>
> store i16 1, i16* %r.sroa.1, align 2<br>
> %r.sroa.0.0.load3 = load volatile i16* %r.sroa.0, align 2<br>
> store volatile i16 %r.sroa.0.0.load3, i16* inttoptr (i64 416 to i16*), align 32<br>
> %r.sroa.1.0.load2 = load volatile i16* %r.sroa.1, align 2<br>
> store volatile i16 %r.sroa.1.0.load2, i16* inttoptr (i64 418 to i16*), align 2<br>
> ret void<br>
> }<br>
><br>
> In the generated ir, the loads are marked as volatile. In this case,<br>
> this is due to the new SROA being conservatively correct. We should<br>
> however expect the test function to look like this much simpler one<br>
> (which was actually the case with llvm <= 3.1) :<br>
><br>
> define void @test(i16 zeroext %a) nounwind uwtable {<br>
> store volatile i16 %a, i16* inttoptr (i64 416 to i16*), align 2<br>
> store volatile i16 1, i16* inttoptr (i64 418 to i16*), align 2<br>
> ret void<br>
> }<br>
><br>
><br>
> I propose to specify the volatile access behavior for those intrinsics :<br>
> instead of one flag, they should have 2 independant volatile flags, one<br>
> for destination accesses, the second for the source accesses.<br>
><br>
> If there is general agreement, I plan to proceed with the following steps :<br>
> 1. Specify the access behavior (this patch).<br>
> 2. Auto-upgrade the existing memcpy, memmove and memset intrinsics into<br>
> the more precise form by replicating the single volatile flag and rework<br>
> the MemIntrinsic hierarchy to provide (is|set)SrcVolatile(),<br>
> (is|set)DestVolatile() and implement (set|is)Volatile in terms of the<br>
> former 2 methods. This will conservatively preserve semantics. No<br>
> functional change so far. Commit 1 & 2.<br>
> 3. Audit all uses of isVolatile() / setVolatile() and move them to the<br>
> more precise form. From this point, more aggressive / precise<br>
> optimizations can happen. Commit 3.<br>
> 4. Teach clang to use the new form.<br>
> 5. Optionally remove the old interface form,as there should be no<br>
> in-tree users left. This would however be an API change breaking<br>
> external code.<br>
><br>
> Cheers,<br>
><br>
> --<br>
> Arnaud de Grandmaison<br>
><br>
</div></div>> <0001-Specify-the-access-behaviour-of-the-memcpy-memmove-a.patch>_______________________________________________<br>
> LLVM Developers mailing list<br>
> <a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
<br>
</blockquote></div><br></div>