[PATCH] D59661: [GVN] Try to be more principled about non-integral pointers

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

LGTM specifically to the noted parts only.  Remainder to be rebased on landed parts and reviewed after a week or so of burn confirms belief named parts are NFC.



================
Comment at: lib/Transforms/Utils/VNCoercion.cpp:49
+      if (CI->isNullValue())
+        if (StoredTy == LoadTy || DL.getTypeSizeInBits(StoredTy) >= DL.getTypeSizeInBits(LoadTy))
+          return true;
----------------
Consistency: we should probably be avoiding mmx here based on constant folding logic and langref, but given the original code being restructured doesn't handle this, I'm okay with this being done as followup.  


================
Comment at: lib/Transforms/Utils/VNCoercion.cpp:52
+    return canCoerceMustAliasedValueToLoad(StoredTy, LoadTy, DL);
+}
+
----------------
I'm okay with the above split landing as a NFCI, then reviewing the rest.


================
Comment at: lib/Transforms/Utils/VNCoercion.cpp:330
   LLVMContext &Ctx = SrcVal->getType()->getContext();
+  if (auto *CI = dyn_cast<Constant>(GetUnderlyingObject(SrcVal, DL)))
+    if (CI->isNullValue())
----------------
This are grouped with the split above.  


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59661/new/

https://reviews.llvm.org/D59661