[PATCH] D108763: Use type sizes when determining dependence

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/depend_diff_types.ll:134
+  %11 = trunc i64 %7 to i19
+  store i19 %11, i19* %10, align 8
+
----------------
fhahn wrote:
> sdesmalen wrote:
> > fhahn wrote:
> > > can you also throw in a read or write to a 32 bit type? Might be good to move those into a separate test.
> > Can you be a bit more specific about what you'd like to see? I can't quickly see what added value a read/write to 32bit type would add here. Or see which part needs moving to a separate test.
> I was thinking about a test that explicitly checks that the alloc types are compared in the `HasSameSize` check. Having a  test with `i19` and `double` accesses doesn't really ensure this, as `DL.getTypeAllocSize(double) !=` all the different sizes (store, alloc) for `i19`,( `i19` and `double` will never have the same size, except in very weird data layouts)
> 
> In a test that has accesses with `i19` and `i32`, we should get different results for the `HasSameSize` check, if we use a different size in the compare (e.g. if we compare the type size directly), like below
> 
> (btw the use of unnamed IR values makes it a bit harder to play with the test cases)
> ```
> define void @neg_dist_dep_type_size_equivalence(i64* nocapture %vec, i64 %n) {
> entry:
>   br label %loop
> 
> loop:
>   %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %loop ]
> 
>   ;; Load from vec[indvars.iv] as double
>   %0 = getelementptr i64, i64* %vec, i64 %indvars.iv
>   %1 = bitcast i64* %0 to i32*
>   %2 = load i32, i32* %1, align 8
>   %3 = mul i32 %2, 5
> 
>   ;; Different sizes
>   %v8 = add nuw nsw i64 %indvars.iv, %n
>   %v9 = getelementptr inbounds i64, i64* %vec, i64 %v8
>   %v10 = bitcast i64* %v9 to i19*
>   %v11 = trunc i32 %3 to i19
>   store i19 %v11, i19* %v10, align 8
> 
>   ;; Loop condition.
>   %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
>   %cond = icmp eq i64 %indvars.iv.next, %n
>   br i1 %cond, label %exit, label %loop
> 
> exit:
>   ret void
> }
> ```
Thanks, that makes sense! I've added a new test. Please have a look to see if that's what you were expecting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108763