[PATCH] D154119: Fix: Distinguish CFI Metadata Checks in MergeFunctions Pass
Duncan P. N. Exon Smith via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 28 09:54:38 PDT 2023
dexonsmith added a comment.
Sorry, Phabricator lost my comment somehow. Adding back a version of it now.
I don't think it's viable to have the two functions arbitrarily compare "less than" if they both have distinct metadata. That leads to assertions like this:
assert(!(V1<V2 && V2<V1) && "Both values are less than the other?");
firing.
It also makes it undefined behaviour to use the comparator in "sort" algorithms. Have a look at:
https://en.cppreference.com/w/cpp/named_req/LessThanComparable
https://en.wikipedia.org/wiki/Weak_ordering#Strict_weak_orderings
And I assume this is used for sorting somehow?
I suggest changing function merging to check for distinct metadata as a second step, after functions compare equal. But there might be another way.
================
Comment at: llvm/lib/Transforms/Utils/FunctionComparator.cpp:810
+ return 0;
+ return -1;
+ }
----------------
This breaks strict weak ordering, which means compareValues can never be used for sorting, which I assume is the whole point.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154119/new/
https://reviews.llvm.org/D154119
More information about the llvm-commits
mailing list