[all-commits] [llvm/llvm-project] b827e7: [SelectOpti] Restrict load sinking

[Sotiris Apostolakis via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: b827e7c6007476504af67290cda2935810dfcacd
      https://github.com/llvm/llvm-project/commit/b827e7c6007476504af67290cda2935810dfcacd
  Author: Sotiris Apostolakis <apostolakis at google.com>
  Date:   2022-09-16 (Fri, 16 Sep 2022)

  Changed paths:
    M llvm/lib/CodeGen/SelectOptimize.cpp
    M llvm/test/CodeGen/X86/select-optimize.ll

  Log Message:
  -----------
  [SelectOpti] Restrict load sinking

This is a follow-up to D133777, which resolved a use-after-free case but
did not cover all possible memory bugs due to misplacement of loads.

In short, the overall problem was that sinked loads could be moved after
state-modifying instructions leading to memory bugs.

The solution is to restrict load sinking unless it is found to be sound.
i) Within a basic block (to-be-sinked load and select-user are in the same BB),
loads can be sinked only if there is no intervening state-modifying instruction.
This is a conservative approach to avoid resorting to alias analysis to detect
potential memory overlap.
ii) Across basic blocks, sinking of loads is avoided. This is because going over
multiple basic blocks looking for memory conflicts could be computationally
expensive and also unlikely to allow loads to sink. Further, experiments showed
that not sinking these loads has a slight positive performance effect.
Maybe for some of these loads, having some separation allows enough time
for the load to be executed in time for its user. This is not the case for
floating point operations that benefit more from sinking.

The solution in D133777 was essentially undone in this patch,
since the latter is a complete solution to the observed problem.

Overall, the performance impact of this patch is minimal.
Tested on two internal Google workloads with instrPGO.
Search application showed <0.05% perf difference,
while the database one showed a slight improvement,
but not statistically significant.

Reviewed By: davidxl

Differential Revision: https://reviews.llvm.org/D133999