[PATCH] D38374: [memcpyopt] Memcpy-memcpy dependence isn't detected across basic blocks

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

"Your suggestions above both involve adding functionality under a flag/option. That alone makes them different situations from just landing the current patch, in terms of seeing the work all the way through into production.
"

(Rust could easily enable that option, of course)
Past that, yes, it is.
But like I said, that's sometimes how you prevent a tragedy of the commons.

"The patch doesn't make anything about the MemorySSA-based patches harder to work on"
First, of course it does.
It's yet another thing to keep working or replace.  Both in in  time spend keeping this working and fixing issues it exposes over time vs spending improving the infrastructure, and in cost to build a replacement.

You are not signing up to maintain memcpyopt and improve it.  That's literally what you are saying above.   You are making an improvement you want and then going to the next thing. You see that as an argument in favor of this patch: You can just commit it and move on, and leave the maintenance/support to others.
I'm not saying that as a value judgement, but as an explanation of why it's not anywhere near as simple as you make it out to be.

What you've said  is just another way of saying "I don't want to have to sign up for that". Which again, i get. Really! 
But if you want the people who basically *have signed up* to fix bugs and keep this stuff working, to be willing to accept it, it also means helping them out time to time.

So if you want me to approve it, i'd like to know it's the right path. Small or large. my line has to be drawn somewhere, otherwise, everything just incrementally gets bigger without any stopping point or design reconsideration.
This is where i am drawing the line for me to approve it - knowing that a thing we believe is a better path doesn't already solve your problem.
I haven't actually asked you to go productionize that (despite your concern that i haven). I've asked you simply to test whether it works and whether that path is feasible. If it works, in fact, i'm happy to commit to productionizing and getting it in in the next week or so.

If someone else who maintains this stuff wants to draw a different line and be more pragmatic, that's fine by me too.
I'm only telling you what i would want to see in this case.


Repository:
  rL LLVM

https://reviews.llvm.org/D38374