Hahnfeld (Jonas Hahnfeld)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 2 2015, 4:52 AM (146 w, 2 d)
RWTH Aachen University

Recent Activity

Thu, Jan 18

Hahnfeld committed rL322858: [OpenMP] Correct generation of offloading entries.
[OpenMP] Correct generation of offloading entries
Thu, Jan 18, 7:42 AM
Hahnfeld committed rC322858: [OpenMP] Correct generation of offloading entries.
[OpenMP] Correct generation of offloading entries
Thu, Jan 18, 7:42 AM
Hahnfeld closed D42168: [OpenMP] Correct generation of offloading entries.
Thu, Jan 18, 7:42 AM
Hahnfeld updated the diff for D42168: [OpenMP] Correct generation of offloading entries.

Use llvm::Twine, add PackedAttr, and adjust regression tests to check the name of the generated constants.

Thu, Jan 18, 5:58 AM
Hahnfeld created D42238: [CMake] Remove -stdlib= which is unused when passing -nostinc++.
Thu, Jan 18, 4:56 AM
Hahnfeld committed rOMP322830: Add missing headers for Debug builds.
Add missing headers for Debug builds
Thu, Jan 18, 3:02 AM
Hahnfeld committed rL322830: Add missing headers for Debug builds.
Add missing headers for Debug builds
Thu, Jan 18, 3:00 AM

Wed, Jan 17

Hahnfeld added a comment to D42168: [OpenMP] Correct generation of offloading entries.
  1. Please, mark the record as packed.
Wed, Jan 17, 8:52 AM
Hahnfeld added a comment to D42168: [OpenMP] Correct generation of offloading entries.

After some investigation, I think this whole code is incorrect. We should not rely on the fact that there is no padding on some architectures and should mix the type generated by the Clang and initial value, generated by LLVM.

Wed, Jan 17, 7:49 AM
Hahnfeld added a comment to D42169: [OMPT] Formatting.

Honestly I don't really care, but please keep the indention in ompt_try_start_tool or move the comment so that clang-format doesn't get it wrong.

Wed, Jan 17, 6:00 AM
Hahnfeld added a comment to D42169: [OMPT] Formatting.

Hmm, this should generally be done for each change and not for the sake of formatting: https://llvm.org/docs/CodingStandards.html#introduction

Wed, Jan 17, 5:19 AM
Hahnfeld created D42168: [OpenMP] Correct generation of offloading entries.
Wed, Jan 17, 5:13 AM
Hahnfeld accepted D41166: [OMPT] Rename ompt_mutex_impl_t to kmp_mutex_impl.

LGTM then

Wed, Jan 17, 1:15 AM

Thu, Jan 11

Hahnfeld added a comment to D41817: [OMPT] Enable OMPT on 32-bit ARM machines.

Ahh, I didn't say it isn't working, but I also didn't explicitly said that: when libomp+OMPT is built with clang on 32-bit ARM, all of OMPT test cases pass iff they are compiled with clang.
The problem is on 32-bit ARM and GCC.

Library built with: Clang  | GCC
Testing with:              |
Clang               works  | fails
GCC                 fails  | fails
Thu, Jan 11, 1:14 AM · Restricted Project
Hahnfeld added a comment to D41817: [OMPT] Enable OMPT on 32-bit ARM machines.

Ok, relevant line changed. I didn't notice that although kmp_platform.h (where KMP_ARCH_ARM is defined) is included from callback.h, kmp_os.h where KMP_COMPILER_GCC is defined is not included.
Also note that both clang++ and g++ define __GNUC__ hence full check for GNU C++ compiler looks as follows:

#if defined(__GNUC__) && !defined(__clang__)
#warning this is a GNU compiler
#else
#warning this is NOT a GNU compiler
#endif
Thu, Jan 11, 12:07 AM · Restricted Project

Wed, Jan 10

Hahnfeld added a comment to D41817: [OMPT] Enable OMPT on 32-bit ARM machines.

I'm not sure why you need to treat KMP_COMPILER_GCC differently, it's the test compiler that is relevant, isn't it?

Wed, Jan 10, 12:29 PM · Restricted Project
Hahnfeld added a comment to D41904: [OMPT] Add tool_not_available testcase.

Hmm, can we also somehow test that the runtime continues searching for a tool in that case? For example having OMP_TOOL_LIBRARIES with a tool that has no ompt_start_tool, one that returns NULL and a final one that does activate OMPT?

Wed, Jan 10, 5:51 AM
Hahnfeld accepted D41176: [OMPT] Fix ompt_task_data handling in implicit barriers.

Test looks good to me.

Wed, Jan 10, 2:43 AM
Hahnfeld requested changes to D41817: [OMPT] Enable OMPT on 32-bit ARM machines.

Does a Clang built runtime work for GCC? In particular, do all tests pass when building with Clang but adding -DOPENMP_TEST_C_COMPILER=gcc -DOPENMP_TEST_CXX_COMPILER=g++?

No, Clang built runtime doesn't work for GCC:

********************
Failing Tests (13):
    libomp :: ompt/parallel/dynamic_enough_threads.c
    libomp :: ompt/parallel/dynamic_not_enough_threads.c
    libomp :: ompt/parallel/no_thread_num_clause.c
    libomp :: ompt/parallel/normal.c
    libomp :: ompt/parallel/not_enough_threads.c
    libomp :: ompt/synchronization/lock.c
    libomp :: ompt/synchronization/nest_lock.c
    libomp :: ompt/synchronization/test_lock.c
    libomp :: ompt/tasks/task_in_joinbarrier.c
    libomp :: ompt/tasks/untied_task.c
    libomp :: ompt/worksharing/for/dynamic.c
    libomp :: ompt/worksharing/for/guided.c
    libomp :: ompt/worksharing/for/runtime.c
Wed, Jan 10, 12:16 AM · Restricted Project
Hahnfeld committed rOMP322160: [OMPT] Fix cast and printf of wait_id in lock test.
[OMPT] Fix cast and printf of wait_id in lock test
Wed, Jan 10, 12:12 AM
Hahnfeld committed rL322160: [OMPT] Fix cast and printf of wait_id in lock test.
[OMPT] Fix cast and printf of wait_id in lock test
Wed, Jan 10, 12:11 AM
Hahnfeld closed D41853: [OMPT] Fix cast and printf of wait_id in lock test.
Wed, Jan 10, 12:11 AM

Tue, Jan 9

Hahnfeld added a comment to D41817: [OMPT] Enable OMPT on 32-bit ARM machines.

Nah, doing architecture && compiler doesn't seem to work. I've got some better idea.

Tue, Jan 9, 11:07 PM · Restricted Project
Hahnfeld added a comment to D41853: [OMPT] Fix cast and printf of wait_id in lock test.

But (uint64_t) &lock actually extends the 32-bit address to 64-bit signed integer: fffef9c8 -> fffffffffffef9c8
This is the same value as provided by the callback function.

Tue, Jan 9, 11:04 PM
Hahnfeld added a comment to D14254: [OpenMP] Initial implementation of OpenMP offloading library - libomptarget device RTLs..

Two global remarks:

  1. I think we agreed on having <thread id> % <warp size> instead of bit operations.

What's wrong with bit-wise operations as long as they are documented? I think we should keep them and then comment what it is that they do.

Tue, Jan 9, 7:49 AM · Restricted Project
Hahnfeld added a comment to D41817: [OMPT] Enable OMPT on 32-bit ARM machines.

Found that __builtin_frame_address(1) gives wrong answer in __kmp_api_GOMP_parallel (a.k.a KMP_API_NAME_GOMP_PARALLEL as in kmp_gsupport.cpp). Then I've found out that this won't work with gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60109
Status: RESOLVED WONTFIX
We can mark it UNSUPPORTED on arm32/gcc

Tue, Jan 9, 7:47 AM · Restricted Project
Hahnfeld accepted D41854: [OMPT] Fix type mismatch in omp_control_tool() implementation that makes it run incorrectly on 32-bit machines.

LGTM

Tue, Jan 9, 2:14 AM · Restricted Project
Hahnfeld created D41853: [OMPT] Fix cast and printf of wait_id in lock test.
Tue, Jan 9, 2:08 AM
Hahnfeld added a comment to D41817: [OMPT] Enable OMPT on 32-bit ARM machines.

It's kmp_gsupport.cpp issue I presume, was it ever tested on any 32-bit platform? I'll try to look at it through the day.

Yes, all tests pass on i386 AFAICS

Tue, Jan 9, 1:57 AM · Restricted Project

Mon, Jan 8

Hahnfeld added a comment to D41817: [OMPT] Enable OMPT on 32-bit ARM machines.

It's kmp_gsupport.cpp issue I presume, was it ever tested on any 32-bit platform? I'll try to look at it through the day.

Mon, Jan 8, 11:42 PM · Restricted Project
Hahnfeld added a comment to D31600: KMP_HW_SUBSET extended with NUMA support when HWLOC enabled.

I'd vote for closing this request (maybe approving it first). The problem with this patch you raised was fixed by the commit rL301349 (D32496).

Mon, Jan 8, 11:00 AM · Restricted Project
Hahnfeld added a comment to D41817: [OMPT] Enable OMPT on 32-bit ARM machines.

I didn't even know these tests are allowed to be compiled with anything else than clang 6.0+...

Mon, Jan 8, 8:30 AM · Restricted Project
Hahnfeld requested changes to D41817: [OMPT] Enable OMPT on 32-bit ARM machines.

I think I saw a few failures related to return addresses on my Raspberry Pi 2... I'll try to retest asap.

Mon, Jan 8, 7:33 AM · Restricted Project
Hahnfeld added a reviewer for D41817: [OMPT] Enable OMPT on 32-bit ARM machines: protze.joachim.

I think I saw a few failures related to return addresses on my Raspberry Pi 2... I'll try to retest asap.

Mon, Jan 8, 5:47 AM · Restricted Project

Sun, Jan 7

Hahnfeld added a comment to D14254: [OpenMP] Initial implementation of OpenMP offloading library - libomptarget device RTLs..

Two global remarks:

  1. I think we agreed on having <thread id> % <warp size> instead of bit operations.
  2. Somewhat related, there are currently at least four different "symbols" for the warp size: warpSize, WARPSIZE, DS_Max_Worker_Warp_Size, and DS_Max_Warp_Number. This needs to be exactly one that is used throughout the runtime.
Sun, Jan 7, 10:10 AM · Restricted Project
Hahnfeld committed rL321965: Remove openmp-ompt builders.
Remove openmp-ompt builders
Sun, Jan 7, 9:01 AM
Hahnfeld committed rOMP321964: Correct types of pointers to doacross_num_done.
Correct types of pointers to doacross_num_done
Sun, Jan 7, 8:57 AM
Hahnfeld committed rL321964: Correct types of pointers to doacross_num_done.
Correct types of pointers to doacross_num_done
Sun, Jan 7, 8:55 AM
Hahnfeld closed D41656: Correct types of pointers to doacross_num_done.
Sun, Jan 7, 8:55 AM

Thu, Jan 4

Hahnfeld added a comment to D41724: [OpenMP][libomptarget] Enable the compilation of multiple bc libraries for runtime inlining.

Does the bitcode lib work with upstream Clang trunk?

Thu, Jan 4, 6:56 AM

Tue, Jan 2

Hahnfeld committed rL321659: [opt-viewer] Check for pygments.lexer.c_cpp.
[opt-viewer] Check for pygments.lexer.c_cpp
Tue, Jan 2, 9:54 AM
Hahnfeld closed D41611: [opt-viewer] Check for pygments.lexer.c_cpp.
Tue, Jan 2, 9:54 AM
Hahnfeld accepted D41508: [OMPT] Build runtime with OMPT support by default.

LG. I'd feel better with an explicit acknowledgment from Intel though...

Tue, Jan 2, 3:39 AM

Mon, Jan 1

Hahnfeld committed rCRT321627: [scudo] Touch memory to count as RSS.
[scudo] Touch memory to count as RSS
Mon, Jan 1, 10:23 AM
Hahnfeld committed rL321627: [scudo] Touch memory to count as RSS.
[scudo] Touch memory to count as RSS
Mon, Jan 1, 10:20 AM
Hahnfeld closed D41649: [scudo] Touch memory to count as RSS.
Mon, Jan 1, 10:20 AM · Restricted Project
Hahnfeld created D41656: Correct types of pointers to doacross_num_done.
Mon, Jan 1, 4:46 AM
Hahnfeld added a comment to D41128: [scudo] Adding a public Scudo interface.

I am not sure I understand correctly the question. So I will attempt an answer, and if it doesn't apply, let me know.

If compiled with Scudo, all allocation functions are handled by the allocator (malloc, realloc, calloc, memalign, etc).
The soft/hard RSS limit feature comes from a need from some applications to enforce an upper limit to the memory used.
This is mostly needed due to the fact that we reserve a large portion of the address space, and as such, setting an rlimit is not viable.
The RSS limit comes as an opportunistic, best-effort, check to allow for some type of upper bound check.
It is not enabled by default (eg: 0), and it is expansive (a few syscalls to read from /proc).
If an application uses directly mmap (or any OS backed memory allocation function), we would only catch a limit "overflow" on the next use of a heap allocation function.
Also if we allocate/free some memory backed by the Secondary allocator quickly (before the check interval elapses), we wouldn't catch it as well.

I hope this answers your question.

Mon, Jan 1, 2:05 AM
Hahnfeld updated the diff for D41649: [scudo] Touch memory to count as RSS.

Use memset instead of custom touch function.

Mon, Jan 1, 1:59 AM · Restricted Project
Hahnfeld added a comment to D41649: [scudo] Touch memory to count as RSS.

How about using calloc instead and benefiting of the "included" zero'd memory feature? That would avoid implementing the touch function.
I won't be able to test anything before Jan. 2nd.

Mon, Jan 1, 1:58 AM · Restricted Project

Sun, Dec 31

Hahnfeld added a comment to D41128: [scudo] Adding a public Scudo interface.

The added test has been disabled on armhf in rCRT320665 because it fails a bot: http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2952/steps/ninja%20check%202/logs/FAIL%3A%20Scudo-armhf%3A%3A%20interface.cpp

I see the exact same error on x86_64 CentOS 7, did you find out what the problem is? I'm happy to assist if you have problems to reproduce the problem...

I haven't been able to repro the issue. If you have a configuration or some insight as to what the failure is, please let me know!

Sun, Dec 31, 3:58 PM
Hahnfeld created D41649: [scudo] Touch memory to count as RSS.
Sun, Dec 31, 3:53 PM · Restricted Project

Thu, Dec 28

Hahnfeld created D41611: [opt-viewer] Check for pygments.lexer.c_cpp.
Thu, Dec 28, 1:55 AM
Hahnfeld removed a reviewer for D41486: [OpenMP][Clang] Add missing argument to runtime functions.: Hahnfeld.

Not needed anymore, we decided on the order of arguments in D41012.

Thu, Dec 28, 1:54 AM

Wed, Dec 27

Hahnfeld added a comment to D41128: [scudo] Adding a public Scudo interface.

The added test has been disabled on armhf in rCRT320665 because it fails a bot: http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2952/steps/ninja%20check%202/logs/FAIL%3A%20Scudo-armhf%3A%3A%20interface.cpp

Wed, Dec 27, 3:06 AM
Hahnfeld committed rCRT321485: [XRay] Add missing include to unit test.
[XRay] Add missing include to unit test
Wed, Dec 27, 2:45 AM
Hahnfeld committed rC321486: [OpenMP] Further adjustments of nvptx runtime functions.
[OpenMP] Further adjustments of nvptx runtime functions
Wed, Dec 27, 2:41 AM
Hahnfeld committed rL321486: [OpenMP] Further adjustments of nvptx runtime functions.
[OpenMP] Further adjustments of nvptx runtime functions
Wed, Dec 27, 2:41 AM
Hahnfeld closed D41012: [OpenMP] Further adjustments of nvptx runtime functions.
Wed, Dec 27, 2:41 AM
Hahnfeld committed rL321485: [XRay] Add missing include to unit test.
[XRay] Add missing include to unit test
Wed, Dec 27, 2:40 AM
Hahnfeld committed rOMP321481: Unify build documentation and convert to reStructuredText.
Unify build documentation and convert to reStructuredText
Wed, Dec 27, 1:24 AM
Hahnfeld added a comment to D41508: [OMPT] Build runtime with OMPT support by default.

Looks good so far. Can you document the change of the default value in the (just committed) README.rst?

Wed, Dec 27, 1:17 AM
Hahnfeld committed rL321481: Unify build documentation and convert to reStructuredText.
Unify build documentation and convert to reStructuredText
Wed, Dec 27, 1:16 AM
Hahnfeld closed D40920: Unify build documentation and convert to reStructuredText.
Wed, Dec 27, 1:16 AM

Sat, Dec 23

Hahnfeld accepted D41165: [OMPT] Set frame address when creating a task with dependences.

Yes, that is the idea behind this change. This avoids to lookup the same pointer in two ways and especially I need the address also at the end of the function.

Sat, Dec 23, 7:01 AM
Hahnfeld added a comment to D41165: [OMPT] Set frame address when creating a task with dependences.

Does the latest change assume current_task == new_taskdata->td_parent (which would make sense to me)? If not this change has more implications than the frame address.

Sat, Dec 23, 6:39 AM
Hahnfeld added inline comments to D41508: [OMPT] Build runtime with OMPT support by default.
Sat, Dec 23, 2:48 AM

Fri, Dec 22

Hahnfeld accepted D41542: [OMPT] Add missing initialization in nested_lwt.c test case.

LG

Fri, Dec 22, 5:17 AM
Hahnfeld accepted D41499: [OMPT] Fix failing test cases for gcc on Ubuntu.

Makes sense.

Fri, Dec 22, 2:25 AM
Hahnfeld requested changes to D41508: [OMPT] Build runtime with OMPT support by default.

I don't think we should enable this unconditionally, only for the architectures where we tested and are sure that the tests will pass: x86, x86_64, ppc64, aarch64 on Linux, macOS if we want to. With the current change we will get into trouble at least on MIPS(64).

Fri, Dec 22, 2:24 AM

Dec 21 2017

Hahnfeld updated the diff for D40920: Unify build documentation and convert to reStructuredText.

Address review comments and pay attention to protected names.

Dec 21 2017, 7:54 AM
Hahnfeld added inline comments to D41171: [OMPT] Handle null pointer in set_callback to improve performance.
Dec 21 2017, 7:46 AM
Hahnfeld added inline comments to D41485: [OpenMP][libomptarget] Add data sharing support in libomptarget.
Dec 21 2017, 6:02 AM
Hahnfeld updated subscribers of D41485: [OpenMP][libomptarget] Add data sharing support in libomptarget.
Dec 21 2017, 4:39 AM
Hahnfeld requested changes to D41486: [OpenMP][Clang] Add missing argument to runtime functions..

D41012? This patch doesn't update the documentation with function signatures.

Dec 21 2017, 4:38 AM
Hahnfeld added inline comments to D41485: [OpenMP][libomptarget] Add data sharing support in libomptarget.
Dec 21 2017, 4:37 AM
Hahnfeld added a comment to D40920: Unify build documentation and convert to reStructuredText.

Ping. Is this ok to land before the 6.0 branch?

Dec 21 2017, 3:04 AM
Hahnfeld updated the diff for D40920: Unify build documentation and convert to reStructuredText.

Removed documentation about the NVPTX runtime for now which isn't committed yet.

Dec 21 2017, 3:02 AM
Hahnfeld accepted D41482: AArch64: add required arch specific code for running OMPT test cases.

LGTM

Dec 21 2017, 2:46 AM
Hahnfeld requested changes to D41166: [OMPT] Rename ompt_mutex_impl_t to kmp_mutex_impl.
Dec 21 2017, 2:43 AM
Hahnfeld accepted D41171: [OMPT] Handle null pointer in set_callback to improve performance.

LGTM

Dec 21 2017, 2:38 AM
Hahnfeld added a comment to D39457: [OPENMP] Current status of OpenMP support..

@hfinkel I think you requested this documentation on the mailing list. Can you take a look if it matches your expectations so we can get this bundled in the 6.0 release?

Dec 21 2017, 1:08 AM

Dec 20 2017

Hahnfeld added inline comments to D14254: [OpenMP] Initial implementation of OpenMP offloading library - libomptarget device RTLs..
Dec 20 2017, 12:59 PM · Restricted Project
Hahnfeld requested changes to D14254: [OpenMP] Initial implementation of OpenMP offloading library - libomptarget device RTLs..

Good to go @Hahnfeld ?

Dec 20 2017, 2:13 AM · Restricted Project
Hahnfeld accepted D41165: [OMPT] Set frame address when creating a task with dependences.

LG. This fixes the test with GCC which already had the enter address, right?

Dec 20 2017, 1:23 AM
Hahnfeld added a comment to D41166: [OMPT] Rename ompt_mutex_impl_t to kmp_mutex_impl.

Makes sense in some way. However if I read the spec correctly the tool gets an unsigned int. So maybe we should keep the enum in the header but prefix it with kmp instead of ompt to make clear that this is not portable? This would allow tools to conditionally compare the passed value

Dec 20 2017, 1:21 AM
Hahnfeld added a comment to D41167: [OMPT] Return appropiate values for ompt runtime entry points when the current thread is not an OpenMP thread.

Should we add a test for the case ompt_* calls from non-OpenMP thread?

Dec 20 2017, 1:10 AM
Hahnfeld accepted D41181: [OMPT] Fix return address handling in a few GOMP interface methods.

LG but please describe which test cases are fixed by this change (and possibly why it is needed). If no test case has been failing before, this is a sign that we need a new one!

Dec 20 2017, 1:08 AM
Hahnfeld added a comment to D41182: [OMPT] Add Workaround for Intel Compiler Bug.
icc bug

What bug it is and which version of ICC is affected?

Dec 20 2017, 1:05 AM
Hahnfeld added inline comments to D41171: [OMPT] Handle null pointer in set_callback to improve performance.
Dec 20 2017, 1:00 AM
Hahnfeld added a comment to D41171: [OMPT] Handle null pointer in set_callback to improve performance.

This change is more a performace optimization. (reset the bit in the bitmap to 0)
The callback was set to NULL before and all callback invocation checks for the NULL-pointer.

In some cases we do not enter a code path if a specific callback is not set. With this patch this code path is also not entered if the callback was set and reset to NULL.

Dec 20 2017, 1:00 AM

Dec 13 2017

Hahnfeld requested changes to D41171: [OMPT] Handle null pointer in set_callback to improve performance.

Can we have a test for this? Like setting the callback, unsetting it again and checking that the function isn't called?

Dec 13 2017, 6:03 AM
Hahnfeld requested changes to D41176: [OMPT] Fix ompt_task_data handling in implicit barriers.

This needs a test.

Dec 13 2017, 6:03 AM
Hahnfeld accepted D40595: [OMPT] Use frames at different level when using clang version 5 or higher with debug flag.

LG

Dec 13 2017, 5:59 AM
Hahnfeld accepted D40384: [OMPT] Add annotations to testcases that are expected to fail when using certain compilers.

Two small things to considers, looks good otherwise.

Dec 13 2017, 5:47 AM
Hahnfeld accepted D41000: AArch64: fix cpuinfo issues.

LGTM if you add a comment in the code explaining why we need this (old format) and that this effectively disables error detection in the new format on AArch64

Dec 13 2017, 5:38 AM · Restricted Project

Dec 9 2017

Hahnfeld added inline comments to D41000: AArch64: fix cpuinfo issues.
Dec 9 2017, 4:34 PM · Restricted Project

Dec 8 2017

Hahnfeld added a comment to D14254: [OpenMP] Initial implementation of OpenMP offloading library - libomptarget device RTLs..

Here is a general comment, we should follow Alex's suggestion to make a generic GPU device and extend that to NVPTX and AMDGCN. My current idea is:

  1. think all the current spell of nvptx as generic_gpu
  2. add template to some functions, and instantiate them with nvptx and amdgcn required types
  3. for example the warp size in nvptx is 32 while in amdgcn is 64

    Thoughts?
Dec 8 2017, 5:03 PM · Restricted Project
Hahnfeld created D41012: [OpenMP] Further adjustments of nvptx runtime functions.
Dec 8 2017, 8:44 AM
Hahnfeld committed rL320153: [CMake] Remove legacy LIBOMP_LIT_ARGS.
[CMake] Remove legacy LIBOMP_LIT_ARGS
Dec 8 2017, 7:08 AM