[Openmp-commits] [PATCH] D90172: [OpenMP] Add Support for Mapping Names in Libomptarget RTL

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added a comment.

Let's split this:

1. introduce ident.h, or source_info.h to be even more descriptive. Only declare `source_loc_info_t` (or similar) and use it instead of the `void*`.
2. introduce `mapped_var_info_t` (or similar) and `getMappedVariableName(mapped_var_info_t *)`. Use it everywhere when you extend the interface.



================
Comment at: openmp/libomptarget/include/Ident.h:56
+                    std::stoi(getSubstring(2)), std::stoi(getSubstring(3))};
+}
+
----------------
This function is unused. Let's add it with a use. Also, we could make it a constructor while we are at it.


================
Comment at: openmp/libomptarget/include/Ident.h:67
+  return name_str.substr(begin + 1, end - begin - 1);
+}
+
----------------
jhuber6 wrote:
> jdoerfert wrote:
> > Nit: `getVariableNameFromNameT`? or similar, getName is too generic
> Do you think I should change all references from `void **` to use some `name_t` in the runtime? I guess it makes it more obvious what it's supposed to be.
Yeah, that sounds good. While at it, I would make `name_t` a `const char *`, though if we hide it anyway it's not that important.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90172