[PATCH] D103180: [InstSimplify] Add constant fold for extractelement + splat for scalable vectors

[Caroline via Phabricator via llvm-commits]

CarolineConcatto marked an inline comment as done.
CarolineConcatto added a comment.

Hi @foad,
Sorry for my miss interpretation.
I believe now it is as you suggested.



================
Comment at: llvm/lib/IR/ConstantFold.cpp:916-917
         return CAZ->getElementValue(CIdx->getZExtValue());
+      if (Constant *SplatVal = Val->getSplatValue())
+        return SplatVal;
     }
----------------
sdesmalen wrote:
> CarolineConcatto wrote:
> > CarolineConcatto wrote:
> > > foad wrote:
> > > > You should be able to remove the two lines above handling the CAZ case, because your new code will handle it.
> > > a
> > Hi @foad 
> > I believe we should keep these 2 tests:
> > !CIdx->uge(ValSVTy->getMinNumElements())
> > because it limits the index to be lower than the minimum width of the scalable vector.
> > 
> > and it should be only for scalable vector because of this 
> >         return CAZ->getElementValue(CIdx->getZExtValue());
> > transformation. 
> > If we change to be vector type  many other  tests fail. For instance this one:
> >  CodeGen/AMDGPU/amdgpu-codegenprepare-idiv.ll
> > Transforms/LoopVectorize/X86/gather_scatter.ll
> Hi @CarolineConcatto, I think @foad meant removing the lines:
> 
>   if (auto *CAZ = dyn_cast<ConstantAggregateZero>(Val))
>     return CAZ->getElementValue(CIdx->getZExtValue());
> 
> If `Val` is a constant aggregate zero (<=> all zeroes), then this is a splat value that's recognized by `getSplatValue`, so your new lines added in this patch will handle the same case making the above lines redundant.
Thank you @sdesmalen, that makes sense.
Change made.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103180