Skip to content

Commit

Permalink
JIT: Update CSE to use reaching definition information
Browse files Browse the repository at this point in the history
Now that CSE always inserts into SSA we can update it to make use of the
reaching definition information that it has access to. CSE already spent
effort to track some extra information to try to do this, which we can
remove.

- Remove `optCSECheckedBoundMap`: this was used by CSE to try to update
  conservative VNs of ancestor bounds checks. This is unnececssary since
  all descendants of the CSE definitions should get the same
  conservative VNs automatically now.
- Remove `CSEdsc::defConservNormVN`; this was used to update
  conservative VNs in the single-def case, which again is unnecessary
  now

Making this change requires a bit of refactoring to the incremental SSA
builder. Before this PR the builder takes all defs and all uses and then
inserts everything into SSA. After this change the builder is used in a
multi-step process as follows:
1. All definitions are added with `IncrementalSsaBuilder::InsertDef`
2. The definitions are finalized with
   `IncrementalSsaBuilder::FinalizeDefs`
3. Uses are inserted (one by one) with
   `IncrementalSsaBuilder::InsertUse`. No finalization are necessary;
   each use is directly put into SSA as a result of calling this
   method.

The refactoring allows CSE to use the incremental SSA builder such that
it can access reaching definition information for each of the uses as
part of making replacements. However, this still requires some
refactoring such that CSE performs replacements of all defs before
performing the replacements of all uses.

Additionally, this PR fixes various incorrect VN updating made by CSE.

Fix dotnet#109745
  • Loading branch information
jakobbotsch committed Nov 19, 2024
1 parent 63d93b7 commit bcd1d0b
Show file tree
Hide file tree
Showing 6 changed files with 388 additions and 553 deletions.
8 changes: 0 additions & 8 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -7272,14 +7272,6 @@ class Compiler
CSEdsc** optCSEhash;
CSEdsc** optCSEtab;

typedef JitHashTable<GenTree*, JitPtrKeyFuncs<GenTree>, GenTree*> NodeToNodeMap;

NodeToNodeMap* optCseCheckedBoundMap; // Maps bound nodes to ancestor compares that should be
// re-numbered with the bound to improve range check elimination

// Given a compare, look for a cse candidate checked bound feeding it and add a map entry if found.
void optCseUpdateCheckedBoundMap(GenTree* compare);

void optCSEstop();

CSEdsc* optCSEfindDsc(unsigned index);
Expand Down
Loading

0 comments on commit bcd1d0b

Please sign in to comment.