[llvm] MachineBlockPlacement: Add tolerance to comparisons (PR #67197)
Rahman Lavaee via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 2 12:52:40 PDT 2023
rlavaee wrote:
> > IIUC, these functions do not satisfy weak ordering.
>
> ...
>
> > In that case, is it a good idea to expose these as pseudo-comparators?
>
> Yes it seems this is only a strict partial order but not a weak ordering. But is this actually a problem, would people infer this from the function names and get confused if it isn't? Are you saying we should find a weak ordering here? I don't immediately see any benefits from that at least for my use case.
>
> Or are you proposing that we should find a different name for these functions to set other expectations? But then what name should we chose, I personally didn't have expectations one way or another...
Yes. I suggest we use different naming and also make these non-member functions.
The current naming does not resonate with the definitions of these functions. I think these functions define whether one frequency is significantly greater or less than the other.
My suggestion is to define a class for this so we can explicitly mention that it does not satisfy transitivity and therefore, it does not induce a strict total ordering. This also allows us to move the implementation detail about the significant bits (which btw I find more related to non-significant number of bits) into this class.
```
class SignificantRelativeComparator {
int NonSignificantBits = 20;
public:
bool Less (const BlockFrequency &LHS, const BlockFrequency &RHS) {
// implementation
}
bool Greater (const BlockFrequency &LHS, const BlockFrequency &RHS) {
// implementation
}
};
```
https://github.com/llvm/llvm-project/pull/67197
More information about the llvm-commits
mailing list