<div dir="ltr">Note: I marked this patch as 'accepted' on Phab over an hour ago, but I haven't gotten an email about it, so sending 'LGTM' via email.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 4, 2018 at 4:36 AM, Jeremy Morse via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">jmorse updated this revision to Diff 145167.<br>
jmorse added a comment.<br>
<br>
Rebase against now-committed test case. The patch against the test is indeed much clearer now, it's obvious what the effect of the code change is.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D45022" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D45022</a><br>
<br>
Files:<br>
  lib/Target/X86/<wbr>X86ISelLowering.cpp<br>
  test/CodeGen/X86/pr30290.ll<br>
<br>
<br>
Index: test/CodeGen/X86/pr30290.ll<br>
==============================<wbr>==============================<wbr>=======<br>
--- test/CodeGen/X86/pr30290.ll<br>
+++ test/CodeGen/X86/pr30290.ll<br>
@@ -5,10 +5,6 @@<br>
 ; When broken, five "1" constants are written into the byval %struct.face,<br>
 ; but the subsequent byval read of that struct (call to bar) gets re-ordered<br>
 ; before those writes, illegally.<br>
-;<br>
-; FIXME: the output shown below is the broken output of llc, "movl $1" is<br>
-; scheduled after the copy between byval arguments starts. This will be fixed<br>
-; with the patch in review D45022.<br>
 source_filename = "test.c"<br>
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:<wbr>32:64-S128"<br>
 target triple = "x86_64-pc-linux-gnu"<br>
@@ -26,8 +22,8 @@<br>
 ; CHECK-NEXT:    .cfi_def_cfa_offset 48<br>
 ; CHECK-NEXT:    vmovaps {{.*#+}} xmm0 = [1,1,1,1]<br>
 ; CHECK-NEXT:    vmovaps %xmm0, {{[0-9]+}}(%rsp)<br>
+; CHECK-NEXT:    movl $1, {{[0-9]+}}(%rsp)<br>
 ; CHECK-NEXT:    vmovups {{[0-9]+}}(%rsp), %xmm0<br>
-; CHECK-NEXT:    movl $1, {{[0-9]+}}(%rsp)<br>
 ; CHECK-NEXT:    vmovups %xmm0, {{[0-9]+}}(%rsp)<br>
 ; CHECK-NEXT:    vmovaps {{[0-9]+}}(%rsp), %xmm0<br>
 ; CHECK-NEXT:    vmovups %xmm0, (%rsp)<br>
Index: lib/Target/X86/<wbr>X86ISelLowering.cpp<br>
==============================<wbr>==============================<wbr>=======<br>
--- lib/Target/X86/<wbr>X86ISelLowering.cpp<br>
+++ lib/Target/X86/<wbr>X86ISelLowering.cpp<br>
@@ -2839,7 +2839,11 @@<br>
   if (Flags.isByVal()) {<br>
     unsigned Bytes = Flags.getByValSize();<br>
     if (Bytes == 0) Bytes = 1; // Don't create zero-sized stack objects.<br>
-    int FI = MFI.CreateFixedObject(Bytes, VA.getLocMemOffset(), isImmutable);<br>
+<br>
+    // FIXME: For now, all byval parameter objects are marked as aliasing. This<br>
+    // can be improved with deeper analysis.<br>
+    int FI = MFI.CreateFixedObject(Bytes, VA.getLocMemOffset(), isImmutable,<br>
+                                   /*isAliased=*/true);<br>
     // Adjust SP offset of interrupt parameter.<br>
     if (CallConv == CallingConv::X86_INTR) {<br>
       MFI.setObjectOffset(FI, Offset);<br>
<br>
<br>
</blockquote></div><br></div>