Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

MergeIdenticalFuncs is unable to merge identical lambdas #34756

Open Quuxplusone opened 6 years ago

Quuxplusone commented 6 years ago
Bugzilla Link PR35783
Status NEW
Importance P enhancement
Reported by Jean-Michaël Celerier (jeanmichael.celerier@gmail.com)
Reported on 2017-12-31 06:20:13 -0800
Last modified on 2019-06-24 01:39:35 -0700
Version trunk
Hardware PC Linux
CC dblaikie@gmail.com, dgregor@apple.com, ditaliano@apple.com, jonatan1626@gmail.com, llvm-bugs@lists.llvm.org, rares.aioanei@gmail.com, rnk@google.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
Case 1: https://godbolt.org/g/JvmxJr
Case 2: https://godbolt.org/g/FWo12z

Ideally, the compiler should be able to detect that the code is the same and
merge it.

I opened a similar bug report in GCC:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83582
Quuxplusone commented 6 years ago
I think the right place where this should be performed is MergeIdenticalFunc.
This is not a clang bug, but a missing optimization in the middle-end.
Quuxplusone commented 6 years ago

LLVM has a function merging pass, but it does not run as part of -O2 or -O3. It does work for my small test cases and probably yours as well.

Quuxplusone commented 6 years ago
(In reply to Reid Kleckner from comment #2)
> LLVM has a function merging pass, but it does not run as part of -O2 or -O3.
> It does work for my small test cases and probably yours as well.

I tried this a couple of days ago & my memory is a little fuzzy, but I think
this particular case wasn't caught, that's why I left this open. I may be
wrong, so I'll take a closer look once I get a chance.
Quuxplusone commented 6 years ago

Here's a simple Clang test case:

https://godbolt.org/g/6PsYYB

I disabled inlining though, because I wasn't able to write a lambda that would not inline :p

Here's another cases:

https://godbolt.org/g/pkxkjA

https://godbolt.org/g/ZBfzNb (though I suppose that for this one, the lambda is ODR-used at some point in std::function's implementation which would make such an optimization invalid by the standard) ?

Quuxplusone commented 6 years ago
It looks like MergeFunctions is not able to handle most of these.
I think it would be a good beginner project if somebody wants to take a look,
so I'm marking this with the beginner keyword.
Quuxplusone commented 6 years ago

Hi, I’m new and I would like to take a look at this to get more familiar with the code base! Is anybody currently working on this or can I work on this?