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

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/VNCoercion.cpp:321
+  if (SrcVal->getType()->isVectorTy()) {
+    if (StoreSize > LoadSize) {
+      auto *SrcValTyV = cast<FixedVectorType>(SrcVal->getType());
----------------
Invert condition and early return?


================
Comment at: llvm/lib/Transforms/Utils/VNCoercion.cpp:377
+        }
+        unsigned Idx = Offset / LoadSize;
+        SrcVal = Builder.CreateExtractElement(SrcVal, Idx);
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154228