vabridgers added a comment.

In D119601#3317317 <https://reviews.llvm.org/D119601#3317317>, @NoQ wrote:

>> steakhal performed systems testing across a large set of open source 
>> projects.
>
> I'm curious if any findings there caused you to make changes in the patch. If 
> so it'd be great to reduce them into test cases so that other people didn't 
> make the same mistake when they edit your new code.

This patch was driven mostly by the negative cases we uncovered as represented 
in patch https://reviews.llvm.org/D118050. Basically, we have an internal 
proprietary multicore CPU with a CPU-local address space and a "global" common 
address space. We support memcpy intrinsics to copy to/from these memories with 
the different address space pointers - which also happen to be different 
pointer widths. The CString checker does an overlap check, and an assert caught 
a pointer comparison using different pointer widths. So the basic fix is to 
avoid overlap checks when the pointers are of different address spaces, and 
then that change ripples into these changes and the required supporting test 
cases. We also see similar problems in the SMTConv layer that we're just 
patching up internally for now in a non-principled way that are not suitable 
for upstreaming.

Since our target is downstream, it's challenging to find equivalent and 
relevant test cases, but we've made some headway since we found some GPUs also 
have similar properties. https://reviews.llvm.org/D118050 has one such test 
case. Perhaps it makes sense to squash that change with this one? @NoQ, do you 
have thoughts or a recommendation for squashing these two changes?

I believe some of the changes local to this patch can be covered with 
equivalent GPU test cases, I'll see if that can be created and add them.

As always, thanks for the guidance. Best


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119601

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

Reply via email to