ABataev added inline comments.

================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:2958-2970
+    auto PerDevice = OffloadEntriesTargetRegion.find(DeviceID);
+    if (PerDevice != OffloadEntriesTargetRegion.end()) {
+      auto PerFile = PerDevice->second.find(FileID);
+      if (PerFile != PerDevice->second.end()) {
+        auto PerParentName = PerFile->second.find(ParentName);
+        if (PerParentName != PerFile->second.end()) {
+          auto PerLine = PerParentName->second.find(LineNum);
----------------
cchen wrote:
> cchen wrote:
> > ABataev wrote:
> > > ABataev wrote:
> > > > Just:
> > > > ```
> > > > if (hasTargetRegionEntryInfo(DeviceID, FileID, ParentName, LineNum))
> > > >   return;
> > > > ```
> > > Actually, even better to do something like this:
> > > ```
> > > if (Flags == 
> > > OffloadEntriesInfoManagerTy::OMPTargetRegionEntryTargetRegion && 
> > > hasTargetRegionEntryInfo(DeviceID, FileID, ParentName, LineNum))
> > >   return;
> > > assert(!hasTargetRegionEntryInfo(DeviceID, FileID, ParentName, LineNum) 
> > > && "Target region entry already registered!");
> > > ```
> > I tried this code but found that `hasTargetRegionEntryInfo` is not doing 
> > what we want since we need the below code to return true in 
> > `hasTargetRegionEntryInfo`.
> > ```
> > if (PerLine->second.getAddress() || PerLine->second.getID()) {
> >   return false;
> > }
> > ```
> Do we want to create a new helper function like hasTargetRegionEntryInfo or 
> something?
Better to extend the old one, I think. Add a new parameter to check if we need 
to check for address and id


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90704/new/

https://reviews.llvm.org/D90704

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to