This is an archive of the discontinued LLVM Phabricator instance.

Remove the __kmp_invoke_microtask() that relies on libffi
ClosedPublic

Authored by jlpeyton on Jul 17 2015, 9:22 AM.

Details

Reviewers
hfinkel
Summary

The first port of the library to a non-x86 architecture (ARM) introduced a kmp_invoke_microtask() function which relied on libffi. Two architecture ports after that (aarch64 and ppc64) rely on yet another kmp_invoke_microtask(). I've rearranged the MACRO logic to say "If this isn't an x86 or MIC architecture, then use the kmp_invoke_microtask() that doesn't use libffi". This also makes the library just a little more port friendly for possible future architecture ports and gets rid of a duplicate kmp_invoke_microtask() that is easily replaced by another existing __kmp_invoke_microtask().

Because I don't have access to non-x86 architectures, this patch should be tested by external users if possible although it isn't very invasive.

Diff Detail

Repository
rL LLVM

Event Timeline

jlpeyton updated this revision to Diff 30008.Jul 17 2015, 9:22 AM
jlpeyton retitled this revision from to Remove the __kmp_invoke_microtask() that relies on libffi.
jlpeyton updated this object.
jlpeyton added a reviewer: hfinkel.
jlpeyton set the repository for this revision to rL LLVM.

Why is libffi a depdendency at all? What if it's not installed? What's
used on OSX and Windows? If it's performance impacting should a more
portable altenternative be explored?

I haven't looked at the code, but is this a way to allow for scalable
OMP 3 tasks?

Why is libffi a depdendency at all?

Because the first ARM port had it in there.

What if it's not installed?

Then the linking step of the build will fail.

What's used on OSX and Windows?

__kmp_invoke_microtask in z_Linux_asm.s and z_Windows_NT-586_asm.asm respectively.

If it's performance impacting should a more portable altenternative be explored?

I believe if anything, it will only improve performance since calls to libffi are avoided.

I haven't looked at the code, but is this a way to allow for scalable OMP 3 tasks?

It is the low-level mechanism to invoke implicit (from omp parallel) and explicit (from omp task) tasks.
It does not relate to scalable tasking algorithms.

hfinkel edited edge metadata.Jul 20 2015, 10:02 AM

Why is libffi a depdendency at all?

Because the first ARM port had it in there.

What if it's not installed?

Then the linking step of the build will fail.

What's used on OSX and Windows?

__kmp_invoke_microtask in z_Linux_asm.s and z_Windows_NT-586_asm.asm respectively.

That's correct, the generic mechanism used by PowerPC, etc. is faster.

The underlying issue is that X86 needs this generic va_args-like dispatching to handle legacy icc-compiled code. Clang, however, only generates code requiring a fixed (small) number of arguments. As I understand it from Jim, icc does not even need this capability any more (acting more like Clang does now), and in any case, we don't need the extra complexity for non-X86 targets.

If it's performance impacting should a more portable altenternative be explored?

I believe if anything, it will only improve performance since calls to libffi are avoided.

I haven't looked at the code, but is this a way to allow for scalable OMP 3 tasks?

It is the low-level mechanism to invoke implicit (from omp parallel) and explicit (from omp task) tasks.
It does not relate to scalable tasking algorithms.

As I understand it from Jim, icc does not even need this capability any more (acting more like Clang does now)

No, Intel compilers continue to generate code that uses the interface that passes multiple arguments, so *do* require the ability to pack and unpack multiple arguments.

  • Jim

James Cownie <james.h.cownie@intel.com>
SSG/DPD/TCAR (Technical Computing, Analyzers and Runtimes)
Tel: +44 117 9071438

hfinkel accepted this revision.Jul 25 2015, 4:53 PM
hfinkel edited edge metadata.

LGTM. We should scrap all libffi dependencies.

This revision is now accepted and ready to land.Jul 25 2015, 4:53 PM
AndreyChurbanov added a subscriber: AndreyChurbanov.

committed svn rev. 244031.