JonChesterfield added inline comments.
================
Comment at: llvm/include/llvm/IR/OpenMPKinds.def:186
+///{
+
+#ifndef OMP_IDENT_FLAG
----------------
jdoerfert wrote:
> JonChesterfield wrote:
> > Meinersbur wrote:
> > > JonChesterfield wrote:
> > > > jdoerfert wrote:
> > > > > Meinersbur wrote:
> > > > > > jdoerfert wrote:
> > > > > > > JonChesterfield wrote:
> > > > > > > > Sharing constants between the compiler and the runtime is an
> > > > > > > > interesting subproblem. I think the cleanest solution is the
> > > > > > > > one used by libc, where the compiler generates header files
> > > > > > > > containing the constants which the runtime library includes.
> > > > > > > I'd prefer not to tackle this right now but get this done first
> > > > > > > and revisit the issue later. OK?
> > > > > > I don't think this is a good solution. It means that libomp cannot
> > > > > > built built anymore without also building clang. Moreover, the
> > > > > > values cannot be changed anyway since it would break the ABI.
> > > > > >
> > > > > > I'd go the other route: The libomp defines what it's ABI is, the
> > > > > > compiler generates code for it.
> > > > > This patch doesn't change what we do, just where. The numbers are
> > > > > hard coded in clang now. Let's start a discussion on the list and if
> > > > > we come up with a different scheme we do it after this landed.
> > > > Revisit later sounds good.
> > > >
> > > > @Meinersbur Do you know of an example of a non-llvm compiler using this
> > > > libomp?
> > > >
> > > > The usual order is build a compiler, then use it to build the runtime
> > > > libraries, then the whole package can build other stuff. Provided the
> > > > compiler doesn't need any of the runtime libraries (compiler-rt, maths
> > > > libraries, libomp etc) itself the system bootstraps cleanly. Especially
> > > > important when cross compiling and I suspect the gpu targets in openmp
> > > > have similarly strict requirements on the first compiler.
> > > >
> > > > Closely related to that, I tend to assume that the runtime libraries
> > > > can be rewritten to best serve their only client - the associated
> > > > compiler - so if libomp is used by out of tree compilers I'd like to
> > > > know who we are at risk of breaking.
> > > > Do you know of an example of a non-llvm compiler using this libomp?
> > >
> > > [[
> > > https://github.com/llvm-project/llvm-project/blob/master/openmp/runtime/src/kmp_gsupport.cpp
> > > | gcc ]] (using libomp's gomp compatibility layer), [[
> > > https://www.openmprtl.org/ | icc ]] (as libomp was initially donated by
> > > Intel).
> > >
> > > I don't understand why it even matters if there are other compilers using
> > > libomp. Every LLVM runtime library can be built stand-alone.
> > > With constant values being determined during compiler bootstrapping,
> > > programs built on one computer would be potentially ABI-incompatible with
> > > a runtime library on another. Think about updating your
> > > compiler-rt/libomp/libc++ on you computer causing all existing binaries
> > > on the system to crash because constants changed in the updated
> > > compiler's bootstrapping process.
> > >
> > > The only use case I know that does this is are operating system's syscall
> > > tables. Linux's reference is [[
> > > https://github.com/torvalds/linux/blob/master/arch/sh/include/uapi/asm/unistd_64.h
> > > | unistd.h ]] which is platform-specific and Windows generates the table
> > > during its [[ https://j00ru.vexillium.org/syscalls/nt/64/ | build process
> > > ]]. Therefore on Windows, system calls can only be done through ntdll.
> > > Even on Linux one should use the system's libc instead of directly
> > > invoking a system call.
> > Thanks. GCC and ICC would presumably be happier with the magic numbers
> > stored with openmp then (though with the move to a monorepo that's a little
> > less persuasive).
> >
> > When constants that affect the ABI change, the result won't work with
> > existing software regardless of whether the compiler or the library
> > contains the change. Either the new compiler builds things that don't work
> > with the old library, or the new library doesn't work with things built by
> > the old compiler. The two have to agree on the ABI.
> >
> > At present, openmp does the moral equivalent of #include OpenMPKinds.def
> > from clang. Moving the constants to libomp means clang will do the
> > equivalent of #include OpenMPKinds.def from openmp. Breaking that
> > dependency means making a new subproject that just holds/generates the
> > constants, that both depend on, which seems more hassle than it's worth.
> >
> > I'd like to generate this header as part of the clang build (though
> > ultimately don't care that much if it's generated as part of the openmp
> > build) because it's going to become increasingly challenging to read as
> > non-nvptx architectures are introduced. Likewise it would be useful to
> > generate the interface.h for deviceRTL (or equivalently a set of unit tests
> > checking the function types) from the same source to ensure it matches and
> > that's not economically feasible within the C preprocessor.
> I am unsure how this conversation evolved and what you want me to do now.
>
> I repeat what I said before:
>
> This does neither change the constants, our usage of them, nor the fact that
> we have them defined in multiple places, just one of the places (now llvm,
> before clang) changed.
>
>
Apologies for the digression. There is no change to the status quo so
considering how to improve matters later cannot be blocking.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69785/new/
https://reviews.llvm.org/D69785
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
