Page MenuHomePhabricator

Hahnfeld (Jonas Hahnfeld)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Mon, Mar 30

Hahnfeld committed rGced99a1a6368: Fix comment for CLANG_SYSTEMZ_DEFAULT_ARCH (authored by Hahnfeld).
Fix comment for CLANG_SYSTEMZ_DEFAULT_ARCH
Mon, Mar 30, 1:04 PM
Hahnfeld added a comment to rGc506adcdf2ca: Move CLANG_SYSTEMZ_DEFAULT_ARCH to config.h..

@thakis thanks for taking care so quickly!

Mon, Mar 30, 1:04 PM
Hahnfeld added inline comments to D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH.
Mon, Mar 30, 5:55 AM · Restricted Project, Restricted Project

Thu, Mar 26

Hahnfeld added a comment to D76780: [OpenMP] Added memory barrier to solve data race.

Why doesn't the KMP_MB after the store to th_next_waiting guarantee that the unblocked thread sees that store?

Answering my own question, the ARM architecture reference manual states that the DMB instruction ensures that all affected memory accesses by the PE executing the DMB that appear in program order before the DMB and those which originate from a different PE, ...which have been Observed-by the PE before the DMB is executed, are Observed-by each PE, ...before any affected memory accesses that appear in program order after the DMB are Observed-by that PE.

I think this means, in this case, that the store of FALSE to th_spin_here is observed in the correct order only by the releasing thread that issued the DMB. Other threads (e.g. the spinning thread) could still see the updates to th_spin_here and th_next_waiting out of order, unless they also issue DMB, which is the fix in this patch.

Thu, Mar 26, 1:03 AM · Restricted Project

Wed, Mar 25

Hahnfeld accepted D76780: [OpenMP] Added memory barrier to solve data race.

Yes, this looks related to memory consistency. As far as I understand the threads synchronize on th_spin_here, so this is guaranteed to be updated. Any other write before this in __kmp_release_queuing_lock is not guaranteed to be synchronized by a weak memory model. This includes th_next_waiting (which triggers the assertion), but also writes by the user application. That's particularly bad because this should be taken care of by the runtime!

Wed, Mar 25, 11:21 AM · Restricted Project

Sat, Mar 14

Hahnfeld added a comment to D75001: [OpenMP][cmake] ignore warning on unknown CUDA version .

I like this way better. I was hoping we could do it in our cmake only :)

Give others a day or so to comment before your commit but I'm fine with this.

Well, that doesn't really work if openmp-commits is only subscribed on commit. That said, the solution is a bit ugly but I don't have an alternative right now.

What is the problem with openmp-commits here? I got the emails, didn't you?

Sat, Mar 14, 2:06 AM · Restricted Project

Fri, Mar 6

Hahnfeld committed rGf0689d2e6209: archer: Remove superfluous dot from warning message (authored by Hahnfeld).
archer: Remove superfluous dot from warning message
Fri, Mar 6, 6:37 AM

Feb 25 2020

Hahnfeld added a comment to D75001: [OpenMP][cmake] ignore warning on unknown CUDA version .

I like this way better. I was hoping we could do it in our cmake only :)

Give others a day or so to comment before your commit but I'm fine with this.

Feb 25 2020, 6:56 AM · Restricted Project

Feb 11 2020

Hahnfeld resigned from D74258: [OpenMP] Switch default C++ standard to C++ 14.
Feb 11 2020, 12:47 AM · Restricted Project

Feb 8 2020

Hahnfeld added a comment to D74258: [OpenMP] Switch default C++ standard to C++ 14.

Unless there are good reasons to switch, I'd object because it requires newer versions of compilers than many HPC systems have.

I disagree. The use case we would be supporting is:

  • building llvm libomptarget from source
  • using it with a binary llvm dist, or some other toolchain
  • won't download or build a newer toolchain
Feb 8 2020, 4:24 AM · Restricted Project
Hahnfeld requested changes to D74258: [OpenMP] Switch default C++ standard to C++ 14.

This should have a RFC on openmp-dev. As far as I understand the linked patch, the aim is to use std::make_unique which is just syntactic sugar IMO. Unless there are good reasons to switch, I'd object because it requires newer versions of compilers than many HPC systems have.

Feb 8 2020, 3:48 AM · Restricted Project

Feb 3 2020

Hahnfeld accepted D73850: [OpenMP][OMPT] fix reduction test for 32-bit x86.

LGTM from the code, I didn't test that the issue is indeed fixed on i386 though

Feb 3 2020, 10:11 AM · Restricted Project

Feb 2 2020

Hahnfeld added a comment to D73850: [OpenMP][OMPT] fix reduction test for 32-bit x86.

@protze.joachim for 64 bit, we still need at least 5 threads IIRC

Feb 2 2020, 10:53 AM · Restricted Project

Jan 29 2020

Hahnfeld requested changes to D73657: [OPENMP] Load plugins from same directory as the libomptarget.so and quick fail mechanism for offloading plugins.

As a general comment, I think this patch should be split into at least two separate ones:

  1. Load plugins from the same directory as libomptarget.so is found.
  2. Quick fail mechanism, although I'm not sure how much this saves? In most cases, we won't have the plugins for a foreign architecture (PPC on x86) anyway, that might only be a concern for accelerators.
Jan 29 2020, 11:59 PM · Restricted Project

Jan 23 2020

Hahnfeld added a comment to D73249: [openmp] Disable archer if LIBOMP_OMPT_SUPPORT is off.

I'm not sure LIBOMP_OMPT_SUPPORT is guaranteed to be there yet when defined in runtime/.

If by 'guaranteed' you mean in the current code, then I've tested this patch with it being both true and false.

Jan 23 2020, 12:48 AM · Restricted Project
Hahnfeld added a comment to D73249: [openmp] Disable archer if LIBOMP_OMPT_SUPPORT is off.

I'm not sure LIBOMP_OMPT_SUPPORT is guaranteed to be there yet when defined in runtime/. Maybe it would be better to move the option to the top-level openmp/CMakeList.txt and rename it to OPENMP_OMPT_SUPPORT (with compatibility support for LIBOMP_OMPT_SUPPORT)

Jan 23 2020, 12:13 AM · Restricted Project

Jan 16 2020

Hahnfeld accepted D72779: [OpenMP][Tool] Fix memory leak and double-allocation.

LG

Jan 16 2020, 11:29 AM · Restricted Project
Hahnfeld added inline comments to D72779: [OpenMP][Tool] Fix memory leak and double-allocation.
Jan 16 2020, 12:35 AM · Restricted Project

Jan 15 2020

Hahnfeld added inline comments to D72779: [OpenMP][Tool] Fix memory leak and double-allocation.
Jan 15 2020, 10:03 AM · Restricted Project

Jan 14 2020

Hahnfeld added a comment to D70412: [OpenMP][Tool] Runtime warning for missing TSan-option.

This revision was not accepted before being committed!

It was a misunderstanding on my side. Johannes told me that he is fine with the changes before I pushed. I didn't check that the accepted flag was not on the patch.
What procedure do you suggest?

Jan 14 2020, 11:58 PM · Restricted Project
Hahnfeld reopened D70412: [OpenMP][Tool] Runtime warning for missing TSan-option.

This revision was not accepted before being committed!

Jan 14 2020, 12:21 PM · Restricted Project

Jan 6 2020

Hahnfeld updated subscribers of D72287: [OpenMP] Fix incorrect property of __has_attribute() macro.

Can you please also submit a patch to libc++ where we copied this code from?

Jan 6 2020, 12:24 PM · Restricted Project

Dec 31 2019

Hahnfeld added a comment to D71988: [OpenMP][WIP] Make the kmp_depend_info type fit in 128 bits..

I would expect that some versioning is needed..

Versioning is fine with me. Versioning the runtime would be even preferable.

Dec 31 2019, 2:33 AM · Restricted Project

Dec 30 2019

Hahnfeld requested changes to D71988: [OpenMP][WIP] Make the kmp_depend_info type fit in 128 bits..

Yes, this must not be changed for compatibility reasons. If this really brings a benefit, we need new entry functions and update the compiler(s).

Dec 30 2019, 2:50 AM · Restricted Project

Dec 11 2019

Hahnfeld added a comment to D71340: [VE,#3] Runtime libraries.

To clarify: this patch is not intended to be committed as-is but gives an overview of the required changes. It's the a patch of a series for the VE backend and includes all the earlier patches.

Dec 11 2019, 5:13 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project

Dec 7 2019

Hahnfeld committed rG071dca24cea9: [OpenMP] Require trivially copyable type for mapping (authored by Hahnfeld).
[OpenMP] Require trivially copyable type for mapping
Dec 7 2019, 4:34 AM
Hahnfeld closed D71134: [OpenMP] Require trivially copyable type for mapping.
Dec 7 2019, 4:34 AM · Restricted Project

Dec 6 2019

Hahnfeld added inline comments to D71134: [OpenMP] Require trivially copyable type for mapping.
Dec 6 2019, 12:06 PM · Restricted Project
Hahnfeld created D71134: [OpenMP] Require trivially copyable type for mapping.
Dec 6 2019, 10:50 AM · Restricted Project

Dec 2 2019

Hahnfeld removed a reviewer for D53250: [ToolChain] Use default linker if the toolchain uses a custom one: Hahnfeld.
Dec 2 2019, 7:55 AM · Restricted Project

Nov 29 2019

Hahnfeld added a comment to D70804: [Frontend] Allow OpenMP offloading to aarch64.

Tests are required

The tests for this were added in D30644, but they are currently failing on AArch64 due to D32035. This patch allows those tests to pass again. What further testing should I add?

Nov 29 2019, 12:10 AM · Restricted Project

Nov 19 2019

Hahnfeld added a comment to D70414: [libomptarget] Build a minimal deviceRTL for amdgcn.

Random drive-by comments to parts of the patch, I won't review in full.

Nov 19 2019, 1:20 AM · Restricted Project

Nov 16 2019

Hahnfeld removed a reviewer for D68100: [OpenMP 5.0] declare mapper runtime implementation: Hahnfeld.
Nov 16 2019, 9:58 AM · Restricted Project
Hahnfeld removed a reviewer for D55725: [OpenMP] Add libs to clang-dedicated directories: Hahnfeld.
Nov 16 2019, 9:58 AM · Restricted Project
Hahnfeld removed a reviewer for D28634: [buildbot] Extend libomp builder to test bot standalone and in-tree builds with and without clang bootstrapping.: Hahnfeld.

Testing of openmp is currently very lacking, but I don't think this change will apply cleanly anymore...

Nov 16 2019, 9:58 AM
Hahnfeld removed a reviewer for D59028: [OpenMP] Enable on device linking with NVLINK to ignore dynamic libraries: Hahnfeld.
Nov 16 2019, 9:50 AM · Restricted Project
Hahnfeld removed a reviewer for D47394: [OpenMP][Clang][NVPTX] Replace bundling with partial linking for the OpenMP NVPTX device offloading toolchain: Hahnfeld.
Nov 16 2019, 9:50 AM · Restricted Project
Herald added a project to D53250: [ToolChain] Use default linker if the toolchain uses a custom one: Restricted Project.

Going through my old revisions, is this still something that we want?

Nov 16 2019, 9:50 AM · Restricted Project
Hahnfeld abandoned D52733: [OpenMP][NVPTX] Avoid data sharing if in parallel region.
Nov 16 2019, 9:50 AM · Restricted Project
Hahnfeld abandoned D65868: [OpenMP] Remove workaround for CMPXCHG.

I won't continue work on this.

Nov 16 2019, 9:50 AM · Restricted Project

Nov 5 2019

Hahnfeld updated subscribers of D68070: [Clang][OpenMP Offload] Create start/end symbols for the offloading entry table with a help of a linker.
Nov 5 2019, 1:52 PM · Restricted Project
Hahnfeld added a comment to D68070: [Clang][OpenMP Offload] Create start/end symbols for the offloading entry table with a help of a linker.

Heads up that this change broke compatibility with old binaries. Not sure if that is something worth to fix, but it should probably at least be spelt out somewhere...

Nov 5 2019, 1:34 PM · Restricted Project

Sep 28 2019

Hahnfeld set the repository for D68166: [Clang][OpenMP Offload] Add new tool for wrapping offload device binaries to rG LLVM Github Monorepo.
Sep 28 2019, 3:08 AM · Restricted Project

Sep 11 2019

Hahnfeld added a comment to D66107: [libFuzzer] Make -merge=1 to reuse coverage information from the control file..

I think the bots are also green, so it might be just related to how I build Clang (with libc++, for example). I'm half way through building ToT with GCC, that should give insight whether it's related to my system or my configuration.

Actually, some ARM bots got broken, so I've uploaded a fix: https://reviews.llvm.org/D67458

Just curious: So the number of new features and coverage edges does not need to be the same? I also have "15 new freatures" for CHECK2.

Yes, features include coverage edges + additional signal (e.g. value profiling), that's why the number is different and why number of features can differ between platforms.

Sep 11 2019, 12:50 PM · Restricted Project, Restricted Project
Hahnfeld added a comment to D66107: [libFuzzer] Make -merge=1 to reuse coverage information from the control file..

Hm, doesn't fail for me, but I guess the feature detection might be platform-dependent to some extent, so I'm fine with replacing the number of the features with a regex. Do you want to upload a change, or should I?

Sep 11 2019, 11:33 AM · Restricted Project, Restricted Project
Hahnfeld added a comment to D66107: [libFuzzer] Make -merge=1 to reuse coverage information from the control file..

The new test is failing for me because CHECK1 is not satisfied. Instead the line says MERGE-OUTER: 3 new files with 12 new features added; 11 new coverage edges (instead of 11 new features). I'm currently investigating what's wrong here, let me know if you have an idea.

Sep 11 2019, 11:06 AM · Restricted Project, Restricted Project
Hahnfeld accepted D67413: [Clang][Bundler] Replace std::vector by SmallVector [NFC].

LGTM

Sep 11 2019, 8:02 AM · Restricted Project
Hahnfeld accepted D67416: [Clang][Bundler] Fix for a potential memory leak [NFC].

LG iff the following is correct: The leak is when dyn_cast can't cast and returns null, right? If so, please add into the summary (and do so for future revisions, it will speed up review).

Sep 11 2019, 8:02 AM · Restricted Project

Sep 10 2019

Hahnfeld added a comment to D45890: [OMPT] Add implementation and tests of Archer tool.

The code itself looks good to me (it should probably be clang-formated). If others have comments, please raise them now.

Sep 10 2019, 10:53 AM · Restricted Project

Sep 8 2019

Hahnfeld committed rG307daa71a8f0: [ASan] Only run dlopen-mixed-c-cxx.c with static runtime (authored by Hahnfeld).
[ASan] Only run dlopen-mixed-c-cxx.c with static runtime
Sep 8 2019, 9:11 AM
Hahnfeld committed rL371336: [ASan] Only run dlopen-mixed-c-cxx.c with static runtime.
[ASan] Only run dlopen-mixed-c-cxx.c with static runtime
Sep 8 2019, 9:11 AM
Hahnfeld closed D67298: [ASan] Only run dlopen-mixed-c-cxx.c with static runtime.
Sep 8 2019, 9:11 AM · Restricted Project, Restricted Project
Hahnfeld added a comment to D67298: [ASan] Only run dlopen-mixed-c-cxx.c with static runtime.

Successfully tested on my box! I'd just use

diff
 - UNSUPPORTED: asan-dynamic-runtime
 +REQUIRES: x86_64-target-arch && !android && !asan-dynamic-runtime

if that works on your side too.

Sep 8 2019, 4:37 AM · Restricted Project, Restricted Project
Hahnfeld added a comment to D67319: Fix dlopen-mixed-c-cxx test case when libstdc++ is not available.

For the record, I don't think this would have worked: I do have libstdc++ available on my system (I just choose SANITIZER_CXX_ABI=libcxxabi for other reasons) and passing -stdlib=libstdc++ when building the test doesn't help as I've already said in D67298. Moreover, there's nothing in the test that requires libstdc++, it just happens to pass due to many chained implications, and restricting a test without understanding the root cause is probably no good idea.

Sep 8 2019, 4:34 AM · Restricted Project, Restricted Project

Sep 7 2019

Hahnfeld updated the summary of D67298: [ASan] Only run dlopen-mixed-c-cxx.c with static runtime.
Sep 7 2019, 6:24 AM · Restricted Project, Restricted Project
Hahnfeld added a comment to D67298: [ASan] Only run dlopen-mixed-c-cxx.c with static runtime.

And that's what I don't understand

My fault for not being clear enough. Let me elaborate the original faulty use case:

You've got an application written in C, compiled with asan, that dlopens a library written in C++, linked to libstdc++
When the application starts, asan installs a few handler based on the loaded symbols. It used to install a handler that points to itself for __cxa_throw because the symbol is not present in the binary.

Sep 7 2019, 6:06 AM · Restricted Project, Restricted Project
Hahnfeld updated the diff for D67298: [ASan] Only run dlopen-mixed-c-cxx.c with static runtime.

I think I just figured out that the original problem is about static runtime, so it doesn't make sense to run the test with dynamic asan.

Sep 7 2019, 3:36 AM · Restricted Project, Restricted Project
Hahnfeld added a comment to D67298: [ASan] Only run dlopen-mixed-c-cxx.c with static runtime.

Mmmmh the whole point of the test is to test how asan behaves when meeting a symbol like __cxa_throw in a dlopened library when the calling compilation unit is not linked against libstdc++. Previous behavior was a segfault, we should now get a reasonable error.

Sep 7 2019, 2:58 AM · Restricted Project, Restricted Project
Hahnfeld added a comment to D67298: [ASan] Only run dlopen-mixed-c-cxx.c with static runtime.

Mmmmh the whole point of the test is to test how asan behaves when meeting a symbol like __cxa_throw in a dlopened library when the calling compilation unit is not linked against libstdc++. Previous behavior was a segfault, we should now get a reasonable error. By defining __cxa_throw in the c file, you remove the origin of the bug, so sure it works, but it no longer tests the faulty situation. Maybe there's a symbol caught by asan and defined in both libcxx and libstdc++?

Sep 7 2019, 1:08 AM · Restricted Project, Restricted Project

Sep 6 2019

Hahnfeld added a comment to D63877: Avoid infinite loop with asan interception.

I've posted a change that makes the test work for me in D67298, please take a look.

Sep 6 2019, 12:25 PM · Restricted Project, Restricted Project
Hahnfeld created D67298: [ASan] Only run dlopen-mixed-c-cxx.c with static runtime.
Sep 6 2019, 12:24 PM · Restricted Project, Restricted Project

Sep 4 2019

Hahnfeld added inline comments to D67031: [Clang][Bundler] Error reporting improvements.
Sep 4 2019, 12:34 PM · Restricted Project
Hahnfeld added a comment to D67031: [Clang][Bundler] Error reporting improvements.

Also, there should be a summary of the changes in here.

Sep 4 2019, 11:29 AM · Restricted Project
Hahnfeld added inline comments to D67031: [Clang][Bundler] Error reporting improvements.
Sep 4 2019, 11:13 AM · Restricted Project
Hahnfeld committed rG673e5476a817: [OpenMP] Change initialization of __kmp_global (authored by Hahnfeld).
[OpenMP] Change initialization of __kmp_global
Sep 4 2019, 10:53 AM
Hahnfeld committed rL370943: [OpenMP] Change initialization of __kmp_global.
[OpenMP] Change initialization of __kmp_global
Sep 4 2019, 10:47 AM
Hahnfeld closed D66292: [OpenMP] Change initialization of __kmp_global.
Sep 4 2019, 10:46 AM · Restricted Project, Restricted Project
Hahnfeld added a comment to D63877: Avoid infinite loop with asan interception.

Is that summary correct?

It is :-)

Sep 4 2019, 12:30 AM · Restricted Project, Restricted Project

Sep 3 2019

Hahnfeld added a comment to D63877: Avoid infinite loop with asan interception.

@Hahnfeld : I understand your concern. However this test case is meant to test handling of stdlib exceptions, so it doesn't look strange to disable it if stdlib is not used, right? [...]

Sep 3 2019, 1:34 PM · Restricted Project, Restricted Project

Sep 2 2019

Hahnfeld committed rL370683: Request commit access for hahnfeld.
Request commit access for hahnfeld
Sep 2 2019, 1:17 PM

Aug 31 2019

Hahnfeld added a comment to D67031: [Clang][Bundler] Error reporting improvements.

This changes error messages, so I'd say it's not NFC.

Aug 31 2019, 7:57 AM · Restricted Project

Aug 30 2019

Hahnfeld added a comment to D63877: Avoid infinite loop with asan interception.

@Hahnfeld : I failed to reproduce the issue locally, but I assume the following patch may solve your issue:

--- a/compiler-rt/test/asan/TestCases/Linux/dlopen-mixed-c-cxx.c
+++ b/compiler-rt/test/asan/TestCases/Linux/dlopen-mixed-c-cxx.c
@@ -5,7 +5,7 @@
 //
 // CHECK: {{.*}}AddressSanitizer: failed to intercept '__cxa_{{.*}}throw{{.*}}'
 //
-// REQUIRES: x86_64-target-arch && !android
+// REQUIRES: x86_64-target-arch && !android && !cxxabi

can you give it a try?

Aug 30 2019, 8:46 AM · Restricted Project, Restricted Project

Aug 15 2019

Hahnfeld accepted D65819: [Driver][Bundler] Improve bundling of object files..

LG, thanks for the changes.

Aug 15 2019, 9:53 AM · Restricted Project, Restricted Project
Hahnfeld accepted D66296: [BUNDLER]Improve the test, NFC..

LG

Aug 15 2019, 8:30 AM · Restricted Project, Restricted Project
Hahnfeld added a comment to D65819: [Driver][Bundler] Improve bundling of object files..

Please submit the test changes unrelated to the code changes in a separate patch!

Aug 15 2019, 8:01 AM · Restricted Project, Restricted Project
Hahnfeld added inline comments to D66292: [OpenMP] Change initialization of __kmp_global.
Aug 15 2019, 7:16 AM · Restricted Project, Restricted Project
Hahnfeld created D66292: [OpenMP] Change initialization of __kmp_global.
Aug 15 2019, 7:13 AM · Restricted Project, Restricted Project
Hahnfeld added a comment to D65868: [OpenMP] Remove workaround for CMPXCHG.

Unfortunately, the Windows atomic code will not compile with this change. See in kmp_atomic.h the different atomic types for Windows. The complaints are regarding assignment operations for the complex types.

../../src/kmp_atomic.cpp(1325): error: class "__kmp_cmplx32_t" has no suitable assignment operator
  ATOMIC_CMPXCHG(cmplx4, add, kmp_cmplx32, 64, +, 8c, 7,
  ^
Aug 15 2019, 7:07 AM · Restricted Project
Hahnfeld requested changes to D65819: [Driver][Bundler] Improve bundling of object files..

The code changes look good to me, but the test doesn't pass on x86. We've faced the same problem when clang-offload-bundler was initially committed and the current testing is the best we were able to do.

Aug 15 2019, 6:54 AM · Restricted Project, Restricted Project
Hahnfeld removed a reviewer for D55892: [OpenMP] 'close' map-type-modifier code generation: Hahnfeld.

Can we close this after D65341 landed?

Aug 15 2019, 6:30 AM · Restricted Project, Restricted Project
Hahnfeld committed rGd2ae0c4f443c: [OpenMP] Enable warning about "implicit fallthrough" (authored by Hahnfeld).
[OpenMP] Enable warning about "implicit fallthrough"
Aug 15 2019, 6:28 AM
Hahnfeld committed rG4d77e50e6ede: [OpenMP] Remove 'unnecessary parentheses' (authored by Hahnfeld).
[OpenMP] Remove 'unnecessary parentheses'
Aug 15 2019, 6:27 AM
Hahnfeld committed rGfb72a03f85dc: [OMPT] Resolve warnings because of ints in if conditions (authored by Hahnfeld).
[OMPT] Resolve warnings because of ints in if conditions
Aug 15 2019, 6:26 AM
Hahnfeld committed rL369003: [OpenMP] Enable warning about "implicit fallthrough".
[OpenMP] Enable warning about "implicit fallthrough"
Aug 15 2019, 6:26 AM
Hahnfeld closed D65871: [OpenMP] Enable warning about "implicit fallthrough".
Aug 15 2019, 6:26 AM · Restricted Project, Restricted Project
Hahnfeld committed rL369002: [OpenMP] Remove 'unnecessary parentheses'.
[OpenMP] Remove 'unnecessary parentheses'
Aug 15 2019, 6:26 AM
Hahnfeld closed D65870: [OpenMP] Remove 'unnecessary parentheses'.
Aug 15 2019, 6:26 AM · Restricted Project, Restricted Project
Hahnfeld committed rL369001: [OMPT] Resolve warnings because of ints in if conditions.
[OMPT] Resolve warnings because of ints in if conditions
Aug 15 2019, 6:26 AM
Hahnfeld closed D65869: [OMPT] Resolve warnings because of ints in if conditions.
Aug 15 2019, 6:25 AM · Restricted Project, Restricted Project
Hahnfeld committed rGdc23c832f4f7: [OpenMP] Turn on -Wall compiler warnings by default (authored by Hahnfeld).
[OpenMP] Turn on -Wall compiler warnings by default
Aug 15 2019, 6:12 AM
Hahnfeld committed rL368999: [OpenMP] Turn on -Wall compiler warnings by default.
[OpenMP] Turn on -Wall compiler warnings by default
Aug 15 2019, 6:12 AM
Hahnfeld closed D65867: [RFC] [OpenMP] Turn on -Wall compiler warnings by default.
Aug 15 2019, 6:12 AM · Restricted Project, Restricted Project

Aug 14 2019

Hahnfeld added a comment to D65819: [Driver][Bundler] Improve bundling of object files..

Okay, so I wasn't happy with the current explanations because I don't like "fixing" an issue without understanding the problem. Here's a small reproducer:

$  cat main.cpp 
#include "test.h"
Aug 14 2019, 2:47 AM · Restricted Project, Restricted Project
Hahnfeld added a comment to D65836: Factor architecture dependent code out of loop.cu.

Jon, please subscribe to openmp-commits so that commit emails get through immediately. Thanks!

Aug 14 2019, 1:08 AM · Restricted Project, Restricted Project

Aug 13 2019

Hahnfeld added a comment to D65819: [Driver][Bundler] Improve bundling of object files..

Will this patch change the ability to consume a bundled object file without calling the unbundler? Using known ELF tools on the produced object files was an important design decision and IIRC was somewhat important for using build systems that are unaware of the bundled format.

Ping.

Missed this. We still produce correct object files, so all the tools will work with this.

I agree on a technical level that it's still a "correct" object, but not what I was looking for: The host object file will only be in the bundled section, so you cannot examine it without unbundling.

For example, with a small test file and clang -fopenmp -fopenmp-targets=x86_64 -c test.c I saw the following:

$ nm test.o                                       
0000000000000000 t .omp_offloading.requires_reg
0000000000000000 T test
                 U __tgt_register_requires

After applying this patch, the output is empty which might be a problem in certain cases.

Unfortunately, this is the only possible solution I see. There are already 2 reports that bundled objects does not work correctly after unbundling.

Can you please again share what exactly is the problem, with a small example? I saw discussions on openmp-dev, but that project was huge, and above you were quoting a man page and hinted to global constructors.

I don't have a small reproducer, unfortunately, only the big one.
Here is the message from the user:

Hi,
I am revisiting this possible compiler bug in Clang 8.0.0.

In the source code I am developing, there's a global static variable, nest::sli_neuron::recordablesMap_ put in the BSS section and it is expected to be fully initialized by the time nest::sli_neuron::sli_neuron() gets called, however in a gdb session:

(gdb) p nest::sli_neuron::recordablesMap_
Python Exception <type 'exceptions.AttributeError'> 'gdb.Type' object has no attribute 'name':
$1 = {<std::map<Name, double (nest::sli_neuron::*)() const, std::less<Name>, std::allocator<std::pair<Name const, double (nest::sli_neuron::*)() const> > >> = std::map with 0 elements, _vptr$RecordablesMap = 0x0}

this doesn't happen when -fopenmp-targets is _not_ used, is it not trivial to come up a reproducer, thus I am sending a work-in-progress report hoping someone will shed some light on this.

Thanks,
Itaru.

Another one:

Hi,
I am seeing a link error shown below:

`.text.startup' referenced in section `.init_array.0' of /tmp/event_delivery_manager-02f392.o: defined in discarded section `.text.startup[_ZN4nest18DataSecondaryEventIdNS_16GapJunctionEventEE18supported_syn_ids_E]' of /tmp/event_delivery_manager-02f392.o
clang-10: error: linker command failed with exit code 1 (use -v to see invocation)

I am not sure how to tackle this as the part is referenced isn't what I am
working on. I am using the latest trunk Clang 10 at this moment.

Steps to reproduce:

Steps to produce:
$ git clone https://https://github.com/nest/nest-simulator/
$ mkdir /tmp/build/nest-clang-offload/
$ cd /tmp/build/nest-clang-offload/
$ cmake -DCMAKE_TOOLCHAIN_FILE=Platform/JURON_Clang -DCMAKE_INSTALL_PREFIX=/path/to/opt/nest-clang -Dwith-gsl=ON -Dwith-openmp=ON -Dwith-mpi=OFF -Dwith-python=OFF -Dwith-offload=ON /path/to/nest-simulator/
Aug 13 2019, 12:32 PM · Restricted Project, Restricted Project
Hahnfeld added a comment to D65819: [Driver][Bundler] Improve bundling of object files..

Will this patch change the ability to consume a bundled object file without calling the unbundler? Using known ELF tools on the produced object files was an important design decision and IIRC was somewhat important for using build systems that are unaware of the bundled format.

Ping.

Missed this. We still produce correct object files, so all the tools will work with this.

I agree on a technical level that it's still a "correct" object, but not what I was looking for: The host object file will only be in the bundled section, so you cannot examine it without unbundling.

For example, with a small test file and clang -fopenmp -fopenmp-targets=x86_64 -c test.c I saw the following:

$ nm test.o                                       
0000000000000000 t .omp_offloading.requires_reg
0000000000000000 T test
                 U __tgt_register_requires

After applying this patch, the output is empty which might be a problem in certain cases.

Unfortunately, this is the only possible solution I see. There are already 2 reports that bundled objects does not work correctly after unbundling.

Aug 13 2019, 11:52 AM · Restricted Project, Restricted Project
Hahnfeld added inline comments to D65819: [Driver][Bundler] Improve bundling of object files..
Aug 13 2019, 11:41 AM · Restricted Project, Restricted Project
Hahnfeld added a comment to D65819: [Driver][Bundler] Improve bundling of object files..

Will this patch change the ability to consume a bundled object file without calling the unbundler? Using known ELF tools on the produced object files was an important design decision and IIRC was somewhat important for using build systems that are unaware of the bundled format.

Ping.

Missed this. We still produce correct object files, so all the tools will work with this.

Aug 13 2019, 11:37 AM · Restricted Project, Restricted Project
Hahnfeld updated the diff for D65871: [OpenMP] Enable warning about "implicit fallthrough".

Rebase.

Aug 13 2019, 11:02 AM · Restricted Project, Restricted Project
Hahnfeld updated the diff for D65867: [RFC] [OpenMP] Turn on -Wall compiler warnings by default.

Change check_c_compiler_flag() to check_cxx_compiler_flag().

Aug 13 2019, 11:00 AM · Restricted Project, Restricted Project