[PATCH] D159174: [Clang] Use stable_sort in AppendTargetMangling

[Pavel Iliin via Phabricator via cfe-commits]

ilinpv added a comment.

In D159174#4635886 , @BeMg wrote:

> I'm working on RISC-V FMV support, and we found the large set of extension 
> features is hard to maintain the priority that doesn't collision at all.

On AArch64 for FMV we are using ##target_version## attribute and 
##AppendTargetVersionMangling## with llvm::stable_sort


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159174

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159174: [Clang] Use stable_sort in AppendTargetMangling

[Piyou Chen via Phabricator via cfe-commits]

BeMg added a comment.

Thanks for the review.

I'm working on RISC-V FMV support, and we found the large set of extension 
features is hard to maintain the priority that doesn't collision at all.

Lack the appropriate priority, it will generate the random mangling name. The 
predictable mangling name is convenient for testing.

This goal could also be achieved by giving the serial number for each feature, 
but I think the stable sort approach is simpler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159174

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159174: [Clang] Use stable_sort in AppendTargetMangling

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

Do you have a test-case where they were out of order? Or is that dependent on 
the C++ library?

I think I just moved this code from elsewhere when I changed it, but it sounds 
like a sensible change to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159174

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159174: [clang] Use stable_sort in AppendTargetMangling

[Piyou Chen via Phabricator via cfe-commits]

BeMg created this revision.
Herald added a subscriber: mgrang.
Herald added a project: All.
BeMg requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For the target features in the same priority, make sure it is not a random 
mangling name.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159174

Files:
  clang/lib/CodeGen/CodeGenModule.cpp


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1627,7 +1627,7 @@
   Out << '.';
   const TargetInfo &Target = CGM.getTarget();
   ParsedTargetAttr Info = Target.parseTargetAttr(Attr->getFeaturesStr());
-  llvm::sort(Info.Features, [&Target](StringRef LHS, StringRef RHS) {
+  llvm::stable_sort(Info.Features, [&Target](StringRef LHS, StringRef RHS) {
 // Multiversioning doesn't allow "no-${feature}", so we can
 // only have "+" prefixes here.
 assert(LHS.startswith("+") && RHS.startswith("+") &&


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1627,7 +1627,7 @@
   Out << '.';
   const TargetInfo &Target = CGM.getTarget();
   ParsedTargetAttr Info = Target.parseTargetAttr(Attr->getFeaturesStr());
-  llvm::sort(Info.Features, [&Target](StringRef LHS, StringRef RHS) {
+  llvm::stable_sort(Info.Features, [&Target](StringRef LHS, StringRef RHS) {
 // Multiversioning doesn't allow "no-${feature}", so we can
 // only have "+" prefixes here.
 assert(LHS.startswith("+") && RHS.startswith("+") &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits