This is an archive of the discontinued LLVM Phabricator instance.

Differential D103222

Fixed bug using string stream by including missing header
ClosedPublic

Authored by Pascal on May 26 2021, 8:46 PM.

Details

Reviewers
jdoerfert
JonChesterfield
Summary

Compilation bug for openmp project and AMDGPU target

Diff Detail

Event Timeline

Pascal created this revision.May 26 2021, 8:46 PM
Herald added subscribers: kerbowa, tpr, nhaehnle, jvesely. · View Herald TranscriptMay 26 2021, 8:46 PM
Pascal requested review of this revision.May 26 2021, 8:46 PM
Herald added a reviewer: jdoerfert. · View Herald TranscriptMay 26 2021, 8:46 PM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added subscribers: openmp-commits, sstefan1. · View Herald Transcript
Pascal added a reviewer: JonChesterfield.May 26 2021, 8:50 PM
Harbormaster completed remote builds in B106427: Diff 348150.May 26 2021, 8:59 PM
JonChesterfield requested changes to this revision.May 27 2021, 2:47 AM

Didn't realize we were using string stream. Probably don't want to, but thanks for the fix in the meantime

This revision now requires changes to proceed.May 27 2021, 2:47 AM
JonChesterfield accepted this revision.May 27 2021, 2:47 AM
This revision is now accepted and ready to land.May 27 2021, 2:47 AM
JonChesterfield added a comment.Jul 14 2021, 8:49 AM

@Pascal should I land this on your behalf?

Pascal added a comment.Jul 14 2021, 5:59 PM

Hi @JonChesterfield, yes go ahead. I am curious as to why clang-tidy reported gelf.h not found. It is just part of the libelf. I assume this diagnostic error has shown up before and has been ignored?

JonChesterfield added a comment.Jul 22 2021, 11:00 AM

Apologies, this change landed as part of D104923 on June 30.

Re: clang-tidy, it doesn't run in the same environment as compilation. This is particularly noticeable for the device runtime, where clang-tidy warns about a lot of unrecognised intrinsics because it makes a wrong assumption about the target architecture. gelf.h may be a problem in practice when this builds by default, though it's also used by generic-elf-64bit so hopefully not. I'm not sure offhand what we're using from gelf.h.

JonChesterfield closed this revision.Jul 22 2021, 11:00 AM

Revision Contents

PathSize
openmp/
libomptarget/
plugins/
amdgpu/
impl/
1 line

Diff 348150

openmp/libomptarget/plugins/amdgpu/impl/system.cpp

/*===-------------------------------------------------------------------------- /*===--------------------------------------------------------------------------
* ATMI (Asynchronous Task and Memory Interface) * ATMI (Asynchronous Task and Memory Interface)
* *
* This file is distributed under the MIT License. See LICENSE.txt for details. * This file is distributed under the MIT License. See LICENSE.txt for details.
*===------------------------------------------------------------------------*/ *===------------------------------------------------------------------------*/
#include <gelf.h> #include <gelf.h>
Lint: Pre-merge checks
Inline Actions

clang-tidy: error: 'gelf.h' file not found [clang-diagnostic-error]
not useful

Lint: Pre-merge checks: clang-tidy: error: 'gelf.h' file not found [clang-diagnostic-error] [[https://github.
#include <libelf.h> #include <libelf.h>
#include <cassert> #include <cassert>
#include <cstdarg> #include <cstdarg>
#include <fstream> #include <fstream>
#include <iomanip> #include <iomanip>
#include <iostream> #include <iostream>
#include <set> #include <set>
#include <sstream>
#include <string> #include <string>
#include "internal.h" #include "internal.h"
#include "machine.h" #include "machine.h"
#include "rt.h" #include "rt.h"
#include "msgpack.h" #include "msgpack.h"
▲ Show 20 Lines • Show All 1,185 Lines • Show Last 20 Lines