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

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: lib/CodeGen/ImplicitNullChecks.cpp:362
 
-  // Finally, we need to make sure that the access instruction actually is
-  // accessing from PointerReg, and there isn't some re-definition of PointerReg
-  // between the compare and the memory access.
-  // If PointerReg has been redefined before then there is no sense to continue
-  // lookup due to this condition will fail for any further instruction.
-  SuitabilityResult Suitable = SR_Suitable;
-  for (auto *PrevMI : PrevInsts)
-    for (auto &PrevMO : PrevMI->operands()) {
-      if (PrevMO.isReg() && PrevMO.getReg() && PrevMO.isDef() &&
-          TRI->regsOverlap(PrevMO.getReg(), PointerReg))
-        return SR_Impossible;
-
-      // Check whether the current memory access aliases with previous one.
-      // If we already found that it aliases then no need to continue.
-      // But we continue base pointer check as it can result in SR_Impossible.
-      if (Suitable == SR_Suitable) {
-        AliasResult AR = areMemoryOpsAliased(MI, PrevMI);
-        if (AR == AR_WillAliasEverything)
-          return SR_Impossible;
-        if (AR == AR_MayAlias)
-          Suitable = SR_Unsuitable;
-      }
-    }
-  return Suitable;
+  // Finally, Check whether the current memory access aliases with previous one.
+  for (auto *PrevMI : PrevInsts) {
----------------
s/Check/check/


================
Comment at: test/CodeGen/X86/non-value-mem-operand.mir:154
+    CFI_INSTRUCTION def_cfa_offset 32
+    frame-setup PUSH64r killed %r13, implicit-def %rsp, implicit %rsp
+    CFI_INSTRUCTION def_cfa_offset 40
----------------
Do we need to keep the `CFI_INSTRUCTION` s?


https://reviews.llvm.org/D34385