[PATCH] D39005: [OpenMP] Clean up variable and function names for NVPTX backend
Hahnfeld removed a reviewer: Hahnfeld. Hahnfeld added a comment. Fixed differently in https://reviews.llvm.org/rL319657. Repository: rL LLVM https://reviews.llvm.org/D39005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39005: [OpenMP] Clean up variable and function names for NVPTX backend
hfinkel added a comment. In https://reviews.llvm.org/D39005#900973, @jlebar wrote: > > The first question that comes to mind is what is the link between data > > layout and name mangling conventions? > > I pulled up http://llvm.org/doxygen/classllvm_1_1DataLayout.html and searched > for "mangling" -- presumably this is what they were referring to. We also > don't need to speculate, rnk still works on LLVM. :) DataLayout generally holds information that the target-independent optimizer needs in order to simplify the IR into our canonical form. This is as opposed to TargetTransformInfo, which provides data necessary to optimize the IR in target-aware ways (e.g., do things that are orthogonal to canonicalization such as inlining and vectorization). It is also as opposed to external utility functions that might be used by the frontend (e.g., llvm::sys::getHostCPUName()). If I recall correctly, this is information that would be used by the frontend when generating the IR, and the function results are controlled by the triple. As a result, I think that a general utility function somewhere would be fine. Repository: rL LLVM https://reviews.llvm.org/D39005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39005: [OpenMP] Clean up variable and function names for NVPTX backend
jlebar added a comment. > The first question that comes to mind is what is the link between data layout > and name mangling conventions? I pulled up http://llvm.org/doxygen/classllvm_1_1DataLayout.html and searched for "mangling" -- presumably this is what they were referring to. We also don't need to speculate, rnk still works on LLVM. :) Repository: rL LLVM https://reviews.llvm.org/D39005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39005: [OpenMP] Clean up variable and function names for NVPTX backend
gtbercea added a comment. In https://reviews.llvm.org/D39005#900226, @jlebar wrote: > > I'd be interested to get the ball rolling in regard to coming up with a fix > > for this. I see some suggestions in past patches. Some help/clarification > > would be much appreciated. > > Happy to help, but I'm not sure what to offer beyond the link in Art's > previous comment. Thanks Justin. Perhaps if we could start by clarifying this statement "One option is that we add a function to LLVM get an available separator character, which can default to '.', but we set to '$' for nvptx, and use that for generating new names at the IR level." as well as this statement "This seems practical. Perhaps it could be part of the name mangling scheme already encoded in DataLayout?". The first question that comes to mind is what is the link between data layout and name mangling conventions? Repository: rL LLVM https://reviews.llvm.org/D39005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39005: [OpenMP] Clean up variable and function names for NVPTX backend
gtbercea added a comment. Hi Artem, Justin, I see that this patch is the same as the patch Arpith wanted to post a while back i.e. https://reviews.llvm.org/D17738. Was there a consensus regarding what the right thing to do is in this case? I'd be interested to get the ball rolling in regard to coming up with a fix for this. I see some suggestions in past patches. Some help/clarification would be much appreciated. Thanks! Repository: rL LLVM https://reviews.llvm.org/D39005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39005: [OpenMP] Clean up variable and function names for NVPTX backend
tra requested changes to this revision. tra added a comment. Justin is right. I completely forgot about this. :-/ Hal offered possible solution: https://reviews.llvm.org/D17738#661115 Repository: rL LLVM https://reviews.llvm.org/D39005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39005: [OpenMP] Clean up variable and function names for NVPTX backend
jlebar added a comment. This has been tried twice before, see https://reviews.llvm.org/D29883 and https://reviews.llvm.org/D17738. I'm as unhappy about this as anyone, and personally I don't have any preference about how we try to solve it. But I think we shouldn't check this in without hearing the objections from those past attempts. Repository: rL LLVM https://reviews.llvm.org/D39005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39005: [OpenMP] Clean up variable and function names for NVPTX backend
gtbercea created this revision.
Herald added a subscriber: jholewinski.
Clean-up variable and function names.
Repository:
rL LLVM
https://reviews.llvm.org/D39005
Files:
lib/Target/NVPTX/NVPTXAssignValidGlobalNames.cpp
Index: lib/Target/NVPTX/NVPTXAssignValidGlobalNames.cpp
===
--- lib/Target/NVPTX/NVPTXAssignValidGlobalNames.cpp
+++ lib/Target/NVPTX/NVPTXAssignValidGlobalNames.cpp
@@ -37,6 +37,8 @@
/// \brief Clean up the name to remove symbols invalid in PTX.
std::string cleanUpName(StringRef Name);
+ /// Set a clean name, ensuring collisions are avoided.
+ void generateCleanName(Value );
};
}
@@ -50,20 +52,31 @@
"Assign valid PTX names to globals", false, false)
bool NVPTXAssignValidGlobalNames::runOnModule(Module ) {
- for (GlobalVariable : M.globals()) {
-// We are only allowed to rename local symbols.
-if (GV.hasLocalLinkage()) {
- // setName doesn't do extra work if the name does not change.
- // Note: this does not create collisions - if setName is asked to set the
- // name to something that already exists, it adds a proper postfix to
- // avoid collisions.
- GV.setName(cleanUpName(GV.getName()));
-}
- }
+ // We are only allowed to rename local symbols.
+ for (GlobalVariable : M.globals())
+if (GV.hasLocalLinkage())
+ generateCleanName(GV);
+
+ // Clean function symbols.
+ for (auto : M.functions())
+if (FN.hasLocalLinkage())
+ generateCleanName(FN);
return true;
}
+void NVPTXAssignValidGlobalNames::generateCleanName(Value ) {
+ std::string ValidName;
+ do {
+ValidName = cleanUpName(V.getName());
+// setName doesn't do extra work if the name does not change.
+// Collisions are avoided by adding a suffix (which may yet be unclean in
+// PTX).
+V.setName(ValidName);
+// If there are no collisions return, otherwise clean up the new name.
+ } while (!V.getName().equals(ValidName));
+}
+
std::string NVPTXAssignValidGlobalNames::cleanUpName(StringRef Name) {
std::string ValidName;
raw_string_ostream ValidNameStream(ValidName);
Index: lib/Target/NVPTX/NVPTXAssignValidGlobalNames.cpp
===
--- lib/Target/NVPTX/NVPTXAssignValidGlobalNames.cpp
+++ lib/Target/NVPTX/NVPTXAssignValidGlobalNames.cpp
@@ -37,6 +37,8 @@
/// \brief Clean up the name to remove symbols invalid in PTX.
std::string cleanUpName(StringRef Name);
+ /// Set a clean name, ensuring collisions are avoided.
+ void generateCleanName(Value );
};
}
@@ -50,20 +52,31 @@
"Assign valid PTX names to globals", false, false)
bool NVPTXAssignValidGlobalNames::runOnModule(Module ) {
- for (GlobalVariable : M.globals()) {
-// We are only allowed to rename local symbols.
-if (GV.hasLocalLinkage()) {
- // setName doesn't do extra work if the name does not change.
- // Note: this does not create collisions - if setName is asked to set the
- // name to something that already exists, it adds a proper postfix to
- // avoid collisions.
- GV.setName(cleanUpName(GV.getName()));
-}
- }
+ // We are only allowed to rename local symbols.
+ for (GlobalVariable : M.globals())
+if (GV.hasLocalLinkage())
+ generateCleanName(GV);
+
+ // Clean function symbols.
+ for (auto : M.functions())
+if (FN.hasLocalLinkage())
+ generateCleanName(FN);
return true;
}
+void NVPTXAssignValidGlobalNames::generateCleanName(Value ) {
+ std::string ValidName;
+ do {
+ValidName = cleanUpName(V.getName());
+// setName doesn't do extra work if the name does not change.
+// Collisions are avoided by adding a suffix (which may yet be unclean in
+// PTX).
+V.setName(ValidName);
+// If there are no collisions return, otherwise clean up the new name.
+ } while (!V.getName().equals(ValidName));
+}
+
std::string NVPTXAssignValidGlobalNames::cleanUpName(StringRef Name) {
std::string ValidName;
raw_string_ostream ValidNameStream(ValidName);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
