[PATCH] D134697: [X86] Consider branch probability in avoiding Store Forward Block

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/test/CodeGen/X86/avoid-sfb-g-no-change3.mir:211-222
-    ; CHECK-LABEL: name: debug
-    ; CHECK: %3:gr8 = MOV8rm
-    ; CHECK: MOV8mr
-    ; CHECK: %4:gr64 = MOV64rm
-    ; CHECK: MOV64mr
-    ; CHECK: %5:gr32 = MOV32rm
-    ; CHECK: MOV32mr
----------------
The test looks not affected by this patch. Should exclude it to reduce the noise?


================
Comment at: llvm/test/CodeGen/X86/avoid-sfb.ll:2-5
 ; RUN: llc < %s -mtriple=x86_64-linux -mcpu=x86-64 -verify-machineinstrs | FileCheck %s -check-prefix=CHECK
 ; RUN: llc < %s -mtriple=x86_64-linux -mcpu=x86-64 --x86-disable-avoid-SFB -verify-machineinstrs | FileCheck %s --check-prefix=DISABLED
 ; RUN: llc < %s -mtriple=x86_64-linux -mcpu=core-avx2 -verify-machineinstrs | FileCheck %s -check-prefix=CHECK-AVX2
 ; RUN: llc < %s -mtriple=x86_64-linux -mcpu=skx -verify-machineinstrs | FileCheck %s -check-prefix=CHECK-AVX512
----------------
We can merge the checks by providing a common prefix. e.g.:
```
-check-prefixes=SSE,CHECK
-check-prefixes=SSE,DISABLED
-check-prefixes=AVX,CHECK-AVX2
-check-prefixes=AVX,CHECK-AVX512
```


================
Comment at: llvm/test/CodeGen/X86/avoid-sfb.ll:23-24
 ; CHECK-NEXT:    movups %xmm0, (%rcx)
-; CHECK-NEXT:    movl (%rdi), %eax
-; CHECK-NEXT:    movl %eax, (%rsi)
-; CHECK-NEXT:    movl 4(%rdi), %eax
-; CHECK-NEXT:    movl %eax, 4(%rsi)
-; CHECK-NEXT:    movq 8(%rdi), %rax
-; CHECK-NEXT:    movq %rax, 8(%rsi)
+; CHECK-NEXT:    movups (%rdi), %xmm0
+; CHECK-NEXT:    movups %xmm0, (%rsi)
 ; CHECK-NEXT:    retq
----------------
Are these changes in this file expected? I'm not sure where's the branch probability from and whether it makes sense or not.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134697/new/

https://reviews.llvm.org/D134697