Page MenuHomePhabricator

[Inliner] Only remove functions with a COMDAT when it's safe to do so
Needs ReviewPublic

Authored by Aaron1011 on Oct 12 2018, 10:10 PM.

Details

Reviewers
pcc
tejohnson
Summary

Currently, Inliner will sometimes delete functions in a COMDAT without checking to see if the COMDAT has any other members.
As described in https://reviews.llvm.org/D53234, this is an invalid treatment of COMDAT members, and can lead to issues when using MD_Associated metadata.

I've modified Inliner to ensure that deletion of inlined functions in a COMDAT is always delayed until 'removeDeadFunctions', where the proper checks are performed.
Additionally, I've modified 'removeDeadFunctions' to always process functions with a COMDAT via 'filterDeadComdatFunctions', regardless of their linkage.

I've added a test case to check that the presence of other members in a COMDAT prevents Inliner from deleting a function.

While adding some extra LLVM_DEBUG prints, I found and fixed a bug with printing unnamed COMDATs. It's a one-line fix, so I didn't think it was worth moving into its own patch.

Diff Detail

Event Timeline

Aaron1011 created this revision.Oct 12 2018, 10:10 PM

Hi Aron, I'm interested in reviewing this. Would you mind responding to my comments on https://reviews.llvm.org/D53234 first, as you have based the correctness of this on the correctness of that change?