From 9f531d9a79f83daa9e888f15d94f817d22647c08 Mon Sep 17 00:00:00 2001
From: Matthias Reisinger <matthias.j.reisinger@gmail.com>
Date: Tue, 16 Aug 2016 15:39:36 +0200
Subject: [PATCH] Simplify bounds checks for multi-dimensional array accesses

This simplifies the array bounds check code that is emitted for
multi-dimensional array accesses that use "regular" indexing, i.e.
accesses of the form `A[i1,i2,...,iN]` to some `N`-dimensional array
`A`.

For example, with this change, the access `A[i,j,k]` to an array
with the three dimensions `m`, `n` and `o` now leads to bounds checks
that correspond to the following pseudo code:

```
if (i >= m)
  out_of_bounds_error();
else if (j >= n)
  out_of_bounds_error();
else if (k >= o)
  out_of_bounds_error();
```

So far, the following more complicated bounds checks would have been
emitted:

```
if (i >= m)
  out_of_bounds_error();
else if (j >= n)
  out_of_bounds_error();
else if (((k * n + j) * m + i) >= m * n * o)
  out_of_bounds_error();
```

Julia also allows one-dimensional and "partial" linear indexing
(see #14770), i.e. the number of indices used to access an array does
not have to match the actual number of dimensions of the accessed array.
For this case we still have use this old scheme.

One motivation for this change was the following: expressions like
`((k * n + j) * m + i)` are non-affine and Polly would not be able
to analyze them. This change therefore also facilitates Polly's bounds
check elimination logic, which would hoist such checks out of loops
or may remove them entirely where possible.
---
 src/cgutils.cpp | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/cgutils.cpp b/src/cgutils.cpp
index 9ad239345595d..e939785f2d69d 100644
--- a/src/cgutils.cpp
+++ b/src/cgutils.cpp
@@ -1247,6 +1247,14 @@ static void assign_arrayvar(jl_arrayvar_t &av, const jl_cgval_t &ainfo, jl_codec
         builder.CreateStore(emit_arraysize(ainfo, i+1, ctx), av.sizes[i]);
 }
 
+// Returns the size of the array represented by `tinfo` for the given dimension `dim` if
+// `dim` is a valid dimension, otherwise returns constant one.
+static Value *emit_arraysize_for_unsafe_dim(const jl_cgval_t &tinfo, jl_value_t *ex, size_t dim,
+                                            size_t nd, jl_codectx_t *ctx)
+{
+    return dim > nd ? ConstantInt::get(T_size, 1) : emit_arraysize(tinfo, ex, dim, ctx);
+}
+
 static Value *emit_array_nd_index(const jl_cgval_t &ainfo, jl_value_t *ex, size_t nd, jl_value_t **args,
                                   size_t nidxs, jl_codectx_t *ctx)
 {
@@ -1267,12 +1275,12 @@ static Value *emit_array_nd_index(const jl_cgval_t &ainfo, jl_value_t *ex, size_
     for(size_t k=0; k < nidxs; k++) {
         idxs[k] = emit_unbox(T_size, emit_expr(args[k], ctx), NULL);
     }
+    Value *ii;
     for(size_t k=0; k < nidxs; k++) {
-        Value *ii = builder.CreateSub(idxs[k], ConstantInt::get(T_size, 1));
+        ii = builder.CreateSub(idxs[k], ConstantInt::get(T_size, 1));
         i = builder.CreateAdd(i, builder.CreateMul(ii, stride));
         if (k < nidxs-1) {
-            Value *d =
-                k >= nd ? ConstantInt::get(T_size, 1) : emit_arraysize(ainfo, ex, k+1, ctx);
+            Value *d = emit_arraysize_for_unsafe_dim(ainfo, ex, k+1, nd, ctx);
 #if CHECK_BOUNDS==1
             if (bc) {
                 BasicBlock *okBB = BasicBlock::Create(jl_LLVMContext, "ib");
@@ -1287,9 +1295,21 @@ static Value *emit_array_nd_index(const jl_cgval_t &ainfo, jl_value_t *ex, size_
     }
 #if CHECK_BOUNDS==1
     if (bc) {
-        Value *alen = emit_arraylen(ainfo, ex, ctx);
-        // if !(i < alen) goto error
-        builder.CreateCondBr(builder.CreateICmpULT(i, alen), endBB, failBB);
+        // We have already emitted a bounds check for each index except for
+        // the last one which we therefore have to do here.
+        bool linear_indexing = nidxs < nd;
+        if (linear_indexing) {
+            // Compare the linearized index `i` against the linearized size of
+            // the accessed array, i.e. `if !(i < alen) goto error`.
+            Value *alen = emit_arraylen(ainfo, ex, ctx);
+            builder.CreateCondBr(builder.CreateICmpULT(i, alen), endBB, failBB);
+        } else {
+            // Compare the last index of the access against the last dimension of
+            // the accessed array, i.e. `if !(last_index < last_dimension) goto error`.
+            Value *last_index = ii;
+            Value *last_dimension = emit_arraysize_for_unsafe_dim(ainfo, ex, nidxs, nd, ctx);
+            builder.CreateCondBr(builder.CreateICmpULT(last_index, last_dimension), endBB, failBB);
+        }
 
         ctx->f->getBasicBlockList().push_back(failBB);
         builder.SetInsertPoint(failBB);