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

[craig.topper]

Summary

Suppresses an implicit TypeSize to uint64_t conversion warning.

We might be able to just not offset it since we're writing to a
Fixed stack object, but I wasn't sure so I just did what
DAGTypeLegalizer::IncrementPointer does.

[sdesmalen]

llvm/include/llvm/Support/TypeSize.h

451 ↗

(On Diff #331065)

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.

[craig.topper]

llvm/include/llvm/Support/TypeSize.h

451 ↗

(On Diff #331065)

So sorry that was not supposed to be in here. That was just how I caught the issue. I must have accidentally committed it

[sdesmalen]

If you remove the change to operator ScalarTy(), I'd be happy to accept this patch and rebase D98856.
I did something similar in 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)

llvm/include/llvm/Support/TypeSize.h

451 ↗

(On Diff #331065)

No worries, thanks for the clarification!

[craig.topper]

Remove extra change. Keep PtrInfo for the first store.

[craig.topper]

Change the right function

[craig.topper]

llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp

171

Should we get rid of Index0 too?

[sdesmalen]

llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp

171

Yes, I think that makes sense, it makes the test a bit simpler to understand.

[craig.topper]

Remove Index0

[sdesmalen]

Thanks, LGTM!

llvm/unittests/CodeGen/SelectionDAGAddressAnalysisTest.cpp

TEST_F(SelectionDAGAddressAnalysisTest, unknownSizeFrameObjects) {
   // <vscale x 4 x i8>
   auto VecVT = EVT::getVectorVT(Context, Int8VT, 4, true);
   // <vscale x 2 x i8>
   auto SubVecVT = EVT::getVectorVT(Context, Int8VT, 2, true);
   SDValue FIPtr = DAG->CreateStackTemporary(VecVT);
   int FI = cast<FrameIndexSDNode>(FIPtr.getNode())->getIndex();
   MachinePointerInfo PtrInfo = MachinePointerInfo::getFixedStack(*MF, FI);
   SDValue Value = DAG->getConstant(0, Loc, SubVecVT);
-  TypeSize Offset0 = TypeSize::Fixed(0);
   TypeSize Offset1 = SubVecVT.getStoreSize();
-  SDValue Index0 = DAG->getMemBasePlusOffset(FIPtr, Offset0, Loc);
   SDValue Index1 = DAG->getMemBasePlusOffset(FIPtr, Offset1, Loc);
-  SDValue Store0 = DAG->getStore(DAG->getEntryNode(), Loc, Value, Index0,
-                                 PtrInfo.getWithOffset(Offset0));
+  SDValue Store0 =
+      DAG->getStore(DAG->getEntryNode(), Loc, Value, FIPtr, PtrInfo);
   SDValue Store1 = DAG->getStore(DAG->getEntryNode(), Loc, Value, Index1,
-                                 PtrInfo.getWithOffset(Offset1));
+                                 MachinePointerInfo(PtrInfo.getAddrSpace()));
   Optional<int64_t> NumBytes0 = MemoryLocation::getSizeOrUnknown(
       cast<StoreSDNode>(Store0)->getMemoryVT().getStoreSize());
   Optional<int64_t> NumBytes1 = MemoryLocation::getSizeOrUnknown(
       cast<StoreSDNode>(Store1)->getMemoryVT().getStoreSize());
 
   bool IsAlias;
   bool IsValid = BaseIndexOffset::computeAliasing(
       Store0.getNode(), NumBytes0, Store1.getNode(), NumBytes1, *DAG, IsAlias);