[PATCH] D144407: [NVPTX] Use proper parameter names in anonymous functions.

[Pavel Kopyl via Phabricator via llvm-commits]

pavelkopyl added inline comments.


================
Comment at: llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp:2624
+    SmallString<128> NameBuf;
+    getTargetMachine().getObjFileLowering()->getNameWithPrefix(
+        NameBuf, &DAG.getMachineFunction().getFunction(), getTargetMachine());
----------------
tra wrote:
> While this does seem to produce correct argument name, at least for the test example, I'm not convinced it's the best way to handle this. The issue is that we're still generating parameter symbol name here separately from where it gets generated for the function signature. Off the top of my head I do not recall where it happens and I didn't have time to track it down yet.
> 
> We should attempt to make sure that the name is guaranteed to be generated using the same algorithm. E.g. via a common helper function.
> 
Thank you for the review. I slightly refactored the code that generates param names to have all in one place.


================
Comment at: llvm/test/CodeGen/NVPTX/anonymous-fn-param.ll:4
+
+; CHECK:      .func (.param .b32 func_retval0) __unnamed_1(
+; CHECK-NEXT: .param .b32 __unnamed_1_param_0
----------------
tra wrote:
> The patch description and the test should be a bit more specific describing what the problem is.
> 
> AFAICT, the problem is that there's currently a mismatch between the parameter name we generate in the function signature (`__unnamed_1_param_0`) and the name we use when we refer to the parameter in the body of the function (`_param_0`). https://godbolt.org/z/dMEKe1anG
> 
> 
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144407