[llvm-branch-commits] [llvm] 98bfdac - [BranchFolding] Remove dubious assert from operator< (#71639)

Tobias Hieta via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Tue Nov 14 00:28:40 PST 2023


Author: Nikita Popov
Date: 2023-11-14T09:22:39+01:00
New Revision: 98bfdac5ce82d1679f8af9a57501471812ab68d7

URL: https://github.com/llvm/llvm-project/commit/98bfdac5ce82d1679f8af9a57501471812ab68d7
DIFF: https://github.com/llvm/llvm-project/commit/98bfdac5ce82d1679f8af9a57501471812ab68d7.diff

LOG: [BranchFolding] Remove dubious assert from operator< (#71639)

`MergePotentialElts::operator<` asserts that the two elements being
compared are not equal. However, sorting functions are allowed to invoke
the comparison function with equal arguments (though they usually don't
for efficiency reasons).

There is an existing special-case that disables the assert if
_GLIBCXX_DEBUG is used, which may invoke the comparator with equal args
to verify strict weak ordering. I believe libc++ also has strict weak
ordering checks under some options nowadays.

Recently, #71312 was reported, where a change to glibc's qsort_r
implementation can also result in comparison between equal elements.
>From what I understood, this is an inefficiency that will be fixed on
the glibc side as well, but I think at this point we should just remove
this assertion.

Fixes https://github.com/llvm/llvm-project/issues/71312.

(cherry picked from commit 74a76a288562c486f377121855ef7db0386e0e43)

Added: 
    

Modified: 
    llvm/lib/CodeGen/BranchFolding.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index 3830f25debaf3cb..0801296cab49f8f 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -485,13 +485,7 @@ BranchFolder::MergePotentialsElt::operator<(const MergePotentialsElt &o) const {
     return true;
   if (getBlock()->getNumber() > o.getBlock()->getNumber())
     return false;
-  // _GLIBCXX_DEBUG checks strict weak ordering, which involves comparing
-  // an object with itself.
-#ifndef _GLIBCXX_DEBUG
-  llvm_unreachable("Predecessor appears twice");
-#else
   return false;
-#endif
 }
 
 /// CountTerminators - Count the number of terminators in the given


        


More information about the llvm-branch-commits mailing list