lildmh marked an inline comment as done.
lildmh added inline comments.

================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8981-8982
+  // Convert the size in bytes into the number of array elements.
+  Size = MapperCGF.Builder.CreateExactUDiv(
+      Size, MapperCGF.Builder.getInt64(ElementSize.getQuantity()));
   llvm::Value *PtrBegin = MapperCGF.Builder.CreateBitCast(
----------------
ABataev wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > lildmh wrote:
> > > > > ABataev wrote:
> > > > > > So, we're still going to use number of elements for mappers? And 
> > > > > > pass it in the same parameter that in other cases is used as size 
> > > > > > in bytes? If so, point to it explicitly in the review for the 
> > > > > > runtime part so all are informed about it.
> > > > > From interface, the mapper function uses size in bytes now. Inside, 
> > > > > it needs number of elements to iterate through all elements. This has 
> > > > > no impact on the runtime part, since it looks like normal mapping 
> > > > > from the interface. All conversion happens inside the mapper function 
> > > > > which is completely generated by the compiler.
> > > > Ok. Then why do we need to convert size in bytes to number of elements 
> > > > here?
> > > This is used to 1) see if we are going to map an array of elements with 
> > > mapper, and 2) iterate all to map them individually.
> > Could you point where we have this kind of analysis here? Because I don't 
> > see anything affected by this change in the patch.
> Is this a bug fix in the previous implementation?
The previous implementation assumes the size is the number of elements, and it 
works correctly under that assumption. Since we change the meaning of size 
here, I add this line of code so the previous implementation can work correctly 
in the new assumption that the size is the size in bytes.


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

https://reviews.llvm.org/D67833



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

Reply via email to