[PATCH] D36900: Re-land MachineInstr: Reason locally about some memory objects before going to AA.

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: lib/CodeGen/MachineInstr.cpp:1703
+
+  assert((OffsetA >= 0) && "Negative MachineMemOperand offset");
+  assert((OffsetB >= 0) && "Negative MachineMemOperand offset");
----------------
These asserts are related to the interface to AA, right? (I think it says so in the comments above)

Are they also important for the new new checks you have added, or could/should the asserts perhaps be moved closer to the AA->alias call?
Now it kind of looks like the asserts are guarding your new checks, but the comments are referring to assumptions made by the API to AA.

(Some more background to why I'm asking:)
Our out-of-tree target is actually creating StackPointer relative MachingMemOperands with negative offsets. In the past we never hit those asserts since the ealier (!MMOa->getValue() || !MMOb->getValue()) check would make us return before checking for negative operands.
I do not think that we need those negative offsets, so I'll probably try to get rid of them regardless of this patch. But as long as you put the asserts before the "if (!ValA || !ValB)", then we must stop using negative offsets in PseudoSourceValue kind of MachingMemOperands or we will hit the assert.


https://reviews.llvm.org/D36900