llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.72k stars 11.41k forks source link

[GVNSink] Reuse value numbering from GVN #92198

Open hiraditya opened 3 months ago

hiraditya commented 3 months ago

GVNSink has a custom gvn code see: https://github.com/llvm/llvm-project/blob/93b47053c6bce5862ba4a5f7d9f6d5cbaa8cbf41/llvm/lib/Transforms/Scalar/GVNSink.cpp#L504 This is not maintainable, we can reuse the one from GVN similar to how GVNHoist does it https://llvm.org/doxygen/GVNHoist_8cpp_source.html#l00225.

hiraditya commented 3 months ago

This is a follow up from the discussion in https://github.com/llvm/llvm-project/issues/77852#issuecomment-1890090879

mvvsmk commented 1 month ago

Hey @hiraditya , I would like to take this issue up, as far as I understand it GVNSink uses it's own implimentation of lookupOrAdd and the complete value system is defined in GVNSink.cpp itself, on the other hand GVNHoist uses the implimentation in GVN.cpp, which I agree is a much more maintanable approach.

To solve this issue I would have to refactor all occurences of the implimentation of the value system by GVNSink and replace with calls to the GVN.cpp implimetations. Right?

If I understand this issue correctly could you assign it to me and I would get working on it. :)

PS : I am here after interacting with llvm-social bangalore group :)

mvvsmk commented 1 month ago

After further analysis , I have observed the following .

  1. Even after the simple refactor of the numbering system to the one used by GVN.h , 11 tests of GVNSink did not pass.
  2. Upon further inspection I found out it's method of counting takes a completely different approach which cannot be matched by GVN.h as concptually it's very different, as explained in the following lines by the Pass author.

https://github.com/llvm/llvm-project/blob/93b47053c6bce5862ba4a5f7d9f6d5cbaa8cbf41/llvm/lib/Transforms/Scalar/GVNSink.cpp#L15-L32

How would you like me to proceed ?

  1. Should I transfer the Value Numbering class fron Sink to GVN.h under a different name ?
  2. Create a separate header to hold this implimentation of GVN so it can be used some place else too ?
  3. I am open to any ideas you may have.