GSoC 2020 Project "Improve MegreFunctions to incorporate MergeSimilarFunctions patches and ThinLTO Support"

Hi Vishal, Ruijie,

Thanks for your interest in the project.
To get started, the first task would be to merge the 5 patches on top of trunk llvm.
The list of patches are listed in the project description: http://llvm.org/OpenProjects.html#llvm_mergesim
Please create an account in llvm phabricator (reviews.llvm.org) if you haven't already, and put your patches there.

Let me know if you have further questions; both llvm-dev and discord are good places to reach out. My userid is: hiraditya

Welcome to GSoC!

Best,
-Aditya

Hi Aditya,

I'm starting to work on merging the patches --- One question, though,
from someone who's still novice to the LLVM code review system:
Should I merge your patches one by one into trunk, and then do a
single "arc diff", or should I do multiple "arc patch" to first merge
the five patches together (building on top of successive branches),
and then all together into trunk, and then do an `arc diff`?

Thanks,
Ruijie

Hey Aditya,

I’m planning to apply the patches one after the other to the LLVM trunk and then do “arc diff” to merge all the 5 patches together. However, being a newbie to LLVM code-review system, I’m stuck at applying the 2nd patch. I applied the 1st patch to the LLVM trunk using the “arc patch Dxxxx” command and it worked fine. When I try to run the same command with the ID of the 2nd patch, I run into the error as given here: https://secure.phabricator.com/T12501.
I don’t know whether that issue was fixed or there’s some way around. I also tried deleting the branch created by patching the 1st patch and tried to install the 2nd patch (D52898) from the master branch. I expected the 2nd patch to also commit the 1st patch as there’s a dependency between them. Even that throws the following error after applying the 1st patch (D52896):

Cherry Pick Failed!
Exception
Command failed with error #1!
COMMAND
git cherry-pick – ‘arcpatch-D52896’

STDOUT
(empty)

STDERR
warning: inexact rename detection was skipped due to too many files.
warning: you may want to set your merge.renamelimit variable to at least 8313 and retry the command.
error: could not apply ca94adea8e6… MergeSimilarFunctions 1/n: a code size pass to merge functions with small differences
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add ’ or 'git rm ’
hint: and commit the result with ‘git commit’

(Run with --trace for a full exception trace.)

I tried running with the --trace option. That doesn’t help much either.
I see that the cherry-pick failed cause there were unstaged files after committing the first patch.

Regards,
Vishal

Please apply one by one. There might be merge conflicts you’d have to resolve :slight_smile: you can ping me on discord in case you get stuck.

Aditya

Please apply one patch at a time and then push one by one as well. When there is a merge conflict try to resolve by yourself or ping me if it is too complicated.

Aditya

Hi all, I see there are a lot of new patches being produced to merge various combinations of the original patches together and rebase with head. I just added myself a reviewer to the ones I saw. A couple high level comments: These various mergings of the patches together don’t need to all be separate patches in Phabricator. Really only whatever you want reviewed by the community needs to go up on Phabricator, which is presumably the final merged and rebase patch. Also, please keep all the original reviewers on the new patch(es). There are a number of comments on the original patches that still need to be resolved fyi, so please take a look at those before proceeding too far. +JF Bastien who also commented to that effect. It might be good to coordinate your efforts as well.

Thanks!
Teresa

Hi Teresa,

Thanks for the comments. I was just trying to merge the 5 patches together. Sorry, for pushing various intermediate merging patches to Phabricator. I didn’t know that even the original patches may need changes. We will discuss with our GSoC project mentor and proceed.

Regards,
Vishal

Hi Aditya,

The 5 patches have been merged together and the new merged patch is available at https://reviews.llvm.org/D76522

Regards,
Vishal

hi Teresa,
This was the starter assignment for the students to merge the patches.
We'll address the comments from the original patches once the intern has been finalized.
JF Bastien has agreed to co-mentor the project, we'll try to address the core issues to make merge-function as robust as possible.

Thanks,
-Aditya

Thanks Aditya and Vishal for the context on the patches. I’m glad to see this work moving forward.
Teresa