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

[Philip Reames via llvm-commits]

reames added a comment.

Original patch submitted, but let's keep the discussion going. Once we settle on what should be, I'm going to prepare a follow on patch to clarify/enforce invariants.

In http://reviews.llvm.org/D15730#315946, @atrick wrote:

> Here's my current understanding, but I'm open to other possibilities.
>
> - Empty memoperands should be interpreted as the most conservative state.
> - Going from zero to non-zero memoperands would indeed be an error.


I'm not sure this will work in practice.  I haven't tried yet, but I believe we're likely to use zero as a starting state and then add all the needed operands.  We do need a conservative state, but I suspect that will have to be a separate explicit poison state.

> 

> 

> - Casual pass writers often forget to propagate memoperands entirely (naïve sloppiness is ok), but dropping only some of them and not all of them would be incorrect (conscious sloppiness is an error).

> - addMemOperand is supposed to be an "internal" API, not called directly from passes.


This doesn't look to be actual true.  Or at least, "internal" means a lot more code than I'd think it should...

> 

> 

> - Merging memory accesses cannot be done naïvely.


Can you describe how it can be done at all?  I don't know today.

> 

> 

> - We should work hard to avoid empty memoperands, and to that end we could introduce PseudoSourceValues with specific meanings. I certainly don't think that overflowing memoperands should result in a zero memoperands state, which as Sanjoy pointed out is more error prone.


How do we avoid this?  Do we introduce more abstract/less precise PSVs?  I'm open to ideas here; I don't really claim to understand what all we're using this for.


Repository:
  rL LLVM

http://reviews.llvm.org/D15730