Skip to content

Commit

Permalink
[BranchFolding] Remove dubious assert from operator< (#71639)
Browse files Browse the repository at this point in the history
`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 llvm/llvm-project#71312.
  • Loading branch information
nikic authored and zahiraam committed Nov 20, 2023
1 parent d98fba7 commit 9ef3a37
Showing 1 changed file with 0 additions and 6 deletions.
6 changes: 0 additions & 6 deletions llvm/lib/CodeGen/BranchFolding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9ef3a37

Please sign in to comment.