[PATCH] D90171: [SVE] Fix TypeSize warning in RuntimePointerChecking::insert

Mon Nov 23 13:43:48 PST 2020

fhahn added a comment.

In D90171#2410782 <https://reviews.llvm.org/D90171#2410782>, @joechrisellis wrote:

> @fhahn Hi! Do you have any objections to landing this as-is without the additional test? Alternatively, any recommendations on how to test this? 😄

I think you would need to use element types with different store & alloca sizes, like `i19` (see https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/IR/DataLayout.h#L446 for some examples). You should be able to adjust a test with runtime checks (e.g. `llvm/test/Analysis/LoopAccessAnalysis/memcheck-off-by-one-error.ll`) and update it to use a type like `i19` to get such a test. If you decide to use `llvm/test/Analysis/LoopAccessAnalysis/memcheck-off-by-one-error.ll`, I think some of the casts in the loop body could be stripped.

================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/runtime-pointer-checking-insert-typesize.ll:1
+; RUN: opt -loop-load-elim -mtriple=aarch64-linux-gnu -mattr=+sve < %s >/dev/null 2>%t
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
----------------
joechrisellis wrote:
> fhahn wrote:
> > Ideally the tests in this directory would only use `-loop-accesses -analyze` and no transformation passes. Is that possible? Also, is `-mattr=+sve` needed?
> > 
> > If `loop-load-elimination` is required, the test should probably go there. Also it might be good to just have all scaleable tests in a single file in the directory>
> Thanks for the pointers!
> 
> > Ideally the tests in this directory would only use -loop-accesses -analyze and no transformation passes. Is that possible? Also, is -mattr=+sve needed?
> 
> Ah okay -- turns out we can just use `-loop-accesses -analyze` for the same result. Fixed this.
> 
> > If loop-load-elimination is required, the test should probably go there.
> 
> It's not required. :)
>> If loop-load-elimination is required, the test should probably go there.
> It's not required. :)

Looks like there's another test that runs `-loop-load-elim`: llvm/test/Analysis/LoopAccessAnalysis/gep-induction-operand-typesize-warning.ll. Could this test also be updated?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90171