[PATCH] D155790: PreISelIntrinsicLowering: don't expand memcpys in minsize functions, even with no-builtins.

Thu Jul 20 16:22:46 PDT 2023

arsenm added inline comments.

================
Comment at: llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp:201
+    // so just avoid expanding here if the caller is a minsize function.
+    bool IsMinSize = Inst->getFunction()->hasMinSize();

----------------
aemerson wrote:
> arsenm wrote:
> > Lowering the intrinsic when the libcall isn't available is mandatory, you can't simply skip it.
> > 
> > For minsize, you could adjust the default size threshold. Also, we could inject into the module an implementation you expand into, so the expansion happens fewer times
> no-builtins is attached to the Function with -fno-builtins right? However the semantics of -fno-builtins is not AFAICT that we need to expand all memcpy intrinsics. The semantics are that clang shouldn't try to recognize things like memcpy loops and turn them into intrinsics. Or that plain memcpy calls shouldn't be converted into intrinsics.
This whole thing is a confusing mess, there's a thread about it going on over in D74162.

"no-builtins" isn't in the LangRef and I don't really understand how it's supposed to differ from `nobuiltin`.

I think we have 2 blurred concepts of which calls are recognized and which calls can be emitted. The call here should probably be considering "RuntimeLibcalls" not TLI, but that doesn't seem to be written down anywhere

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155790