[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

2022-11-03 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1262-1265 +if (identify_magic((*BufferOrErr)->getBuffer()) == +file_magic::elf_shared_object) + continue; + tra wrote: > jhenderson wrote: > > This

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

2022-11-03 Thread Joseph Huber via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3384f05a2cdb: [llvm-objdump][Offload] Use common offload extraction method (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

2022-11-03 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. LGTM, modulo separating clang-related change into a separate patch. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1262-1265 +if (identify_magic((*BufferO

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

2022-11-03 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment. In D136796#3904512 , @jhenderson wrote: > In D136796#3903393 , @jhuber6 wrote: > >> Is this good to land now? > > The LLVM community practice is to wait a week between pings unless there'

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

2022-11-03 Thread James Henderson via Phabricator via cfe-commits
jhenderson added a comment. In D136796#3903393 , @jhuber6 wrote: > Is this good to land now? The LLVM community practice is to wait a week between pings unless there's something urgent (and if something is urgent, please say so). Aside from the clang b

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

2022-11-02 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment. Is this good to land now? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136796/new/ https://reviews.llvm.org/D136796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

2022-11-01 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 472265. jhuber6 added a comment. Addressing comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136796/new/ https://reviews.llvm.org/D136796 Files: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

2022-11-01 Thread James Henderson via Phabricator via cfe-commits
jhenderson added inline comments. Comment at: llvm/test/tools/llvm-objdump/Offloading/elf_dynamic.test:1 -## Check that we can dump an offloading binary directly. -# RUN: yaml2obj %S/Inputs/binary.yaml -o %t.bin -# RUN: llvm-objdump --offloading %t.bin | FileCheck %s --match-full

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

2022-10-27 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 471132. jhuber6 added a comment. Adding separate tests for `ET_REL`, `ET_EXEC`, and `ET_DYN`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136796/new/ https://reviews.llvm.org/D136796 Files: clang/tools/cla

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

2022-10-27 Thread James Henderson via Phabricator via cfe-commits
jhenderson added a comment. As a general rule, dumping tools should avoid hard errors, because usually they prevent the dumping tool from continuing to dump other useful and valid information as part of the same execution. I don't have a strong objection in this particular case, because I presu

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

2022-10-26 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 created this revision. jhuber6 added reviewers: JonChesterfield, tra, yaxunl, jdoerfert, tianshilei1992. Herald added a subscriber: hiraditya. Herald added a reviewer: alexander-shaposhnikov. Herald added a reviewer: jhenderson. Herald added a reviewer: MaskRay. Herald added a project: All