[llvm] [BranchFolding] Remove dubious assert from operator< (PR #71639)

[Nikita Popov via llvm-commits]

https://github.com/nikic created https://github.com/llvm/llvm-project/pull/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 understoo,d 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.

>From 84417e124b8c459232350021cb7ef0b774d74bfc Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 8 Nov 2023 09:56:17 +0100
Subject: [PATCH] [BranchFolding] Remove dubious assert from operator<

`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.

Most 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.
---
 llvm/lib/CodeGen/BranchFolding.cpp | 6 ------
 1 file changed, 6 deletions(-)

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