[llvm] r274668 - Add 'thinlto_src_module' metadata to imported function

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 6 18:02:30 PDT 2016


Another way is to have a cl::opt, possibly defaulted to enable in debug build.
The test would enable it unconditionally and would pass both in assert and release builds.

— 
Mehdi

> On Jul 6, 2016, at 4:25 PM, Teresa Johnson <tejohnson at google.com> wrote:
> 
> Add:
> ; REQUIRES: asserts
> 
> to the test. Which probably means that this would be better off tested via a new test.
> 
> Teresa
> 
> On Wed, Jul 6, 2016 at 4:15 PM, Piotr Padlewski <prazek at google.com <mailto:prazek at google.com>> wrote:
> How should I test that? So after adding this defines, tests will fail on non debug build or non statistics
> 
> On Wed, Jul 6, 2016 at 3:52 PM, Piotr Padlewski <prazek at google.com <mailto:prazek at google.com>> wrote:
> Good point, I will fix it in a minute
> 
> 
> On Wed, Jul 6, 2016 at 2:01 PM, Teresa Johnson via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> 
> 
> On Wed, Jul 6, 2016 at 1:45 PM, Mehdi Amini via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> 
> > On Jul 6, 2016, at 1:26 PM, Piotr Padlewski via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> >
> > Author: prazek
> > Date: Wed Jul  6 15:26:25 2016
> > New Revision: 274668
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=274668&view=rev <http://llvm.org/viewvc/llvm-project?rev=274668&view=rev>
> > Log:
> > Add 'thinlto_src_module' metadata to imported function
> >
> > Added metadata to be able to make statistics on how many functions
> > that have been imported have been removed. Also module name might
> > be helpfull when debugging.
> 
> 
> It is not clear to me why this is enabled in production build, or not covered under a debugging flag.
> 
> Good point. Since this is initially designed to drive some statistics, perhaps enabling it for non-release builds similar to when --stats is enabled (see ADT/Statistic.h: #if !defined(NDEBUG) || defined(LLVM_ENABLE_STATS)).
> 
> Teresa
> 
>> 
> Mehdi
> 
> 
> 
> 
> 
> >
> > Reviewers: tejohnson, eraman
> >
> > Subscribers: mehdi_amini, llvm-commits
> >
> > Differential Revision: http://reviews.llvm.org/D21943 <http://reviews.llvm.org/D21943>
> >
> > Modified:
> >    llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
> >    llvm/trunk/test/Transforms/FunctionImport/funcimport.ll
> >
> > Modified: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp?rev=274668&r1=274667&r2=274668&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp?rev=274668&r1=274667&r2=274668&view=diff>
> > ==============================================================================
> > --- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp (original)
> > +++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp Wed Jul  6 15:26:25 2016
> > @@ -591,6 +591,12 @@ bool FunctionImporter::importFunctions(
> >                    << SrcModule->getSourceFileName() << "\n");
> >       if (Import) {
> >         F.materialize();
> > +        // Add 'thinlto_src_module' metadata for statistics and debugging.
> > +        F.setMetadata("thinlto_src_module",
> > +                      llvm::MDNode::get(DestModule.getContext(),
> > +                                        {llvm::MDString::get(
> > +                                            DestModule.getContext(),
> > +                                            SrcModule->getSourceFileName())}));
> >         GlobalsToImport.insert(&F);
> >       }
> >     }
> >
> > Modified: llvm/trunk/test/Transforms/FunctionImport/funcimport.ll
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionImport/funcimport.ll?rev=274668&r1=274667&r2=274668&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionImport/funcimport.ll?rev=274668&r1=274667&r2=274668&view=diff>
> > ==============================================================================
> > --- llvm/trunk/test/Transforms/FunctionImport/funcimport.ll (original)
> > +++ llvm/trunk/test/Transforms/FunctionImport/funcimport.ll Wed Jul  6 15:26:25 2016
> > @@ -14,7 +14,7 @@
> >
> > ; Test import with smaller instruction limit and without the -disable-force-link-odr
> > ; RUN: opt -function-import -summary-file %t3.thinlto.bc %t.bc -import-instr-limit=5 -S | FileCheck %s --check-prefix=INSTLIM5ODR
> > -; INSTLIM5ODR: define linkonce_odr void @linkonceodr()
> > +; INSTLIM5ODR: define linkonce_odr void @linkonceodr() {
> >
> >
> > define i32 @main() #0 {
> > @@ -47,7 +47,7 @@ declare void @analias(...) #1
> > declare void @linkoncealias(...) #1
> >
> > ; INSTLIMDEF-DAG: Import referencestatics
> > -; INSTLIMDEF-DAG: define available_externally i32 @referencestatics(i32 %i)
> > +; INSTLIMDEF-DAG: define available_externally i32 @referencestatics(i32 %i) !thinlto_src_module !0 {
> > ; INSTLIM5-DAG: declare i32 @referencestatics(...)
> > declare i32 @referencestatics(...) #1
> >
> > @@ -56,27 +56,27 @@ declare i32 @referencestatics(...) #1
> > ; Ensure that the call is to the properly-renamed function.
> > ; INSTLIMDEF-DAG: Import staticfunc
> > ; INSTLIMDEF-DAG: %call = call i32 @staticfunc.llvm.
> > -; INSTLIMDEF-DAG: define available_externally hidden i32 @staticfunc.llvm.
> > +; INSTLIMDEF-DAG: define available_externally hidden i32 @staticfunc.llvm.{{.*}} !thinlto_src_module !0 {
> >
> > ; INSTLIMDEF-DAG: Import referenceglobals
> > -; CHECK-DAG: define available_externally i32 @referenceglobals(i32 %i)
> > +; CHECK-DAG: define available_externally i32 @referenceglobals(i32 %i) !thinlto_src_module !0 {
> > declare i32 @referenceglobals(...) #1
> >
> > ; The import of referenceglobals will expose call to globalfunc1 that
> > ; should in turn be imported.
> > ; INSTLIMDEF-DAG: Import globalfunc1
> > -; CHECK-DAG: define available_externally void @globalfunc1()
> > +; CHECK-DAG: define available_externally void @globalfunc1() !thinlto_src_module !0
> >
> > ; INSTLIMDEF-DAG: Import referencecommon
> > -; CHECK-DAG: define available_externally i32 @referencecommon(i32 %i)
> > +; CHECK-DAG: define available_externally i32 @referencecommon(i32 %i) !thinlto_src_module !0 {
> > declare i32 @referencecommon(...) #1
> >
> > ; INSTLIMDEF-DAG: Import setfuncptr
> > -; CHECK-DAG: define available_externally void @setfuncptr()
> > +; CHECK-DAG: define available_externally void @setfuncptr() !thinlto_src_module !0 {
> > declare void @setfuncptr(...) #1
> >
> > ; INSTLIMDEF-DAG: Import callfuncptr
> > -; CHECK-DAG: define available_externally void @callfuncptr()
> > +; CHECK-DAG: define available_externally void @callfuncptr() !thinlto_src_module !0 {
> > declare void @callfuncptr(...) #1
> >
> > ; Ensure that all uses of local variable @P which has used in setfuncptr
> > @@ -87,7 +87,7 @@ declare void @callfuncptr(...) #1
> >
> > ; Ensure that @referencelargelinkonce definition is pulled in, but later we
> > ; also check that the linkonceodr function is not.
> > -; CHECK-DAG: define available_externally void @referencelargelinkonce()
> > +; CHECK-DAG: define available_externally void @referencelargelinkonce() !thinlto_src_module !0 {
> > ; INSTLIM5-DAG: declare void @linkonceodr()
> > declare void @referencelargelinkonce(...)
> >
> > @@ -100,11 +100,12 @@ declare void @weakfunc(...) #1
> > declare void @linkoncefunc2(...) #1
> >
> > ; INSTLIMDEF-DAG: Import funcwithpersonality
> > -; INSTLIMDEF-DAG: define available_externally hidden void @funcwithpersonality.llvm.{{.*}}() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
> > +; INSTLIMDEF-DAG: define available_externally hidden void @funcwithpersonality.llvm.{{.*}}() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) !thinlto_src_module !0 {
> > ; INSTLIM5-DAG: declare hidden void @funcwithpersonality.llvm.{{.*}}()
> >
> > ; INSTLIMDEF-DAG: Import globalfunc2
> > ; INSTLIMDEF-DAG: 13 function-import - Number of functions imported
> > +; CHECK-DAG: !0 = !{!"{{.*}}/Inputs/funcimport.ll"}
> >
> > ; The actual GUID values will depend on path to test.
> > ; GUID-DAG: GUID {{.*}} is weakalias
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
> 
> 
> 
> -- 
> Teresa Johnson |	 Software Engineer |	 tejohnson at google.com <mailto:tejohnson at google.com> |	 408-460-2413 <tel:408-460-2413>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
> 
> 
> 
> 
> 
> 
> -- 
> Teresa Johnson |	 Software Engineer |	 tejohnson at google.com <mailto:tejohnson at google.com> |	 408-460-2413

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160706/6ff7c029/attachment.html>


More information about the llvm-commits mailing list