[PATCH] D132700: [DSE] Add value type checks for masked store candidates in Dead Store Elimination

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added a comment.

LGTM, thanks! Please add the extra test case as discussed separately.



================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:252
+    VectorType *DeadTy = cast<VectorType>(DeadII->getArgOperand(0)->getType());
+    if (KillingTy->getScalarSizeInBits() != DeadTy->getScalarSizeInBits())
+      return OW_Unknown;
----------------
craig.topper wrote:
> mcberg2021 wrote:
> > fhahn wrote:
> > > This should be fine if `KillingTy->getScalarSizeInBits >= DeadTy->getScalarSizeInBits()`, right? I might have missed it, but t looks like there's no test case like the one @mcberg2021 mentioned above?
> > That too should work.  I will update for the subsumption case. However I would like to leave the mask decomposition support for a later change.
> If the mask is unknown then the store is not writing contiguous bytes. There can be gaps. So it is not sufficient to check that scalar size is larger.
Yeah good point, we will need to combine this with the mask analysis. Still good to add the extra test coverage. The new test seems to have constant masks, it would be good to also have variants where the mask is unknown (e.g. arg to function)


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

https://reviews.llvm.org/D132700