tianshilei1992 marked 6 inline comments as done. tianshilei1992 added inline comments.
================ Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:2944-2947 + // This could happen if the device compilation is invoked standalone. + if (!hasTargetRegionEntryInfo(DeviceID, FileID, ParentName, LineNum)) + initializeTargetRegionEntryInfo(DeviceID, FileID, ParentName, LineNum, + OffloadingEntriesNum); ---------------- ABataev wrote: > jdoerfert wrote: > > ABataev wrote: > > > tianshilei1992 wrote: > > > > ABataev wrote: > > > > > jdoerfert wrote: > > > > > > tianshilei1992 wrote: > > > > > > > ABataev wrote: > > > > > > > > jdoerfert wrote: > > > > > > > > > ABataev wrote: > > > > > > > > > > I would add a chack that to auxiliary device was specified. > > > > > > > > > > And if it was specified, it means this is not device-only > > > > > > > > > > mode and still need to emit an error. > > > > > > > > > No it doesn't. There is nothing wrong with > > > > > > > > > https://godbolt.org/z/T1h9b5, and as I said before, I can > > > > > > > > > build the situation in various other ways as well, some of > > > > > > > > > which will be outside of the users control. A global can > > > > > > > > > exist in the host/device code only. > > > > > > > > I'm not saying that this is wrong. This code was used to check > > > > > > > > that the compiler works correctly and it just allows developer > > > > > > > > to understand that there is a problem with the compiler if it > > > > > > > > misses something and there is a difference between host and > > > > > > > > device codegens. If we don't want to emit an error here, still > > > > > > > > would be good to have something like an assert to be sure that > > > > > > > > the host/device codegens are synced. > > > > > > > That check still doesn't work for the test case provided by > > > > > > > @jdoerfert because host IR doesn't contain that global in the > > > > > > > offload info. > > > > > > As @tianshilei1992 says, my test case does show how this can never > > > > > > be an assertion/warning even for regular host+device compliation. > > > > > > There is no guarantee a host version exists, or a device one does. > > > > > > We need to gracefully allow either. > > > > > So, this is caused by the `nohost` variant selector, right? In this > > > > > case we don't emit it for the host and don't have corresponding entry > > > > > in the metadata? > > > > Correct. > > > Ok, now I see. Is it possible to distinguish this corner case from the > > > general one, where the variable is available for both, host and device, > > > or it will require some extra work/design? If it is easy to differentiate > > > these 2 cases, it would be good to keep something like an error > > > message/assert for the general case and remove the restriction for this > > > corner case. Otherwise, proceed with the current solution. > > FWIW, I can also use ifdef, or various other context selectors to achieve > > the same effect. > Ok, then go ahead with this solution. I'm thinking we can probably set a new option to invoke device only compilation, like `--cuda-device-only` we already had for CUDA. However, the thing is, unlike CUDA, we cannot tell which parts are kernels implicitly w/o host compilation. So if we provide the option, we might end up with only recognizing code in `declare target` or all its variant. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94871/new/ https://reviews.llvm.org/D94871 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
