jdoerfert 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);
----------------
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.
FWIW, I can also use ifdef, or various other context selectors to achieve the 
same effect.


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

Reply via email to