[PATCH] D33300: [ImplicitNullChecks] Uphold an invariant in areMemoryOpsAliased

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added inline comments.


================
Comment at: lib/CodeGen/ImplicitNullChecks.cpp:384
+        if (AR == AR_MayAlias) {
+          if (RedefinesPointerReg)
+            return SR_Impossible;
----------------
sanjoy wrote:
> skatkov wrote:
> > It seems that RedefinesPointerReg will check always the current MI, while you need a check of PrevMI.
> > 
> My intent was that as we scan down `NotNullSucc`, we will bail out of the entire transform (by returning `SR_Impossible`) the first time we see an instruction redefining `PointerReg`.  But this logic can be made clearer by adding an assert, which I will do in an update tomorrow.
Sanjoy, let's consier the first instruction in NotNullSucc, It might happen that it is a Suitable but canHoistInst will return false.
In this case no one ever will check whether it re-defines PointerReg, correct?


https://reviews.llvm.org/D33300