[PATCH] D28546: LowerTypeTests: Implement importing of type identifiers.

[Evgeniy Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:735
+LowerTypeTestsModule::TypeIdLowering
+LowerTypeTestsModule::importTypeId(StringRef TypeId) {
+  TypeTestResolution &TTRes = Summary->getTypeIdSummary(TypeId).TTRes;
----------------
This function does not import SizeM1BitWidth.
I realize it is unused, but this could be a surprise for someone later, comments in TypeIdLowering say that it should be set for ByteArray, Inline, AllOnes.
Do we even need this field? It looks like it could be reconstructed from SizeM1.




================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:746
+    auto *GV = dyn_cast<GlobalVariable>(C);
+    if (!GV || GV->getVisibility() == GlobalValue::HiddenVisibility)
+      return C;
----------------
I don't get this condition. Why would this variable exist and be non-hidden?


================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:758
+    if (AbsWidth == PtrWidth)
+      SetAbsRange(~0ull, ~0ull);
+    else if (AbsWidth)
----------------
should this say 0ull, ~0ull ?


https://reviews.llvm.org/D28546