[PATCH] D90776: [FastISel] Sink more materializations to first use

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

Thanks! How much of the benefit do you think comes from looking across flush points, and how much do you think comes from handling no-op casts?

I think we should also consider scrapping the local value map or maybe flushing it after selecting each instruction. I don't think we're getting a whole lot of value from sharing local values between multiple non-call instructions. Most non-call instructions are pointer, integer, and FP arithmetic. Most integer arithmetic supports folding immediates. Pointer arithmetic may benefit from reusing a value. Consider a series of GEPs (array/field accesses) built off the same global variable loaded from the GOT.

I think we can use the same measurement Paul used to show this change was useful: flush the map after every instruction, rebuild clang with a new compiler, measure the binary size and prevalence of line 0 source locations. If size goes down and line zero locations go down, the local value map is more trouble than it's worth, or, at least, it should be limited to memoizing values per instruction. If we flush the map after every instruction, we can confidently give local values the location of the current instruction, and we don't need to do any complex sinking. We should be able to delete this code.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp:256
-
-    // We don't need to order instructions past the last flush point.
-    if (I.getIterator() == LastFlushPoint)
----------------
This looks like it's reverting rGa28e767f06d. This code was added to avoid O(n^2) complexity. Do you have an alternative solution to that issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90776