[PATCH] D145616: [LV] Use speculatability within entire loop to avoid strided load predication

Tue Mar 21 07:32:46 PDT 2023

anna marked an inline comment as done.
anna added inline comments.

================
Comment at: llvm/test/Transforms/LoopVectorize/X86/load-deref-pred.ll:1023
 ; CHECK-NEXT:    [[TMP63:%.*]] = insertelement <4 x i1> [[TMP62]], i1 [[TMP59]], i32 3
 ; CHECK-NEXT:    [[TMP64:%.*]] = getelementptr i32, ptr [[ALLOCA]], i64 [[TMP0]]
 ; CHECK-NEXT:    [[TMP65:%.*]] = getelementptr i32, ptr [[ALLOCA]], i64 [[TMP4]]
----------------
anna wrote:
> One thing I noticed is that we drop the `inbounds` on GEPs when we converted the masked loads to unmasked versions (perhaps because we cannot prove if the `inbounds` is correct without the predication?). We do not do the same "dropping of inbounds" when we removed predication for the strided case.  Any idea why is that? It looks like we should be dropping on the strided case, but I don't know the LV code well enough to see where this is done and what is missing. 
Just to loop back on this: 
I did some digging into history of where this inbounds drop was introduced. It was here: https://reviews.llvm.org/D111846. Also, there is a specific comment stating we do not need to drop inbounds (and other poison generating flags) when the original instructions are gather/scatter. If backends convert the gather/scatter into use "base + offsets", those backends need fixing (just paraphrasing from the comment here: https://reviews.llvm.org/D111846#3098547).

So, I'll go ahead and land this change.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145616