[PATCH] D98736: [SelectionDAG] Don't pass a scalable vector to MachinePointerInfo::getWithOffset in a unit test.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

If you remove the change to `operator ScalarTy()`, I'd be happy to accept this patch and rebase D98856 <https://reviews.llvm.org/D98856>.
I did something similar in D98856 <https://reviews.llvm.org/D98856> for this test, but wasn't sure if the first `MachineMemOperand` should just be PtrInfo directly, since the added offset is 0.
(It probably doesn't matter that much, because the information isn't really used by computeAliasing if I'm understanding the code correctly)



================
Comment at: llvm/include/llvm/Support/TypeSize.h:451
-#ifdef STRICT_FIXED_SIZE_VECTORS
     return getFixedValue();
-#else
----------------
craig.topper wrote:
> sdesmalen wrote:
> > I don't think we're at a point yet where we can always safely remove the warning in favour of the assert (from getFixedValue).
> > 
> > While it may not break any of the tests (other than the one you fixed in this patch), users of Clang that use the SVE intrinsics, or that try out some of the experimental vectorization work, don't want their compiler to actually break when LLVM goes down a code-path that hasn't been migrated yet to work for scalable vectors. Note that the code may still be doing the right thing, but just using the wrong interfaces.
> > 
> > Having the warning instead of an error has helped us bring up scalable-vector support incrementally. I agree that we need to work to remove this in the near future, and hopefully the work on fixed-width vectorization (mapping to scalable vectors at SelectionDAG), and the work on scalable-auto-vec will increase our test-coverage so that we'll soon have enough confidence to remove this code altogether.
> > 
> > I've created a patch D98856 that puts the behaviour to demote the error to a warning message under an option, which is enabled by default. This means all tests fail if the wrong interface is used, but it allows tools (like Clang) to suppress the error for now.
> So sorry that was not supposed to be in here. That was just how I caught the issue. I must have accidentally committed it
No worries, thanks for the clarification!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98736