[PATCH] D87378: [SVE] Fix InstCombinerImpl::PromoteCastOfAllocation for scalable vectors

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:103
+  // checks.
+  if (isa<ScalableVectorType>(AllocElTy) || isa<ScalableVectorType>(CastElTy))
+    return nullptr;
----------------
sdesmalen wrote:
> This also seems to prevent the optimisation on bitcasts between scalable-vector-type T1 and an alloca with scalable-vector-type T2. You can make the expression use `!=` instead of `||`, but then the code below needs some more changes to work on TypeSize.
Yes, this was deliberate. I think I discussed this in the commit message. I was trying to avoid making needless code changes that's all because casting alloca to scalable still causes us to fail the alignment checks below. I simply couldn't write a test case that proved the additional work needed was correct. I thought it better to avoid changing the code path and then being unable to defend it. I still have the original patch that converted everything to using TypeSize, which I could make use of here if we are able to come up with a test that showed a before and after difference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87378