Instead of "handle updating uses", can we just say "Updates uses"?
How about we call this updateMemIntrinsicUse, or updateIfIsMemIntrinsic? We can add other stuff to other functions.
The nesting here, and particularly the llvm_unreachable after the else below, is pretty hard for me to read.
Could we restructure the control flow as
auto *MI = dyn_cast<MemIntrinsic>(&U); if (!MI || MI->isVolatile()) return false; ...
auto* with dyn_cast, here and elsewhere?
Would you mind adding /*foo=*/false here and below? See my now five-year-old screed, http://jlebar.com/2011/12/16/Boolean_parameters_to_API_functions_considered_harmful..html
It's not just intrinsic users, right? Regular Instructions could have the same problem?
How would you feel if we restructured this to be
Use &U = *I; ++I; while (I != E && I->getUser() == CurUser) ++I; if (updateSimplePtrUse(U) || updateMemIntrinsicUse(U)) continue; ...
This seems to be a consistent level of abstraction, instead of having one function which does an update and the other which just does a check and leaves the update to the caller. And it's also obvious how to extend this code to handle other cases -- just add a new updateFoo function to the if statement.
My definition of "SimplePtrUse" was that it has a single pointer operand that can be trivially replaced, so this was specifically avoiding that problem and treating multi users as the complex case. My patches for icmp/select have that problem, and they are handled separately from updateSimplePtrUse
I understand, but there's no harm in running the loop even in this case, right?
It just seems to me much simpler conceptually to say: