Simplify scalar indexing #19958
Simplify scalar indexing #19958
Conversation
|
Hm, retargeted to master to get CI to run. |
|
@nanosoldier |
|
|
|
Oh my goodness. I should not be trying to do this on my phone. Thanks Kristoffer. @nanosoldier |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
|
Ah, of course, Nanosoldier uses the merge commit from the PR. Those improvements are certainly unrelated. The regressions are likely real. |
mbauman
force-pushed
the
mb/simpler-scalar-indexing
branch
from
1154b92 to
bc565e1
Compare
|
@nanosoldier |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
|
Windows failure is a timeout. Might be worth restarting it. |
|
What's the status on this one, will this be refactored? |
|
The first commit is still a nice simplification. The second commit is messy and unnecessary if we do the partial linear indexing deprecation within checkbounds… and it's also likely the cause of the performance regressions. |
mbauman
force-pushed
the
mb/simpler-scalar-indexing
branch
2 times, most recently
from
e9477be to
853e920
Compare
|
@nanosoldier |
mbauman
force-pushed
the
mb/simpler-scalar-indexing
branch
from
853e920 to
bed4377
Compare
mbauman
changed the title
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
|
Bump – Is this ready to go? |
|
Yup, but it's just an internal refactoring that touches some pretty foundational methods, so I had been thinking it could wait for the branch. It's totally ready to go, though… so anyone is welcome to push that button! I'll re-start CI since the last result is getting a little stale. |
|
+37 -98 is a reason to merge in my opinion... |
This removes the generated functions for scalar indexing
and modifies Array indexing to get it ready for the deprecation of generalized linear indexing (#14770). Let's see if CI likes this. Last time I tried this simplification Win64 didn't like it. And then we can send it through nanosoldier.This is currently targeting my indexing conversion branch, so nanosoldier should be compared against that, not master.