[PATCH] D82892: [NFC] Added comparision for all types in haveSameSpecialState() of Instruction.cpp

[Aditya Kumar via Phabricator via llvm-commits]

hiraditya added inline comments.


================
Comment at: llvm/lib/IR/Instructions.cpp:161
+  case Type::PointerTyID:
+    assert(PTyL && PTyR && "Both types must be pointers here.");
+    return compareIntegers(PTyL->getAddressSpace(), PTyR->getAddressSpace());
----------------
nikic wrote:
> The compareTypes() function looks a bit problematic to me. The issue is that MergeFunctions considers integer types and all pointer types of the same size equal. Other places in LLVM require that the type must be exactly identical. The function as implemented here seems to go some middle way, where pointers are not considered equal to integers, but all pointer types are the same. That's in the spirit of opaque pointers, but we're not there yet.
> Other places in LLVM require that the type must be exactly identical
Could you provide an instance where this is required. That will help formulate the right check here. The current check is replicating what MergeFunction did. If there's a bug, it'll be good to fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82892