[PATCH] D18013: Using MPI for Profiling Data Reduction

[Xinliang David Li via llvm-commits]

On Wed, Mar 9, 2016 at 5:14 PM, Hal Finkel <hfinkel at anl.gov> wrote:

>
>
> ------------------------------
>
> *From: *"Xinliang David Li" <davidxl at google.com>
> *To: *"Hal Finkel" <hfinkel at anl.gov>
> *Cc: *"Daniel Waters" <dwatersg at gmail.com>, "Vedant Kumar" <vsk at apple.com>,
> "Eric Christopher" <echristo at gmail.com>, "Duncan P. N. Exon Smith" <
> dexonsmith at apple.com>, "Sean Silva" <chisophugis at gmail.com>,
> "llvm-commits" <llvm-commits at lists.llvm.org>,
> reviews+D18013+public+36299b239a77f675 at reviews.llvm.org
> *Sent: *Wednesday, March 9, 2016 6:59:39 PM
>
> *Subject: *Re: [PATCH] D18013: Using MPI for Profiling Data Reduction
>
>
>
> On Wed, Mar 9, 2016 at 4:47 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>
>>
>>
>> ------------------------------
>>
>> *From: *"Xinliang David Li" <davidxl at google.com>
>> *To: *reviews+D18013+public+36299b239a77f675 at reviews.llvm.org
>> *Cc: *"Daniel Waters" <dwatersg at gmail.com>, "Hal Finkel" <hfinkel at anl.gov>,
>> "Vedant Kumar" <vsk at apple.com>, "Eric Christopher" <echristo at gmail.com>,
>> "Duncan P. N. Exon Smith" <dexonsmith at apple.com>, "Sean Silva" <
>> chisophugis at gmail.com>, "llvm-commits" <llvm-commits at lists.llvm.org>
>> *Sent: *Wednesday, March 9, 2016 6:41:45 PM
>> *Subject: *Re: [PATCH] D18013: Using MPI for Profiling Data Reduction
>>
>>
>>
>>
>> On Wed, Mar 9, 2016 at 4:35 PM, Sean Silva <chisophugis at gmail.com> wrote:
>>
>>> silvas added a subscriber: silvas.
>>>
>>> ================
>>> Comment at: lib/profile/InstrProfilingFile.c:86
>>> @@ -74,2 +85,3 @@
>>>    const char *Filename;
>>> +#ifdef _MSC_VER
>>>    FILE *File;
>>> ----------------
>>> Let's split this change into a separate patch for a focused discussion
>>> on the requirements and possible solutions.
>>>
>>> ================
>>> Comment at: lib/profile/InstrProfilingReduce.c:1
>>> @@ +1,2 @@
>>> +/*===- InstrProfilingReduce.c - Support library for applications using
>>> MPI ===*\
>>> +|*
>>> ----------------
>>> Let's call this InstrProfilingMPI.c consistent with the comment.
>>>
>>> ================
>>> Comment at: lib/profile/InstrProfilingStub.c:14
>>> @@ +13,2 @@
>>> +void MPI_Reduce() { abort(); }
>>> +void PMPI_Finalize() { abort(); }
>>> ----------------
>>> davidxl wrote:
>>> > Is the MPI library defining thse functions  static or a DSO? If the
>>> former, then perhaps declare those stubs weak so that we don't rely on the
>>> link order of the runtime library
>>> Why do we need these stubs?
>>>
>>>
>> I thought it is needed when the binary is not linked with mpi library,
>> but InstrProfilingReduce.o is linked in. Now that you asked, it seems that
>> for that case, InstrProfilingReduce.o should not be linked in at all.
>>
>> I think the culprit is that __llvm_write_profile_data flag is defined in
>> that file. With my suggestion to move it to InstrProfiling.c,  I think
>> there is no need for the stub file. Good catch.
>>
>> It is __llvm_write_profile_data, but that's on purpose. The problem is
>> that we need a hard dependency on InstrProfilingReduce.o in order to force
>> the linker to bring it in. Otherwise, because there is already a weak
>> definition of MPI_Finalize in the MPI library, the linker would not bring
>> in InstrProfilingReduce.o at all and our MPI_Finalize implementation would
>> never be called.
>>
>> You're correct that we then need the stubs to make linking succeed when
>> we're not actually linking in an MPI library (e.g. for every regression
>> test).
>>
>>
> ok. Can those stubs be declared weak and merged into
> InstrProfilingReduce.c ?
>
> I don't think so because the MPI functions in the actual MPI library
> themselves are weak and the weak vs. weak resolving is likely ill-defined.
>
> One option might be that, in our MPI_Finalize implementation, we *only*
> use the "strong" PMPI_* variants. Then having weak stubs might work. What
> do you think?
>

That will be ideal.  Are the 'weak'/'strong' binding for those functions
well specified in the standard (can be relied upon)?

David


>
>
>  -Hal
>
>
> David
>
>
>> Thanks again,
>> Hal
>>
>>
>> David
>>
>>
>>
>>> http://reviews.llvm.org/D18013
>>>
>>>
>>>
>>>
>>
>>
>>
>> --
>> Hal Finkel
>> Assistant Computational Scientist
>> Leadership Computing Facility
>> Argonne National Laboratory
>>
>
>
>
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160309/f2455eac/attachment.html>