[PATCH] D148055: [BOLT] Fix creation of invalid CFG in presence of dead code

[Rafael Auler via Phabricator via llvm-commits]

rafauler updated this revision to Diff 512592.
rafauler added a comment.

Make test case run UCE to check that we are really removing the
unnecessary direct jump instruction


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148055

Files:
  bolt/lib/Core/BinaryFunction.cpp
  bolt/test/X86/unreachable-jmp.s


Index: bolt/test/X86/unreachable-jmp.s
===================================================================
--- /dev/null
+++ bolt/test/X86/unreachable-jmp.s
@@ -0,0 +1,66 @@
+# This checks that we don't create an invalid CFG when there is an
+# unreachable direct jump right after an indirect one.
+
+# REQUIRES: system-linux
+
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown \
+# RUN:   %s -o %t.o
+# RUN: link_fdata %s %t.o %t.fdata
+# RUN: llvm-strip --strip-unneeded %t.o
+# RUN: %clang %cflags -no-pie %t.o -o %t.exe -Wl,-q -nostdlib
+# RUN: llvm-bolt %t.exe -o %t.out --data %t.fdata \
+# RUN:     --eliminate-unreachable --print-cfg | FileCheck %s
+
+  .globl _start
+  .type _start, %function
+_start:
+  .cfi_startproc
+# FDATA: 0 [unknown] 0 1 _start 0 0 1
+  push  %rbp
+  mov   %rsp, %rbp
+  push  %rbx
+  push  %r14
+  subq  $0x20, %rsp
+  movq  %rdi, %rcx
+b:
+  jmpq  *JUMP_TABLE(,%rcx,8)
+# FDATA: 1 _start #b# 1 _start #hotpath# 0 20
+# Unreachable direct jump here. Our CFG should still make sense and properly
+# place this instruction in a new basic block.
+  jmp  .lbb2
+.lbb1:  je  .lexit
+.lbb2:
+  xorq  %rax, %rax
+  addq  $0x20, %rsp
+  pop %r14
+  pop %rbx
+  pop %rbp
+  ret
+hotpath:
+  movq  $2, %rax
+  addq  $0x20, %rsp
+  pop %r14
+  pop %rbx
+  pop %rbp
+  ret
+.lexit:
+  movq  $1, %rax
+  addq  $0x20, %rsp
+  pop %r14
+  pop %rbx
+  pop %rbp
+  ret
+  .cfi_endproc
+  .size _start, .-_start
+
+  .rodata
+  .globl JUMP_TABLE
+JUMP_TABLE:
+  .quad .lbb1
+  .quad .lbb2
+  .quad hotpath
+
+# No basic blocks above should have 4 successors! That is a bug.
+# CHECK-NOT:   Successors: {{.*}} (mispreds: 0, count: 20), {{.*}} (mispreds: 0, count: 0), {{.*}} (mispreds: 0, count: 0), {{.*}} (mispreds: 0, count: 0)
+# Check successful removal of stray direct jmp
+#  CHECK:  UCE removed 1 block
Index: bolt/lib/Core/BinaryFunction.cpp
===================================================================
--- bolt/lib/Core/BinaryFunction.cpp
+++ bolt/lib/Core/BinaryFunction.cpp
@@ -2038,6 +2038,7 @@
       MCInst *PrevInstr = PrevBB->getLastNonPseudoInstr();
       assert(PrevInstr && "no previous instruction for a fall through");
       if (MIB->isUnconditionalBranch(Instr) &&
+          !MIB->isIndirectBranch(*PrevInstr) &&
           !MIB->isUnconditionalBranch(*PrevInstr) &&
           !MIB->getConditionalTailCall(*PrevInstr) &&
           !MIB->isReturn(*PrevInstr)) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D148055.512592.patch
Type: text/x-patch
Size: 2424 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230411/73c5564a/attachment.bin>