[all-commits] [llvm/llvm-project] 7564a9: Fix assertion in GCStrategy.

[Campbell Suter via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 7564a9ad30d3c03d0b4d0693908c00d1b85c98d4
      https://github.com/llvm/llvm-project/commit/7564a9ad30d3c03d0b4d0693908c00d1b85c98d4
  Author: Denis Antrushin <dantrushin at gmail.com>
  Date:   2023-01-25 (Wed, 25 Jan 2023)

  Changed paths:
    M llvm/include/llvm/IR/GCStrategy.h

  Log Message:
  -----------
  Fix assertion in GCStrategy.

It meant to check that `UseRS4GC` requires `UseStatepoints`.
Instead it always required `UseStatepoints` when `useRS4GC()`
was called.


  Commit: 7092dae032290dd4cdc99d7573e02687f2ab0c76
      https://github.com/llvm/llvm-project/commit/7092dae032290dd4cdc99d7573e02687f2ab0c76
  Author: Campbell Suter <znix at znix.xyz>
  Date:   2023-01-25 (Wed, 25 Jan 2023)

  Changed paths:
    M llvm/docs/Statepoints.rst
    M llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

  Log Message:
  -----------
  [RS4GC] Remove the hardcoded GC strategy names (v2)

Previously, RewriteStatepointsForGC had a hardcoded list of GC
strategies for which it would run, and using it with a custom strategy
required patching LLVM.

The logic for selecting the variables that are considered managed was
also hardcoded to use pointers in address space 1, rather than
delegating to GCStrategy::isGCManagedPointer.

This patch fixes both of these flaws: this pass now applies to all
functions whose GCStrategy returns true for useStatepoints, and checking
if a pointer is managed or not is also now done by the strategy.

One potentially questionable design decision in this change: the pass will
be enabled for all GC strategies that use statepoints. It seems unlikely
this would be a problem - consumers that don't use this pass probably
aren't adding it to the pass manager anyway - but if you had two different
GC strategies and only one wants this pass enabled then that'd need a new
flag in GCStrategy, which I can add if anyone thinks it's necessary.

This is an updated version of D140458, rebased to account for LLVM's
changes since D140504 (required by this patch) landed.

Reviewed By: dantrushin

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


Compare: https://github.com/llvm/llvm-project/compare/1799a714a7a7...7092dae03229