[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2172748 , @protze.joachim wrote: > I carefully made sure, that the freshly built clang was used to execute the > test. I opened https://bugs.llvm.org/show_bug.cgi?id=46836 to track the issue > and made it release

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

protze.joachim reopened this revision. protze.joachim added a comment. This revision is now accepted and ready to land. I carefully made sure, that the freshly built clang was used to execute the test. I opened https://bugs.llvm.org/show_bug.cgi?id=46836 to track the issue and made it release

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

protze.joachim added a comment. This patch breaks compilation of previously working code. I added the following to `openmp/libomptarget/test/offloading/offloading_success.c`: +// RUN: %libomptarget-compile-run-and-check-nvptx64-nvidia-cuda which results in # command stderr: ptxas

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

This revision was automatically updated to reflect the committed changes. Closed by commit rGc98699582a63: [OpenMP][NFC] Remove unused (always fixed) arguments (authored by jdoerfert). Changed prior to commit: https://reviews.llvm.org/D83268?vs=275871=277218#toc Repository: rG LLVM Github

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

JonChesterfield added a comment. __kmpc_spmd_kernel_init is always called with RequiresDataSharing == 0 Specifically, it's only called from clang, and emitSPMDEntryHeader unconditionally passes zero to it I.e. I think there's more stuff that can be cleaned up in the theme of the above, suggest

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2136060 , @ABataev wrote: > Better to ask the users. Maybe, send an RFC to openmp-devs? Sure: http://lists.llvm.org/pipermail/openmp-dev/2020-July/003531.html Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

Hahnfeld added a comment. In D83268#2135930 , @JonChesterfield wrote: > Aside from the API stability concern this looks uncontentious. Removes dead > arguments, generally makes things simpler. Thus LGTM. > > @Hahnfeld @ABataev - are you sufficiently

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2136055 , @ABataev wrote: > In D83268#2136054 , @jdoerfert wrote: > > > In D83268#2136029 , @ABataev wrote: > > > > >

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

ABataev added a comment. In D83268#2136049 , @jdoerfert wrote: > In D83268#2136031 , @ABataev wrote: > > > There is still compatibility between clang10 and clang11. Or they are > > incompatible in LLVM IR level? >

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

ABataev added a comment. In D83268#2136054 , @jdoerfert wrote: > In D83268#2136029 , @ABataev wrote: > > > `llvm-project/openmp/libomptarget` > > > Please use more words. libomptarget is part of libomp

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2136029 , @ABataev wrote: > `llvm-project/openmp/libomptarget` Please use more words. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83268/new/

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2136031 , @ABataev wrote: > There is still compatibility between clang10 and clang11. Or they are > incompatible in LLVM IR level? That is the point. They might or might not be, right? There is no guarantee they

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

ABataev added a comment. In D83268#2135951 , @jdoerfert wrote: > > I don't think gcc can be using this runtime library for nvptx. > > Yes, and: We are (going to) use clang specific intrinsics to avoid CUDA > (soon). > > > Use of the new library with the

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

ABataev added a comment. In D83268#2136021 , @jdoerfert wrote: > In D83268#2135988 , @ABataev wrote: > > > In D83268#2135955 , @jdoerfert > > wrote: > > > > > > Especially

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2135988 , @ABataev wrote: > In D83268#2135955 , @jdoerfert wrote: > > > > Especially taking into account, that libomp can be built separately. > > > > This is *not* affecting

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2135989 , @ABataev wrote: > In D83268#2135954 , @JonChesterfield > wrote: > > > What can libomp be built by separately? Nvcc and gcc don't use this > > runtime. That leaves us

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

ABataev added a comment. In D83268#2135954 , @JonChesterfield wrote: > In D83268#2135938 , @ABataev wrote: > > > > @Hahnfeld @ABataev - are you sufficiently persuaded that preserving the > > > current interface

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

ABataev added a comment. In D83268#2135955 , @jdoerfert wrote: > > Especially taking into account, that libomp can be built separately. > > This is *not* affecting libomp in any way. libomptarget and device runtimes are part of libomp. If you're going

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. > Especially taking into account, that libomp can be built separately. This is *not* affecting libomp in any way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83268/new/ https://reviews.llvm.org/D83268

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

JonChesterfield added a comment. In D83268#2135938 , @ABataev wrote: > > @Hahnfeld @ABataev - are you sufficiently persuaded that preserving the > > current interface is not worth the development cost? > > No, I'm not. Long before that, we relied on the

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert marked an inline comment as done. jdoerfert added a comment. > I don't think gcc can be using this runtime library for nvptx. Yes, and: We are (going to) use clang specific intrinsics to avoid CUDA (soon). > Use of the new library with the previous version of the compiler. Except

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

ABataev added a comment. In D83268#2135930 , @JonChesterfield wrote: > Aside from the API stability concern this looks uncontentious. Removes dead > arguments, generally makes things simpler. Thus LGTM. > > @Hahnfeld @ABataev - are you sufficiently

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. Aside from the API stability concern this looks uncontentious. Removes dead arguments, generally makes things simpler. Thus LGTM. @Hahnfeld @ABataev - are you sufficiently

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

JonChesterfield added a comment. I'm not sure we have a consensus on api stability. Usually llvm allows mixing libraries and compilers from different sources, e.g. the various libunwind or compiler-rt vs libgcc. Libomptarget in general appears to be considered fixed and has external users

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

ABataev added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:42 - /// *outlined_function, int16_t - /// IsOMPRuntimeInitialized); OMPRTL_NVPTX__kmpc_kernel_prepare_parallel, I think, instead the optimizer can try to detect if the

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

ABataev added a comment. In D83268#2135724 , @jdoerfert wrote: > In D83268#2135081 , @Hahnfeld wrote: > > > This is definitely not NFC and breaks API compatibility (but apparently > > nobody cares anymore?). > > >

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2135081 , @Hahnfeld wrote: > This is definitely not NFC and breaks API compatibility (but apparently > nobody cares anymore?). This is the device RTL. I am not aware we (want to) keep the API stable. If we are, I'm

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

ABataev added a comment. In D83268#2135081 , @Hahnfeld wrote: > This is definitely not NFC and breaks API compatibility (but apparently > nobody cares anymore?). +1. Better to introduce new entry points and mark these ones as deprecated. Repository:

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

Hahnfeld added a comment. This is definitely not NFC and breaks ABI compatibility (but apparently nobody cares anymore). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83268/new/ https://reviews.llvm.org/D83268

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert created this revision. jdoerfert added reviewers: jhuber6, fghanim, JonChesterfield, grokos, AndreyChurbanov, ye-luo, tianshilei1992, ggeorgakoudis. Herald added subscribers: llvm-commits, cfe-commits, sstefan1, guansong, bollu, yaxunl, jholewinski. Herald added projects: clang,