Page MenuHomePhabricator

[LTO] Add a function `LTOCodeGenerator::getMergedModule`
AcceptedPublic

Authored by tianshilei1992 on Nov 18 2021, 3:43 PM.

Details

Summary

One of the uses of LTOCodeGenerator is to take it as a middle+back end. Sometimes
it is very helpful to access, especially get information from the optimized module.
If the information can be changed in optimization, it cannot be get before the
module is added to LTOCodeGenerator. This patch adds a function
LTOCodeGenerator::getMergedModule to access the MergedModule.

Diff Detail

Event Timeline

tianshilei1992 created this revision.Nov 18 2021, 3:43 PM
tianshilei1992 requested review of this revision.Nov 18 2021, 3:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 18 2021, 3:43 PM

The change is fine. Can you elaborate a bit more about your use case? Or at least add a test for it?

tianshilei1992 edited the summary of this revision. (Show Details)Nov 20 2021, 7:39 AM
steven_wu requested changes to this revision.Nov 29 2021, 9:01 AM

Few things to improve before commit:

  • Drop the words like "in my case", "I", etc. from the commit message. Eg, In my case, after optimize is called, I need to access the underlying module to get information. can be rewritten as API can be used to access the underlying module to get information after optimized is called. It would sound much better.
  • Please add a use-case of the API in test suite otherwise it can be removed as dead code. You can look into getting it used in llvm-lto tool and add a testcase to exercise the code path.
This revision now requires changes to proceed.Nov 29 2021, 9:01 AM
tianshilei1992 edited the summary of this revision. (Show Details)Dec 10 2021, 2:16 PM

add an option in llvm-lto to dump linked module

add a test case

steven_wu accepted this revision.Dec 13 2021, 9:15 AM

Thanks! LGTM now.

This revision is now accepted and ready to land.Dec 13 2021, 9:15 AM
This revision was landed with ongoing or failed builds.Dec 13 2021, 1:19 PM
This revision was automatically updated to reflect the committed changes.

Thanks! LGTM now.

Thanks for your review!

tianshilei1992 reopened this revision.Dec 13 2021, 1:32 PM

Reopen to resolve building issues reported by build bolt.

This revision is now accepted and ready to land.Dec 13 2021, 1:32 PM

I cannot reproduce the failure on my side. Will re-land the patch.

tianshilei1992 reopened this revision.Dec 20 2021, 5:34 PM
This revision is now accepted and ready to land.Dec 20 2021, 5:34 PM

It seems like the root cause of link error reported in many BuildBolt is Module::dump is stripped in release mode. Probably should not use that function at the first place. Need to come up with another way to test.

@steven_wu Do you have any suggestion on how to test it in llvm-lto? I can't find a feasible use of the function in llvm-lto.