[PATCH] D59730: [GVN] teach ConstantFolding correct handling of non-integral addrspace casts

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Analysis/ConstantFolding.cpp:101
     return Constant::getAllOnesValue(DestTy);
+  if (DL.isNonIntegralPointerType(C->getType()->getScalarType()) !=
+      DL.isNonIntegralPointerType(DestTy->getScalarType()))
----------------
This line is incorrect.  a) It volates the specification of the function (return ConstantExpr::getBitCast(C, DestTy);).  b) for the ASs not to match this would have to be an addrspace cast, not a bitcast.  Which shouldn't reach here.  


================
Comment at: lib/Transforms/Utils/VNCoercion.cpp:324
-  // memtransfer is implicitly a raw byte code
-  if (DL.isNonIntegralPointerType(LoadTy->getScalarType()))
-    // TODO: Can allow nullptrs from constant zeros
----------------
there's probably something missing here.  You're removing an early exit without introducing a new one.  Given we still need to bail in at least some cases, that's highly suspicious?


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

https://reviews.llvm.org/D59730