[llvm] Port Swift's merge function pass to llvm: merging functions that differ in constants (PR #68235)

[Manman Ren via llvm-commits]

manman-ren wrote:


> I'm not sure the best way to approach this is. A simple solution would be to just add another flag `ignore_constants` that ignores constants when set, which I think should work. If we want to specialize `StructuralHash` at compile time, we could maybe use something like template metaprogramming, but not sure how popular that is within LLVM. The main point here is to just try and reduce code duplication as the current `FunctionHashIgnoringConst` is very similar in many ways to `StructuralHash`.

Yes we should try to reduce code duplication. By looking at FunctionHashIgnoringConst, there are a few issues:
FunctionHashIgnoringConst currently inherits from FunctionComparatorIgnoringConst.
--> we call beginCompare, and cmpConstants from FunctionComparator
--> also use GlobalNumberState from FunctionComparator 
We also have two optional parameters to save extra information:
      std::map<int, Instruction *> *IdxToInst = nullptr,
      std::map<std::pair<int, int>, uint64_t> *IdxToConstHash = nullptr
--> I guess we can keep them as optional in StructuralHash. How many setting should we support in StructuralHash? It currently supports default and DetailedHash. Does it mean we will have default, DetailedHash, and IgnoringConst?

> 
> I'd suggest splitting off the additional hashing changes into separate PR(s) to `StructuralHash`. There's already an RFC and these changes would benefit expensive checks, so I don't think there should be any trouble getting them in. I think there are a couple main differences that need to be incorporated into `StructuralHash`:
> 
> * Additional constant cases (would be nice to have in `StructuralHash` as we only handle a couple of cases right now, but not sure if this is necessary for a function hash that is supposed to ignore constants).
> * More detailed type hashing
> * Special casing to grab more metadata from specific instructions like loads and comparisons.
> * Inline assembly
> * Function signatures
> * The addition of a flag to ignore constants
> 
> Splitting the above into individual patches would probably make it easiest for reviewers and to get the patches in.

Yes, I will split the patch into hashing changes and non-hashing changes.

Thanks!
Manman


https://github.com/llvm/llvm-project/pull/68235