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

[Alexander Richardson via Phabricator via llvm-commits]

arichardson added inline comments.


================
Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:788
                                                    const Module &M) const {
-  // FIXME: There is really no guarantee that sizeof(size_t) is equal to
-  // sizeof(int*) for every target. So the assumption used here to derive the
-  // SizeTBits based on the size of an integer pointer in address space zero
-  // isn't always valid.
-  unsigned SizeTBits = M.getDataLayout().getPointerSizeInBits(/*AddrSpace=*/0);
+  unsigned SizeTBits = getSizeTSize(M, /*AddressSpace=*/0);
   unsigned NumParams = FTy.getNumParams();
----------------
I'm not particularly keen on keeping the hardcoded zero in all callers. I am not sure if having different size_t widths depending on the address space is even possible.

Maybe we should just drop the address space argument and use DL.getDefaultGlobalsAS or DL.getAllocaAS (or possibly add a new library-function address space to the datalayout).


================
Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:1801
+
+  // Here we assume that sizeof(size_t) is equal to sizeof(int*) in address
+  // space zero. This should work for most targets.
----------------
no longer hardcodes address space zero, so this comments needs updating.


================
Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:1805
+  const DataLayout &DL = M.getDataLayout();
+  return DL.getPointerSizeInBits(AddressSpace);
+}
----------------
I think this should be the index size. If that doesn't work for you, you could special-case it based on your target triple?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110585