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

[Artem Belevich via Phabricator via llvm-commits]

tra added a comment.

The tests you have only check the end result and do not directly verify the new functionality the patch adds.

Ideally, it would be great if we could verify that your changes that add and remove RegProxy do the right thing.
Could you check if it would make sense to add couple of MIR tests: https://llvm.org/docs/MIRLangRef.html#testing-individual-code-generation-passes

I can think of three interesting tests:

- verify that RegProxy is inserted in the right places.
- verify that RegProxy gets removed by NVPTXProxyRegErasure
- verify that we correctly lower libcalls with and without NVPTXProxyRegErasure

Other than that the patch looks good to me.



================
Comment at: lib/Target/NVPTX/NVPTXInstrInfo.td:2258
+  NVPTXInst<(outs regclass:$dst), (ins regclass:$src),
+            !strconcat("mov.", !strconcat(SzStr, " \t$dst, $src;")),
+            [(set regclass:$dst, (ProxyReg regclass:$src))]>;
----------------
Couple of nits:
* `!strconcat` accepts multiple arguments, so you only need one `!strconcat` to do the job.
* There's also a `#` which is a very convenient shortcut for concatenating strings.
  `"a" # "b"` is the same as `!strconcat("a", "b")`

Either of those could be used here.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D34708