[PATCH] D15730: [MachineLICM] Fix handling of memoperands

[Sanjoy Das via llvm-commits]

sanjoy added a comment.

In http://reviews.llvm.org/D15730#315930, @reames wrote:

> I'm going to move forward with the current change, but the point Sanjoy raises is entirely correct.  In fact, I've already found several more instances of this same bug while digging through related code.  I think we need to have three states (no memref, has memrefs, had memrefs/poison).  An MI should never be able to transition backwards through those states.


That, or we can have a special "everything" memref.  In that scheme, it would always be correct (but not optimal) to replace an arbitrary set of memrefs with one instance of the "everything" memref.  This would let you squash 256 memrefs into one if you want to.

(Side note: `NumMemRefs` is an `uint16_t` -- does this mean the limit is 65536, and not 256?)

> I'll sent a patch out that adds that, along with some assertions in the MachineVerifier and the access APIs.  I suspect they'll uncover a lot more bugs of this nature.  I suspect we're not seeing this in practice just because few MIs (even patchpoints and statepoints) actually have 256 unique memory locations they touch.



http://reviews.llvm.org/D15730