[PATCH] D108885: [Delinerization] Keep array element byte offset.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur created this revision.
Meinersbur added reviewers: Whitney, bmahjour, sebpop, grosser.
Herald added a reviewer: bollu.
Herald added subscribers: javed.absar, hiraditya.
Meinersbur requested review of this revision.
Herald added a project: LLVM.

The offset into the array element cannot just be discarded. In most cases it will be zero, but like any of the detected subscript any memory access must be checked whether it is contained in the byte range of the memory access (or aliases with neighboring array elements). This is because delinearization is just a heuristic, there is not guarantee that the byte offset is a constant, is non-negative, is less than the array element size, or the memory access is entirely contained within the array element (delinerization does not even know the access size).

Fix by removing the special handling of the least significant dimension in ScalarEvolution::computeAccessFunctions. The makes the returned Subscripts array one larger than the Sizes array. This actually would be expected, a subscript each size plus one subscript for the division remainder representing the outermost dimension of unknown size.

This bug caused Polly to miscompile blender (`526.blender_r` from SPEC CPU 2017) in -polly-process-unprofitable mode. The SCEV expression incorrectly delinearized has been reduced in the test case `byte_offset.ll`. The dropped offset into the array element of size 4 (a float) is `((sext i32 %mul7.i4534 to i64) + {(sext i32 %i1 to i64),+,((sext i32 (1 + ((1 + %shl.i.i) * (1 + %shl.i.i)) + %shl.i.i) to i64) * (sext i32 %i1 to i64))}<%for.body703>)`. This significant component was just dropped, and the wrong pointer was computed when regenerating code from the remaining delinearized subscripts.

This occurred during blender's subsurface scattering implementation. As a result, blender's rendering diverged from the reference image.

| Bug | Reference |
| --- | --------- |
| F18719158: sh5_reduced_0234.org-bug.png <https://reviews.llvm.org/F18719158>   | F18719160: sh5_reduced_0234.org-ref.png <https://reviews.llvm.org/F18719160>         |


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108885

Files:
  llvm/include/llvm/Analysis/ScalarEvolution.h
  llvm/lib/Analysis/Delinearization.cpp
  llvm/lib/Analysis/DependenceAnalysis.cpp
  llvm/lib/Analysis/LoopCacheAnalysis.cpp
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/test/Analysis/Delinearization/a.ll
  llvm/test/Analysis/Delinearization/byte_offset.ll
  llvm/test/Analysis/Delinearization/constant_functions_multi_dim.ll
  llvm/test/Analysis/Delinearization/divide_by_one.ll
  llvm/test/Analysis/Delinearization/himeno_1.ll
  llvm/test/Analysis/Delinearization/himeno_2.ll
  llvm/test/Analysis/Delinearization/iv_times_constant_in_subscript.ll
  llvm/test/Analysis/Delinearization/multidim_ivs_and_integer_offsets_3d.ll
  llvm/test/Analysis/Delinearization/multidim_ivs_and_integer_offsets_nts_3d.ll
  llvm/test/Analysis/Delinearization/multidim_ivs_and_parameteric_offsets_3d.ll
  llvm/test/Analysis/Delinearization/multidim_only_ivs_2d.ll
  llvm/test/Analysis/Delinearization/multidim_only_ivs_3d.ll
  llvm/test/Analysis/Delinearization/multidim_only_ivs_3d_cast.ll
  llvm/test/Analysis/Delinearization/parameter_addrec_product.ll
  polly/lib/Analysis/ScopDetection.cpp
  polly/test/ScopDetect/array_elt_byte_offset.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D108885.369298.patch
Type: text/x-patch
Size: 28831 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210829/6e8746d9/attachment.bin>