[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:755
     --NumParams;
-    if (!IsSizeTTy(FTy.getParamType(NumParams)))
+    if (!FTy.getParamType(NumParams)->isIntegerTy(SizeTBits))
       return false;
----------------
nikic wrote:
> arichardson wrote:
> > nikic wrote:
> > > I'd suggest to land these changes as an NFC refactoring -- just determine `SizeTBits` as `DL.getPointerSizeInBits()` for now.
> > That would be incorrect for CHERI (well if we hardcode AS0 it will happen to work since we keep AS0 as 64-bit integer pointers and use AS200 for capabilities).
> > It should IMO be `DL.getIndexSizeInBits(DL.getAllocaAddrSpace())` (or `getDefaultGlobalsAddressSpace()`) since that gives you `ptrdiff_t` which *should* be compatible with size_t.
> > 
> > One problem we have here is that there is no "libfunc address space" so there is no easy way to determine the expected address space for libfuncs without a pointer argument, etc. Should it be alloca/globals/something new?
> > 
> To be clear, I agree that this is incorrect, but that would be an NFC refactoring of the existing logic, so we can land the mass refactoring separately from the logic change.
Sorry if that wasn't clear here - I wasn't advising against the NFC refactoring just giving some context why the current code is wrong for CHERI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110585