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

[Paul Robinson via Phabricator via llvm-commits]

probinson added a comment.

> 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.

Toward the end of my work to come up with this patch, I was also thinking the value map is quite likely more trouble than it's worth.  It increases register pressure, and once you start spilling the values, that ends up costing more than rematerializing would.  Flushing the map on every instruction is an alternative that I thought of fairly late in this process (it has taken a couple of weeks to work out what was going on).  I will probably try that experiment next week (I'm off tomorrow).



================
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)
----------------
rnk wrote:
> 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?
I was not aware of that, thanks for pointing it out.  The need to number the entire block comes up when the value instruction is not used until after the next call.  That typically happens in a case like `foo(&a, bar(&b));` where we compute `&a` before we start doing the call to `bar`.  The PR37010 example has many thousands of those in a row, and we flush every time we see a call, so my patch will certainly run into this problem.
On the other hand... if we could manage to preserve the original source info for the `&a` then we wouldn't need to do this sinking hack at all.  Sinking the instruction doesn't recover the original source location, it gives us something sorta usable but not really correct.  So I'd rather not sink these things at all, actually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90776