[PATCH] D91734: [FastISel] Flush local value map on every instruction

Wed Nov 18 11:45:27 PST 2020

probinson created this revision.
probinson added reviewers: rnk, echristo.
probinson added a project: debug-info.
Herald added subscribers: pengfei, arphaman, atanasyan, jrtc27, hiraditya, nemanjai, sdardis, qcolombet.
Herald added a project: LLVM.
probinson requested review of this revision.

Local values are constants or addresses that can't be folded into
the instruction that uses them. FastISel materializes these in a
"local value" area that always dominates the current insertion
point, to try to avoid materializing these values more than once
(per block).

https://reviews.llvm.org/D43093 added code to sink these local
value instructions to their first use, which has two beneficial
effects. One, it is likely to avoid some unnecessary spills and
reloads; two, it allows us to attach the debug location of the
user to the local value instruction. The latter effect can
improve the debugging experience for debuggers with a "set next
statement" feature, such as the Visual Studio debugger and PS4
debugger, because instructions to set up constants for a given
statement will be associated with the appropriate source line.

There are also some constants (primarily addresses) that could be
produced by no-op casts or GEP instructions; the main difference
from "local value" instructions is that these are values from
separate IR instructions, and therefore could have multiple users
across multiple basic blocks. D43093 <https://reviews.llvm.org/D43093> avoided sinking these, even
though they were emitted to the same "local value" area as the
other instructions. The patch comment for D43093 <https://reviews.llvm.org/D43093> states:

  Local values may also be used by no-op casts, which adds the
  register to the RegFixups table. Without reversing the RegFixups
  map direction, we don't have enough information to sink these
  instructions.

This patch undoes most of D43093 <https://reviews.llvm.org/D43093>, and instead flushes the local
value map after(*) every IR instruction, using that instruction's
debug location.  This avoids sometimes incorrect locations used
previously, and emits instructions in a more natural order.

This does mean materialized values are not re-used across IR
instruction boundaries; however, only about 5% of those values
were reused in an experimental self-build of clang.

(*) Actually, just prior to the next instruction.  It seems like
it would be cleaner the other way, but I was having trouble
getting that to work.

There is additional cleanup to be done after the functional
patch; the SinkLocalValues option can go away, as well as
the LastFlushPoint member.  I figured the functional patch
is big enough as it is, and these NFC follow-ups would be easy.
be

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91734

Files:
  llvm/include/llvm/CodeGen/FastISel.h
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/test/CodeGen/AArch64/arm64-abi_align.ll
  llvm/test/CodeGen/AArch64/arm64-elf-globals.ll
  llvm/test/CodeGen/AArch64/arm64-fast-isel-call.ll
  llvm/test/CodeGen/AArch64/arm64-fast-isel-gv.ll
  llvm/test/CodeGen/AArch64/arm64-fast-isel-intrinsic.ll
  llvm/test/CodeGen/AArch64/arm64-fast-isel.ll
  llvm/test/CodeGen/AArch64/arm64-patchpoint-webkit_jscc.ll
  llvm/test/CodeGen/AArch64/cfguard-checks.ll
  llvm/test/CodeGen/AArch64/large-stack.ll
  llvm/test/CodeGen/ARM/fast-isel-call.ll
  llvm/test/CodeGen/ARM/fast-isel-intrinsic.ll
  llvm/test/CodeGen/ARM/fast-isel-ldr-str-thumb-neg-index.ll
  llvm/test/CodeGen/ARM/fast-isel-ldrh-strh-arm.ll
  llvm/test/CodeGen/ARM/fast-isel-select.ll
  llvm/test/CodeGen/ARM/fast-isel.ll
  llvm/test/CodeGen/Mips/Fast-ISel/callabi.ll
  llvm/test/CodeGen/Mips/Fast-ISel/fastalloca.ll
  llvm/test/CodeGen/Mips/Fast-ISel/fpcmpa.ll
  llvm/test/CodeGen/Mips/Fast-ISel/icmpa.ll
  llvm/test/CodeGen/Mips/Fast-ISel/logopm.ll
  llvm/test/CodeGen/Mips/Fast-ISel/overflt.ll
  llvm/test/CodeGen/Mips/Fast-ISel/shftopm.ll
  llvm/test/CodeGen/Mips/Fast-ISel/simplestore.ll
  llvm/test/CodeGen/Mips/Fast-ISel/simplestorei.ll
  llvm/test/CodeGen/Mips/emergency-spill-slot-near-fp.ll
  llvm/test/CodeGen/PowerPC/elf-common.ll
  llvm/test/CodeGen/PowerPC/fast-isel-load-store.ll
  llvm/test/CodeGen/PowerPC/mcm-1.ll
  llvm/test/CodeGen/PowerPC/mcm-13.ll
  llvm/test/CodeGen/PowerPC/mcm-2.ll
  llvm/test/CodeGen/PowerPC/mcm-3.ll
  llvm/test/CodeGen/PowerPC/mcm-6.ll
  llvm/test/CodeGen/PowerPC/mcm-9.ll
  llvm/test/CodeGen/PowerPC/mcm-default.ll
  llvm/test/CodeGen/X86/atomic-unordered.ll
  llvm/test/CodeGen/X86/atomic64.ll
  llvm/test/CodeGen/X86/crash-O0.ll
  llvm/test/CodeGen/X86/fast-isel-call-cleanup.ll
  llvm/test/CodeGen/X86/fast-isel-constant.ll
  llvm/test/CodeGen/X86/fast-isel-mem.ll
  llvm/test/CodeGen/X86/fast-isel-select.ll
  llvm/test/CodeGen/X86/lvi-hardening-loads.ll
  llvm/test/CodeGen/X86/membarrier.ll
  llvm/test/CodeGen/X86/pr32241.ll
  llvm/test/CodeGen/X86/pr32256.ll
  llvm/test/CodeGen/X86/pr32284.ll
  llvm/test/CodeGen/X86/pr32340.ll
  llvm/test/CodeGen/X86/pr44749.ll
  llvm/test/CodeGen/X86/volatile.ll
  llvm/test/DebugInfo/COFF/lines-bb-start.ll
  llvm/test/DebugInfo/Mips/delay-slot.ll
  llvm/test/DebugInfo/X86/fission-ranges.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D91734.306170.patch
Type: text/x-patch
Size: 125115 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201118/15bbb29c/attachment-0001.bin>