+ Allow and handle memcpy, memset and memmove intrinsics. + Allow to add lower/upper bounds to the access range of non-affine memory accesses (useful with a range analysis)
Details
Diff Detail
Event Timeline
Added early pointer check & adjusted based on the change of the misc intrinsics commit
Hi Johannes,
sorry I never got to review this patch. It looks small, self-contained and seems to be going into the right direction. Hence, I could most likely review this quickly if there is still interest. Does this patch still apply or does it require a rebase?
include/polly/Support/ScopHelper.h | ||
---|---|---|
169 | This doesn't make sense to me. | |
lib/Analysis/ScopDetection.cpp | ||
492 | should it be getArgOperand (which is equivalent at the moment, but safer)? | |
500 | How about use getRawDest or getDest (this one strip the pointer-cast)? | |
lib/Analysis/ScopInfo.cpp | ||
761 | simply isa<MemIntrinsic>(getAccessInstruction())? | |
2578 | why we relax this assertion? |
include/polly/Support/ScopHelper.h | ||
---|---|---|
169 | We may simply return nullptr here. |
lib/Analysis/ScopInfo.cpp | ||
---|---|---|
2578 | Because the alias sets in the alias set tracker look differen when we also add memory intrinsics. This does not change the semantics of the algorithm though, it just refrlects that the alias groups can be small now such that we do not care. |
I don't see that the copied' size gets evaluated apart from checking that it's affine.
Does the following work as expected?
float A[128][128], B[128]; for (int i = 0; i < 128; i+=1) memcpy(A[i], B, 128*sizeof(float));
include/polly/Support/ScopHelper.h | ||
---|---|---|
224 | Maybe add a comment explaining this? I guess this is taken from LoadInst/StoreInst implementation of isUnordered() with getOrdering() returning NotAtomic. | |
lib/Analysis/ScopDetection.cpp | ||
38 | Can you be more specific: memset, memcpy, memmove are allowed? | |
lib/Analysis/ScopInfo.cpp | ||
306 | LLVM coding style suggests to use auto only in specific cases. Eg. where the type already appears on the right side or the type is already abstracted away. If you keep auto, the asterisk is superfluous. | |
test/ScopInfo/memcpy.ll | ||
17 | Shouldn't this be a ReadAccess? This look like memcpy write two locations instead of reading one, writing the other. | |
test/ScopInfo/memmove.ll | ||
17 | dito |
The examples contain length values already. I think they work. The evaluation can be found in MemoryAccess::buildMemIntrinsicAccessRelation.
lib/Analysis/ScopInfo.cpp | ||
---|---|---|
306 |
Since the variable is called BB I guess the type is kinda obvious.
But it tells you that this is a pointer. We had this discussion before and while ppl started to ignore it and use plain auto I still think it is a good idea to add * and & if appropriate. |
This passes all LNT tests with an unprofitable build.
@etherzhhb, @Meinersbur: Are there still open problems/comments or can I commit this?
load, store or memintrinsic ...