[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

Guillaume Chatelet via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 7 08:13:36 PDT 2019


gchatelet added a comment.

In D61634#1493350 <https://reviews.llvm.org/D61634#1493350>, @theraven wrote:

> I wonder if a list of comma-separated names is the right approach.  Would it make more sense to add a new attribute for each of the helpers, such as `#no-runtime-for-memcpy`? That should make querying simpler (one lookup in the table, rather than a lookup and a string scan) and also make it easier to add and remove attributes (merging is now just a matter of trying to add all of them, with the existing logic to deduplicate repeated attributes working).


So I decided to go that route for two reasons:

- The `no_runtime_for` attribute will be used almost exclusively by runtime implementers, on average lookup will return false and the parsing part should be marginal (famous last words?)
- We need to support every function in TargetLibraryInfo <https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Analysis/TargetLibraryInfo.def> (I count 434 of them) and I'm not sure adding one attribute per function will scale or can stay in sync.

Now I haven't thought about merging indeed, we may be able to reuse or clone the approach used for `target-features`?
For instance some backend disable specific functions and we may warn the user if it happens. e.g. `setLibcallName(RTLIB::SHL_I128, nullptr);` in X86ISelLowering.cpp <https://raw.githubusercontent.com/llvm/llvm-project/master/llvm/lib/Target/X86/X86ISelLowering.cpp>

I'm not saying one attribute per helper is not feasible but I'd like to put it into perspective with other constraints.

> We probably need to also carefully audit all of the transform passes to find ones that insert calls.  Last time I looked, there were four places in LLVM where memcpy could be expanded, I wonder if there are a similar number where it can be synthesised...

Yes indeed, it's going to be long and painful, here are the functions calling `CallLoweringInfo::setLibCallee` :

**llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp**

- ExpandLibCall(LC, Node, isSigned)
- ExpandChainLibCall(LC, Node, isSigned)
- ExpandDivRemLibCall(Node, Results)
- ExpandSinCosLibCall(Node, Results)
- ConvertNodeToLibcall(Node)
- ConvertNodeToLibcall(Node)

**llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp**

- ExpandIntRes_XMULO(N, Lo, Hi)

**llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp**

- ExpandChainLibCall(LC, Node, isSigned)

**llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp**

- getMemcpy(Chain, dl, Dst, Src, Size, Align, isVol, AlwaysInline, isTailCall, DstPtrInfo, SrcPtrInfo)
- getAtomicMemcpy(Chain, dl, Dst, DstAlign, Src, SrcAlign, Size, SizeTy, ElemSz, isTailCall, DstPtrInfo, SrcPtrInfo)
- getMemmove(Chain, dl, Dst, Src, Size, Align, isVol, isTailCall, DstPtrInfo, SrcPtrInfo)
- getAtomicMemmove(Chain, dl, Dst, DstAlign, Src, SrcAlign, Size, SizeTy, ElemSz, isTailCall, DstPtrInfo, SrcPtrInfo)
- getMemset(Chain, dl, Dst, Src, Size, Align, isVol, isTailCall, DstPtrInfo)
- getAtomicMemset(Chain, dl, Dst, DstAlign, Value, Size, SizeTy, ElemSz, isTailCall, DstPtrInfo)

**llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp**

- visitIntrinsicCall(I, Intrinsic)

**llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp**

- makeLibCall(DAG, LC, RetVT, Ops, isSigned, dl, doesNotReturn, isReturnValueUsed, isPostTypeLegalization)
- LowerToTLSEmulatedModel(GA, DAG)

**llvm/lib/Target/AArch64/AArch64ISelLowering.cpp**

- LowerFSINCOS(Op, DAG)

**llvm/lib/Target/AArch64/AArch64SelectionDAGInfo.cpp**

- EmitTargetCodeForMemset(DAG, dl, Chain, Dst, Src, Size, Align, isVolatile, DstPtrInfo)

**llvm/lib/Target/ARM/ARMISelLowering.cpp**

- LowerToTLSGeneralDynamicModel(GA, DAG)

**llvm/lib/Target/ARM/ARMSelectionDAGInfo.cpp**

- EmitSpecializedLibcall(DAG, dl, Chain, Dst, Src, Size, Align, LC)

**llvm/lib/Target/Hexagon/HexagonSelectionDAGInfo.cpp**

- EmitTargetCodeForMemcpy(DAG, dl, Chain, Dst, Src, Size, Align, isVolatile, AlwaysInline, DstPtrInfo, SrcPtrInfo)

**llvm/lib/Target/PowerPC/PPCISelLowering.cpp**

- LowerINIT_TRAMPOLINE(Op, DAG)

**llvm/lib/Target/X86/X86ISelLowering.cpp**

- LowerWin64_i128OP(Op, DAG)
- LowerFSINCOS(Op, Subtarget, DAG)

**llvm/lib/Target/X86/X86SelectionDAGInfo.cpp**

- EmitTargetCodeForMemset(DAG, dl, Chain, Dst, Val, Size, Align, isVolatile, DstPtrInfo)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61634





More information about the cfe-commits mailing list