[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