[PATCH] [OPENMP] Fixed codegen for arrays in 'copyprivate' clause.

John McCall rjmccall at gmail.com
Fri Apr 10 10:26:13 PDT 2015


Thank you.  Unfortunately, the assignment loop is now wrong — you're copying indexes 1 to N instead of 0 to N-1.


================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:99
@@ +98,3 @@
+      Builder.CreateICmpULT(SrcNumElements, DestNumElements), SrcNumElements,
+      DestNumElements);
+  auto DestEnd = Builder.CreateGEP(DestBegin, NumElements);
----------------
Can these actually be different?  It looks like you're decomposing the same array type.

I figured you were just doing that to get the type adjustment on DestBegin right.  That's fine and basically harmless; LLVM will be able to delete any unused operations you do (if it's a VLA type) pretty easily.  You could also reasonably just enhance emitArrayLength to do the type adjustment to multiple pointers at once; 2 is pretty common for exactly this kind of purpose.  Or you could just bitcast DestBegin to the resulting type of SrcBegin and let LLVM figure it out.

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:119
@@ +118,3 @@
+
+  // Shift the address back by one element.
+  auto DestElement = Builder.CreateConstGEP1_32(DestElementCurrent, /*Idx0=*/1,
----------------
You're now shifting the address *forward*, and you need to do this after the element copy.  That is, you should be doing the element copy from SrcElementCurrent to DestElementCurrent, and you should probably rename DestElement/SrcElement to DestElementNext/SrcElementNext.

http://reviews.llvm.org/D8914

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list