[PATCH] D45022: [X86] Mark all byval parameters as aliased

[Jeremy Morse via Phabricator via llvm-commits]

jmorse updated this revision to Diff 145031.
jmorse added a comment.

Improves comments in test description, use utils/update_llc_test_checks.py to generate CHECK lines in test.

@spatel just to confirm I understand you correctly, you're saying to commit the test first (where it would fail) followed by the code change that fixes it, to discourage unconsidered reversion, yes?


https://reviews.llvm.org/D45022

Files:
  lib/Target/X86/X86ISelLowering.cpp
  test/CodeGen/X86/pr30290.ll


Index: test/CodeGen/X86/pr30290.ll
===================================================================
--- /dev/null
+++ test/CodeGen/X86/pr30290.ll
@@ -0,0 +1,40 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mcpu=btver2 %s -o - | FileCheck %s
+; Test desc: two functions (foo, bar) with byval arguments, should not have
+; reads/writes from/to byval storage re-ordered.
+; When broken, five "1" constants are written into the byval %struct.face,
+; but the subsequent byval read of that struct (call to bar) gets re-ordered
+; before those writes, illegally.
+source_filename = "test.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+%struct.face = type { [7 x i32] }
+
+; Function Attrs: noinline nounwind uwtable
+declare void @bar(%struct.face* byval nocapture readonly align 8);
+
+; Function Attrs: noinline nounwind uwtable
+define void @foo(%struct.face* byval nocapture align 8) local_unnamed_addr {
+; CHECK-LABEL: foo:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    subq $40, %rsp
+; CHECK-NEXT:    .cfi_def_cfa_offset 48
+; CHECK-NEXT:    vmovaps {{.*#+}} xmm0 = [1,1,1,1]
+; CHECK-NEXT:    vmovaps %xmm0, {{[0-9]+}}(%rsp)
+; CHECK-NEXT:    movl $1, {{[0-9]+}}(%rsp)
+; CHECK-NEXT:    vmovups {{[0-9]+}}(%rsp), %xmm0
+; CHECK-NEXT:    vmovups %xmm0, {{[0-9]+}}(%rsp)
+; CHECK-NEXT:    vmovaps {{[0-9]+}}(%rsp), %xmm0
+; CHECK-NEXT:    vmovups %xmm0, (%rsp)
+; CHECK-NEXT:    callq bar
+; CHECK-NEXT:    addq $40, %rsp
+; CHECK-NEXT:    .cfi_def_cfa_offset 8
+; CHECK-NEXT:    retq
+  %2 = bitcast %struct.face* %0 to <4 x i32>*
+  store <4 x i32> <i32 1, i32 1, i32 1, i32 1>, <4 x i32>* %2, align 8
+  %3 = getelementptr inbounds %struct.face, %struct.face* %0, i64 0, i32 0, i64 4
+  store i32 1, i32* %3, align 8
+  call void @bar(%struct.face* byval nonnull align 8 %0)
+  ret void
+}
Index: lib/Target/X86/X86ISelLowering.cpp
===================================================================
--- lib/Target/X86/X86ISelLowering.cpp
+++ lib/Target/X86/X86ISelLowering.cpp
@@ -2839,7 +2839,11 @@
   if (Flags.isByVal()) {
     unsigned Bytes = Flags.getByValSize();
     if (Bytes == 0) Bytes = 1; // Don't create zero-sized stack objects.
-    int FI = MFI.CreateFixedObject(Bytes, VA.getLocMemOffset(), isImmutable);
+
+    // FIXME: For now, all byval parameter objects are marked as aliasing. This
+    // can be improved with deeper analysis.
+    int FI = MFI.CreateFixedObject(Bytes, VA.getLocMemOffset(), isImmutable,
+                                   /*isAliased=*/true);
     // Adjust SP offset of interrupt parameter.
     if (CallConv == CallingConv::X86_INTR) {
       MFI.setObjectOffset(FI, Offset);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45022.145031.patch
Type: text/x-patch
Size: 2735 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180503/092dd95e/attachment.bin>