[PATCH] D34708: [NVPTX] Allow to make libcalls that are defined in the current module.

[Denys Zariaiev via Phabricator via llvm-commits]

denzp added a comment.

In https://reviews.llvm.org/D34708#792791, @arsenm wrote:

> This is something I've thought about doing before. Does this work with the TLI lib call simplification as is, and this is the only required codegen part?


As far as I understood, we can make libcall either with:

- TargetLowering::makeLibCall <http://llvm.org/doxygen/TargetLowering_8cpp_source.html#l00119>
- SelectionDAGLegalize::ExpandLibCall <http://llvm.org/doxygen/LegalizeDAG_8cpp_source.html#l01953>

And I both should work. I'm going to add one more test case for `TargetLowering::makeLibCall` path.



================
Comment at: lib/Target/NVPTX/NVPTXISelLowering.cpp:1841-1848
+  if (!CS) {
+    // Unfortunately, libcall expansion does not respect `Chain`...
+    // Our `CALLSEQ_END` becomes dead node (i.e. nobody references it) because
+    // only retval `LoadParam`s will be used after expansion. That means the
+    // node will be deleted at the next iteration of legalization and we have
+    // to make effors to keep it alive.
+    DAG.KeepNodeAlive(Chain.getNode(), true);
----------------
arsenm wrote:
> This is probably the wrong way to do this. You probably want to do a getMergeValues with the returned value and the entry chain
Thanks, it's a good idea and a right way. But... we are still losing reference during legalization:

```
Legalizing: t28: f64,ch = merge_values t25, t27
t28: f64,ch = merge_values t25, t27 ... replacing: t28: f64,ch = merge_values t25, t27
     with:      t25: f64,ch,glue = NVPTXISD::LoadParam<LD8[<unknown>]> t24, Constant:i32<1>, Constant:i32<0>, t24:1
      and:      t27: ch,glue = callseq_end t25:1, TargetConstant:i32<0>, TargetConstant:i32<1>, t25:2
```

Unfortunately, nobody is referencing `t27` and it's being removed as a dead node.


================
Comment at: lib/Target/NVPTX/NVPTXISelLowering.cpp:2313-2315
+SDValue
+NVPTXTargetLowering::LowerLibcallFnNameSymbol(SDValue Op,
+                                              SelectionDAG &DAG) const {
----------------
arsenm wrote:
> Can you move this to generic DAG code?
It has quite specific for NVPTX logic, usually other backends don't have restrictions on presence of function definition in current module. Are there another use cases for this?


https://reviews.llvm.org/D34708