[PATCH] D110585: [Analysis][SimplifyLibCalls] Refactor code related to size_t in lib func signatures. NFCI

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

In D110585#3028146 <https://reviews.llvm.org/D110585#3028146>, @nikic wrote:

> As I mentioned on the other review, I think that `DL.getIndexSizeInBits()` should be used for the size_t size. If we do that, do we really need a TLI hook?

At least for my target getIndexSize won't match with size_t either. I made a quick experiment with changing the DataLayout to set the index size to something different than pointer size, but that was not so easy because I got lots of failures. At least from LoadStoreVectorizer (Vectorizer::areConsecutivePointers -> Value::stripAndAccumulateConstantOffsets -> assert "The offset bit width does not match the DL specification:"). So it might take some time to fix all such problems (and we've been have IndexSize equals PointerSize but not size of size_t in bits since the beginning of time afaict).

I thought the TLI hook was useful to give this lookup a name and having the logic for how to find size of size_t in one place, regardless. As currently I think we agree that the faulty assumption of getIntPtrType=>"type for size_t" is incorrect, and that assumption has been used in more than one place (and by now it is kind of tricky to find if that has been done in several more places).
If everyone is expected to know about "IndexSize is supposed to be matching size_t", then I agree that the helper just will appear as an unneeded extra wrapper. But I've never seen, or heard, anything about that earlier except in these reviews. I figure that the `getSizeTSize` hook will be the place where we kind of define/document that relation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110585