Page MenuHomePhabricator

[MegreSimilarFunctions] D52896, D52898 and D52966 merged into LLVM trunk
Needs RevisionPublic

Authored by vish99 on Mar 20 2020, 10:47 AM.

Details

Summary

A patch merging all D52896, D52898 and D52966

Diff Detail

Event Timeline

vish99 created this revision.Mar 20 2020, 10:47 AM
vish99 retitled this revision from MergeSimilarFunctions 1/n: a code size pass to merge functions with small differences to D52896, D52898 and D52966 merged into LLVM trunk.Mar 20 2020, 10:50 AM
vish99 edited the summary of this revision. (Show Details)
vish99 edited reviewers, added: hiraditya; removed: tobiasvk, sebpop, adasgupt, pranavb, tejohnson, SirishP, brzycki.
vish99 removed subscribers: evgeny777, inglorion, cfe-commits and 19 others.
jfb requested changes to this revision.Mar 20 2020, 11:55 AM

I've provided a large amount of feedback on prior iterations of this, and would like to see it addressed.

Most important: I'm still worried that anyone changing IR can introduce miscompiles in function merging because the optimization doesn't know about the new IR properties. This has happened before and will happen again.

I'd also much rather see incremental changes to the existing code than another way to do function merging, with no plan to de-duplicate.

This revision now requires changes to proceed.Mar 20 2020, 11:55 AM
vish99 retitled this revision from D52896, D52898 and D52966 merged into LLVM trunk to [MegreSimilarFunctions] D52896, D52898 and D52966 merged into LLVM trunk.Mar 20 2020, 12:24 PM

I also had comments on some of the original patches. I see some emails about this as a GSoC project, but is there a description of the planned work somewhere?

The title of this new patch should mention MergeSimilarFunctions and list the original patches in the summary. When you say they are merged into LLVM trunk, I assume you mean they are being rebased with trunk?

Also what's the relationship between this one and D76481 by @rjf?

hiraditya added a comment.EditedMar 21 2020, 5:04 PM

@reviewers

This is only for preliminary assignment for the student to get some idea of the project. We'll most likely restart with suggestions from the original patch and the mailing list. Please ignore this patch for now. Sorry for introducing noise.