[PATCH] D55485: [X86] Fix avoid store forwarding blocks pass for negative displacements

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: lsaba, spatel, RKSimon.
Herald added a subscriber: llvm-commits.

Fixes https://bugs.llvm.org/show_bug.cgi?id=39926.

The size of the first copy was computed as `std::abs(std::abs(LdDisp2) - std::abs(LdDisp1))`, which results in skipped bytes if the signs of `LdDisp2` and `LdDisp1` differ. As far as I can see, this should just be `LdDisp2 - LdDisp1`. The case where `LdDisp1 > LdDisp2` is already handled in the code above, in which case `LdDisp2` is set to `LdDisp1` and this subtraction will evaluate to `Size1 = 0`, which is the correct value to skip an overlapping copy.


Repository:
  rL LLVM

https://reviews.llvm.org/D55485

Files:
  lib/Target/X86/X86AvoidStoreForwardingBlocks.cpp
  test/CodeGen/X86/pr39926.ll


Index: test/CodeGen/X86/pr39926.ll
===================================================================
--- /dev/null
+++ test/CodeGen/X86/pr39926.ll
@@ -0,0 +1,48 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=avx | FileCheck %s
+define i8 @test_offset(i8* %base) {
+; CHECK-LABEL: test_offset:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NEXT:    movb $0, 7(%rdi)
+; CHECK-NEXT:    movw $0, 5(%rdi)
+; CHECK-NEXT:    movl $0, 1(%rdi)
+; CHECK-NEXT:    movl -4(%rdi), %eax
+; CHECK-NEXT:    movl %eax, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:    movb (%rdi), %al
+; CHECK-NEXT:    movb %al, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:    movl 1(%rdi), %eax
+; CHECK-NEXT:    movl %eax, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:    movzwl 5(%rdi), %eax
+; CHECK-NEXT:    movw %ax, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:    movb 7(%rdi), %al
+; CHECK-NEXT:    movb %al, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:    movl 8(%rdi), %eax
+; CHECK-NEXT:    movl %eax, -{{[0-9]+}}(%rsp)
+; CHECK-NEXT:    movb -{{[0-9]+}}(%rsp), %al
+; CHECK-NEXT:    popq %rcx
+; CHECK-NEXT:    .cfi_def_cfa_offset 8
+; CHECK-NEXT:    retq
+entry:
+  %z = alloca [128 x i8], align 16
+  %gep0 = getelementptr inbounds i8, i8* %base, i64 7
+  store volatile i8 0, i8* %gep0
+  %gep1 = getelementptr inbounds i8, i8* %base, i64 5
+  %bc1 = bitcast i8* %gep1 to i16*
+  store volatile i16 0, i16* %bc1
+  %gep2 = getelementptr inbounds i8, i8* %base, i64 1
+  %bc2 = bitcast i8* %gep2 to i32*
+  store volatile i32 0, i32* %bc2
+
+  %y1 = getelementptr inbounds i8, i8* %base, i64 -4
+  %y2 = bitcast [128 x i8]* %z to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %y2, i8* %y1, i64 16, i1 false)
+
+  %gep4 = getelementptr inbounds [128 x i8], [128 x i8]* %z, i64 0, i64 4
+  %ret = load i8, i8* %gep4
+  ret i8 %ret
+}
+
+; Function Attrs: argmemonly nounwind
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i1)
Index: lib/Target/X86/X86AvoidStoreForwardingBlocks.cpp
===================================================================
--- lib/Target/X86/X86AvoidStoreForwardingBlocks.cpp
+++ lib/Target/X86/X86AvoidStoreForwardingBlocks.cpp
@@ -586,7 +586,7 @@
       StDisp2 += OverlapDelta;
       Size2 -= OverlapDelta;
     }
-    Size1 = std::abs(std::abs(LdDisp2) - std::abs(LdDisp1));
+    Size1 = LdDisp2 - LdDisp1;
 
     // Build a copy for the point until the current blocking store's
     // displacement.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55485.177406.patch
Type: text/x-patch
Size: 2562 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181209/341e8c70/attachment-0001.bin>