RWTH Aachen University
- User Since
- Apr 2 2015, 4:52 AM (237 w, 6 d)
Sat, Sep 28
Sep 11 2019
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.
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 10 2019
The code itself looks good to me (it should probably be clang-formated). If others have comments, please raise them now.
Sep 8 2019
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 7 2019
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 6 2019
I've posted a change that makes the test work for me in D67298, please take a look.
Sep 4 2019
Also, there should be a summary of the changes in here.
Sep 3 2019
Sep 2 2019
Aug 31 2019
This changes error messages, so I'd say it's not NFC.
Aug 30 2019
Aug 15 2019
LG, thanks for the changes.
Please submit the test changes unrelated to the code changes in a separate patch!
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.
Can we close this after D65341 landed?
Aug 14 2019
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"
Jon, please subscribe to openmp-commits so that commit emails get through immediately. Thanks!
Aug 13 2019
Change check_c_compiler_flag() to check_cxx_compiler_flag().
Aug 12 2019
Aug 11 2019
Remove compiler check for -Wno-uninitialized.
Remove another unnecessary cast.
Aug 10 2019
This test is also failing for me on CentOS 7, Intel x86. Any clues what's wrong here?
Aug 9 2019
LG after addressing the last minor nits (comment + cast + check).
Can you please rebase on top of D66019? Sorry, that'll probably give you some conflicts :-/
Aug 7 2019
Thanks, that's even easier than a pointer to NULL :)
Aug 6 2019
Can you detail what "incorrect linking" means? AFAIK the additional sections were just bloating the executable, but how do they affect correctness?
The test will currently fail with older versions of Clang. It must at least be marked UNSUPPORTED for Clang versions older than what-will-be Clang 10.
Aug 3 2019
Apologies, but I think my fix for -Wcast-qual is wrong and the code is still broken. Luckily, the latest Intel Compiler still warns about it which forced me to rethink what the pointers try to achieve.
Aug 2 2019
I'd propose to mark the test close_modifier.c as unsupported with older compiler versions. Additionally, can you add a small, new test with manual calls to __tgt_target_data_begin / _end without relying on the compiler?
Oh, but the tests again look add. Please run them through clang-format.
Aug 1 2019
I'm fine with the last update.
I think this still needs some work.
There's still no call to __tgt_register_requires in the two tests, so I guess they won't pass with older versions of Clang.