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

Mon Feb 27 18:55:22 PST 2023

tra added a comment.

Thank you for the patch. I'm OK with the fix in principle, but would like to check if we could make parameter name generation common for both function signature and the parameter symbol.

================
Comment at: llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp:2624
+    SmallString<128> NameBuf;
+    getTargetMachine().getObjFileLowering()->getNameWithPrefix(
+        NameBuf, &DAG.getMachineFunction().getFunction(), getTargetMachine());
----------------
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.

================
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
----------------
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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144407