[PATCH] D108763: Use type sizes when determining dependence

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/depend_diff_types.ll:1
+; RUN: opt -S -tbaa -loop-accesses -analyze -enable-new-pm=0 < %s | FileCheck %s --check-prefix=CHECK-SAFE
+; RUN: opt -S -disable-output -passes='require<scalar-evolution>,require<tbaa>,require<aa>,loop(print-access-info)' < %s 2>&1 | FileCheck %s --check-prefix=CHECK-SAFE
----------------
I think there's an effort to remove remove the legacy opt invocations from `opt` and this already happened for the tests in this directory. Could you remove the line?


================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/depend_diff_types.ll:2
+; RUN: opt -S -tbaa -loop-accesses -analyze -enable-new-pm=0 < %s | FileCheck %s --check-prefix=CHECK-SAFE
+; RUN: opt -S -disable-output -passes='require<scalar-evolution>,require<tbaa>,require<aa>,loop(print-access-info)' < %s 2>&1 | FileCheck %s --check-prefix=CHECK-SAFE
+; RUN: opt -S -disable-output -passes='require<scalar-evolution>,require<tbaa>,require<aa>,loop(print-access-info)' -store-to-load-forwarding-conflict-detection < %s 2>&1 | FileCheck %s --check-prefix=CHECK-UNSAFE
----------------
does the test need `require<tbaa>`? It looks like there's no `!tbaa` metadata in the test.


================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/depend_diff_types.ll:8
+; are done as i32 types. When doing dependence analysis the type should not
+; matter if it can be determined that they are the same size.
+
----------------
All tests here have types where `TypeSize == StoreSize == AllocaSize`. Could you add a test with a type where `TypeSize != StoreSize != AllocaSize`, e.g. using something like `i19`.


================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/depend_diff_types.ll:25
+; CHECK-SAFE-NEXT:        BackwardVectorizable:
+; CHECK-SAFE-NEXT:            store float %3, float* %6, align 8 ->
+; CHECK-SAFE-NEXT:            store i32 %7, i32* %0, align 8
----------------
Why are there no check lines for `CHECK-UNSAFE` for this function? Same for `CHECK-SAFE` for the function below/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108763