[PATCH] D154228: [GVN] Use vector ops when doing loadCoercion on a vector value

[Manuel Brito via Phabricator via llvm-commits]

ManuelJBrito added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/VNCoercion.cpp:377
+        }
+        unsigned Idx = Offset / LoadSize;
+        SrcVal = Builder.CreateExtractElement(SrcVal, Idx);
----------------
nikic wrote:
> This looks incorrect to me for the case where the division isn't exact, e.g. this is a miscompile:
> ```
> define i16 @test(ptr %loc, <4 x i16> %v) {
> ; CHECK-LABEL: define i16 @test
> ; CHECK-SAME: (ptr [[LOC:%.*]], <4 x i16> [[V:%.*]]) {
> ; CHECK-NEXT:    store <4 x i16> [[V]], ptr [[LOC]], align 8
> ; CHECK-NEXT:    [[GEP:%.*]] = getelementptr i8, ptr [[LOC]], i64 1
> ; CHECK-NEXT:    [[TMP1:%.*]] = extractelement <4 x i16> [[V]], i64 0
> ; CHECK-NEXT:    ret i16 [[TMP1]]
> ;
>   store <4 x i16> %v, ptr %loc
>   %gep = getelementptr i8, ptr %loc, i64 1
>   %ref = load i16, ptr %gep
>   ret i16 %ref
> }
> ```
> The loaded value is the high part of elements zero and the low part of element 1, not just element 0.
Nice catch! Thank you! NumEltsRequiredFromVec needs to take into consideration the offset. I'll try to support this case without making this even more complex.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154228