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

Thu Mar 18 08:03:11 PDT 2021

craig.topper added inline comments.

================
Comment at: llvm/include/llvm/Support/TypeSize.h:451
-#ifdef STRICT_FIXED_SIZE_VECTORS
     return getFixedValue();
-#else
----------------
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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98736