[clang] [llvm] [OpenMP] Fix non-contiguous array omp target update (PR #156889)

Amit Tiwari via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 26 07:21:23 PDT 2026


amitamd7 wrote:

> I have revisited this PR (I am sorry I got distracted and forgot to merge it).
> 
> I confirmed the issues still persist in the latest version and that this fixes them.
> 
> In the latest version of the PR I also added the test above as a test under `offload`. However, while I was testing, I found that some tests have been introduced since I submitted the PR which in my opinion are wrong, namely these two:
> 
> ```
> offload/test/offloading/strided_update_variable_stride_misc.c
> offload/test/offloading/strided_update_count_expression_complex.c
> ```
> 
> I think both were introduced here #181987
> 
> @amitamd7 Would you be able to confirm that I am not missing something here?
> 
> For the first test:
> 
> ```
> offload/test/offloading/strided_update_variable_stride_misc.c
> ```
> 
> From my reading of it, the result for `// CHECK: Test 1: Variable stride = 1` should all be updated because we update with slice [0:10:1], however the CHECK lines represent the state before any update. I fixed this to have all indices updated in the CHECK lines and with this patch it seems to pass.
> 
> For the second test:
> 
> ```
> offload/test/offloading/strided_update_count_expression_complex.c
> ```
> 
> The checklines for this specific case seem wrong:
> 
> ```
> // CHECK: Test 2 - complex count with offset (from):
> // CHECK: s1 results:
> ```
> 
> We update the array with the slice [2:4:2] but in the original CHECK lines, the update started at 3rd index instead of the 2nd. So I have fixed the CHECK lines to represent the result I would expect and marked the test as XFAIL because it fails. The reason it fails seems unrelated to what this patch is attempting to fix, and I think it is related to the update command getting the address of the struct s1 as a whole instead of the array inside it for the base pointer of the update operation.
> 
> (cc'ing the reviewers of the other patch for visibility @alexey-bataev @abhinavgaba)

For test1 (strided_update_variable_stride_misc.c): You're correct. The CHECK lines I wrote reflect the original host state before the update -- that was my mistake. The slice [0:10:1] with stride=1 transfers all 10 elements from the device, so the expected output should be something like [0, 2, 4, 6, 8,...] (each element 2*i after data1[i] += i). 

For test2 (strided_update_count_expression_complex.c) You're correct here as well. s1.arr[2 : 4 : 2] should update indices 2, 4, 6,... but my CHECK lines show updates at indices 3, 5, 7, 9 which is off by one from the starting offset. Your corrected CHECK lines and the XFAIL marking for the separate base-pointer issue make sense to me.

Thank you for fixing these in your patch, and catching the mistakes. Please go ahead with this one.

https://github.com/llvm/llvm-project/pull/156889


More information about the cfe-commits mailing list