[PATCH] D95142: [SLC] Simplify strcpy and friends with non-zero address spaces

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:647-649
+      if (DL &&
+          PTy->getPointerAddressSpace() != DL->getDefaultGlobalsAddressSpace())
+        return false;
----------------
jrtc27 wrote:
> arichardson wrote:
> > arsenm wrote:
> > > This wouldn't really be right for AMDGPU, but we have no library calls anyway. I also think it's not right to use GlobalsAddressSpace for this purpose. This has nothing to do with globals or creating new values
> > More than happy to drop this restriction and just accept any i8*.
> > 
> > I guess something like a "malloc/heap" address space would be more accurate since it's used when checking the `realloc` return value. It doesn't really matter too much for CHERI since we use 200 for program+alloca+globals.
> The issue is there's no notion of "the address space that libfuncs use". The additional granularity for globals vs stack (vs others too maybe?) is unhelpful here; for us they'll all be the same (AS 200), but there is no sensible mapping from C language `void *` (or, in this case, `char *`) to the AS in use.
I believe the current precedent assumes libcalls must use address space 0


================
Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:650
+        return false;
+      return PTy->getPointerElementType()->isIntegerTy(8);
+    }
----------------
jrtc27 wrote:
> arichardson wrote:
> > arsenm wrote:
> > > Should never depend on pointee element type
> > I know this will cause issues in the future, but I wasn't sure how else I can check for i8* ignoring the addres space.
> > 
> > What is the recommended way to avoid this? Or should I just check for ->isPointerType()?
> But then it's not a char pointer?
The pointee type is meaningless in the IR and will be going away. You can just ignore it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95142