[PATCH] D146208: [ASAN] Support memory checks on vp.load/store.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:1670
+  for (unsigned Idx = 0; Idx < Num; ++Idx) {
+    IRB.SetInsertPoint(InsertBefore);
+    Func(IRB, ConstantInt::get(Ty, Idx));
----------------
fakepaper56 wrote:
> craig.topper wrote:
> > Why do we need to keep setting the insertion point every iteration? Will that cause the last index to be checked first?
> > 
> > Is there a test case for constant evl. I didn't see one.
> > Why do we need to keep setting the insertion point every iteration?
> This is to follow the SplitBlockAndInsertForEachLane(ElementCount, Type *, Instruction *, std::function<void(IRBuilderBase&, Value*)>).  And the old one is to follow the legacy code.
> 
> >Will that cause the last index to be checked first?
> I think it won't because the letter index will be inserted to the back of the former index.
> 
> > Is there a test case for constant evl.
> Fixed vector masked.load will have a constant evl here. store.v4f32.1110 in llvm/test/Instrumentation/AddressSanitizer/asan-masked-load-store.ll is an example.
> > Why do we need to keep setting the insertion point every iteration?
> This is to follow the SplitBlockAndInsertForEachLane(ElementCount, Type *, Instruction *, std::function<void(IRBuilderBase&, Value*)>).  And the old one is to follow the legacy code.
> 
> >Will that cause the last index to be checked first?
> I think it won't because the letter index will be inserted to the back of the former index.

You're right. InsertBefore is still after whatever the previous iteration inserted so it didn't really change the insertion point.

> 
> > Is there a test case for constant evl.
> Fixed vector masked.load will have a constant evl here. store.v4f32.1110 in llvm/test/Instrumentation/AddressSanitizer/asan-masked-load-store.ll is an example.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146208