(Closes #2846) extend KernelModuleInlineTrans so that it re-names inlined routines

[arporter]

No description provided.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.95%. Comparing base (91d493f) to head (19be2f7).

❌ Your project check has failed because you have indirect coverage changes. Learn more about Unexpected Coverage Changes and reasons for indirect coverage changes.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3290      +/-   ##
==========================================
- Coverage   99.95%   99.95%   -0.01%     
==========================================
  Files         378      378              
  Lines       53764    53692      -72     
==========================================
- Hits        53742    53669      -73     
- Misses         22       23       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[arporter]

Somehow, #3294 got merged and that was a precursor to this (makes module-inline trans mandatory for kernel transformations). I think this must be because I created a branch from that branch to do #1734 and, since that new branch included all the commits for #3294 (some of which I had manually undone subsequently), merging #1734 automatically merged #3294?? Anyway, the upshot is that this is still blocked while I sort out the mess and get #3334 done.

[arporter]

This is now proceeding OK. However, I've realised that, as things currently stand, we'll end up generating multiple OpenCL implementations of the same kernel (since the call to each is module-inlined separately and given a new name). This isn't ideal and the code duplication that could result from having multiple copies of the same kernel will substantially bloat the executable. We could have an option to the transformation to use a pre-existing version of the routine if it exists? What do @sergisiso @LonelyCat124 and @hiker think?

[arporter]

I realised over lunch that my question over-simplifies things - the very first time we module-inline a routine, it gets re-named so a simple check on the name of any subsequent target routine won't reveal that it was previously module inlined.

[sergisiso]

@arporter I believe the previous version was already doing a subroutine comparison (with the equal operator in the routine node) and decide if it was the same than the already existing or not. Could we do something similar? If the subroutine is equal in all but name* to the inlining candidate (e.g. after updating symbols), use the existing one.

• the 'all by name' is tricky because its not what the equal operator does.

Another option is change the behaviour of ModuleInline so it updates ALL the calls to that subroutine.

[arporter]

Another option is change the behaviour of ModuleInline so it updates ALL the calls to that subroutine.

I like that idea. We can have it as an option so that users can do either.