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

[Serguei Katkov via Phabricator via llvm-commits]

skatkov requested changes to this revision.
skatkov added a comment.
This revision now requires changes to proceed.

It seems there is a bug.



================
Comment at: lib/CodeGen/ImplicitNullChecks.cpp:375
   for (auto *PrevMI : PrevInsts)
     for (auto &PrevMO : PrevMI->operands()) {
       // Check whether the current memory access aliases with previous one.
----------------
sanjoy wrote:
> I also noticed that there was no need to loop over `PrevMI->operands()` for the second clause here (`if (Suitable == SR_Suitable)` ...).  I'll remove that control flow in a next patch which should be NFC.
Agreed.


================
Comment at: lib/CodeGen/ImplicitNullChecks.cpp:384
+        if (AR == AR_MayAlias) {
+          if (RedefinesPointerReg)
+            return SR_Impossible;
----------------
It seems that RedefinesPointerReg will check always the current MI, while you need a check of PrevMI.



https://reviews.llvm.org/D33300