[Openmp-commits] [Openmp-dev] Three more patches

[C Bergström]

I don't want to block your patch, but I would love to see any and all
functionality be testable by non-intel compilers if and where possible.

Can you give the dummies step by step instructions on how to test/validate
this feature. Maybe starting a document called TESTING or similar is a good
idea. If the Intel weekly results aren't publicly available then that's not
useful to anyone outside of Intel. I won't know if I broke something when I
change the code..

Thanks

On Fri, Jan 16, 2015 at 8:46 PM, Churbanov, Andrey <
Andrey.Churbanov at intel.com> wrote:

>  OK, I have committed the lrb2mic* patches (revisions 226271, 226272).
>
>
>
> As to ittnotify fixes/testing, we don't have such tests.  We rely on a
> regular (weekly) testing of ittnotify functionality in our library by the
> Intel(R) VTune Amplifier quality assurance team.  Theoretically it is
> possible to write a test that would check this functionality without
> running VTune, but this would require the test to emulate a lot of
> functionality of the tool: provide ittnotify library (e.g. built from
> sources at
> https://software.intel.com/en-us/articles/intel-itt-api-open-source),
> register this library within the OpenMP RTL, collect data reported by the
> OpenMP RTL, analyze the data, report the result.  We just don't have the
> will and resources to write such a complicated test, given that the
> functionality is tested regularly by our colleagues from VTune team.
>
>
>
> Can I commit the ittnotify patch without providing the test?
>
>
>
> Thanks,
>
> Andrey
>
>
>
> *From:* openmp-dev-bounces at cs.uiuc.edu [mailto:
> openmp-dev-bounces at cs.uiuc.edu] *On Behalf Of *C Bergstrom
> *Sent:* Thursday, January 15, 2015 9:45 AM
> *To:* Peyton, Jonathan L
> *Cc:* openmp-dev at dcs-maillist2.engr.illinois.edu
> *Subject:* Re: [Openmp-dev] Three more patches
>
>
>
>
>
> On Thu, Jan 15, 2015 at 6:59 AM, Peyton, Jonathan L <
> jonathan.l.peyton at intel.com> wrote:
>
>  I have three more patches here and there descriptions below.
> Lrb2mic_cmake_build.patch depends on lrb2mic_make_build.patch so it must go
> after. But the ittnotify_fixes.patch is independent of the rest.
>
>
>
> Lrb2mic_make_build.patch – This patch (and the similarly named
> lrb2mic_cmake_build.patch) gets rid of all the references to lrb when
> building for Intel® Many Integrated Core Architecture.  I know most people
> aren’t compiling for this architecture and this patch is somewhat invasive,
> but it does not affect the Intel® 64 or IA-32 builds, nor any other
> architecture’s build.  Here are the main features this patch fixes:
>
> 1) exports directory renamed to lin_knc, depending on which MIC flavor you
> are compiling for,
>
> 2) The internal perl scripts and library modules now have a variable
> $target_mic_arch which holds the value of “knc”, “knf”, (“knl” in the
> future) for Knights Corner, Knights Ferry, and Knights Landing.
>
> 3) tools used for checks are properly named to use MIC specific tools
> (e.g., instead of objcopy use x86_64-k1om-linux-objcopy)
>
> 4) build.pl doesn’t take os=lrb, but instead takes arch=mic &
> mic_arch=knc.  This is done under the hood of the top level Makefile.  You
> still build using the top level Makefile by typing $ make compiler=icc
> arch=mic
>
> 5) During this change, I got rid of some extraneous build.pl options
> (e.g., tcheck, target-compiler, etc.) which aren’t used anymore.
>
>
>
> Lrb2mic_cmake_build.patch – This patch is the CMake changes that
> correspond to the above patch.  This patch is built off the last one and is
> only the CMake files that are affected.  This patch also does a few other
> things:
>
> 1) I changed some compiler flag settings for icc only that were wrong for
> MIC builds.
>
> 2) Instead of explicit linking to –ldl , I changed cmake to use
> CMAKE_DL_LIBS which links the –ldl library if it exists on platform you are
> compiling for.
>
> 3) Now to compile for MIC you specify –Dos=lin –Darch=mic instead of
> –Dos=mic ; it is an architecture after all that runs Linux as the os.
>
>
>
> Ittnotify_fixes.patch – This patch fixes a few things concerning the
> ittnotify interface.
>
> 1) Metadata for single construct used to be reported from all threads.
> This was fixed to be reported only by the master.
>
> 2) Region-begin was mistakenly saved for any active level but had to be
> saved only for the first level.
>
> 3) Added code to report location for #omp single metadata.
>
>
>
> To apply patches:
>
> $ patch –p0 < lrb2mic_make_build.patch
>
> $ patch –p0 < lrb2mic_cmake_build.patch
>
> $ patch –p0 < ittnotify_fixes.patch
>
>
>
> Comments or Questions?
>
>
>
> For the ittnotify fixes is there any corresponding test case? Even if we
> don't have a test harness which can easily run it, having something would
> be great so we can get in the habit of requiring this for bug fixes.
>
>
>
> For the other patches maybe someone @Intel can test and sign off on the
> cod review
>
>
>
> Thanks
>
>
> --------------------------------------------------------------------
> Closed Joint Stock Company Intel A/O
> Registered legal address: Krylatsky Hills Business Park,
> 17 Krylatskaya Str., Bldg 4, Moscow 121614,
> Russian Federation
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20150116/5418fecb/attachment.html>