[PATCH] D86928: [SVE][CodeGen] Fix TypeSize/ElementCount related warnings in sve-split-store.ll

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:552
     // TRUNCSTORE:i1 X -> TRUNCSTORE:i8 (and X, 1)
-    EVT NVT = EVT::getIntegerVT(*DAG.getContext(),
-                                StVT.getStoreSizeInBits());
+    EVT NVT = EVT::getIntegerVT(*DAG.getContext(), StSize.getFixedSize());
     Value = DAG.getZeroExtendInReg(Value, dl, StVT);
----------------
david-arm wrote:
> sdesmalen wrote:
> > What guarantees that `getFixedSize` can be used here?
> I think @eli.friedman mentioned that in an earlier comment that was only for scalar cases, although I agree there are no checks to actually prevent this. I just assumed that we wouldn't be creating arbitrarily large integer types, i.e. 128, 256, etc. I can try adding an extra "!StVT.isVector()" check and see if any tests fail?
It's implicitly prevented by the way legalization works.  In LegalizaeDAG, we assume all vector operations are legal: any illegal vector ops should have been handled by LegalizeVectorOps.

(I don't want to get into a deep discussion of why it works this way, but essentially, the reason it works this way is to allow LegalizeVectorOps to generate operations that involve illegal types.)


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:2449
+      Type *HiMemTy = HiMemVT.getTypeForEVT(*DAG.getContext());
+      Alignment = DAG.getDataLayout().getABITypeAlign(HiMemTy);
+      MPI = MachinePointerInfo(N->getPointerInfo().getAddrSpace());
----------------
The ABI alignment isn't relevant here.  The relevant numbers are the specified alignment, and the size of LoMemVT.  Basically, we need to reduce the alignment if it's bigger than the size of LoMemVT.

I'm sure we have at least one example of this elsewhere.


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

https://reviews.llvm.org/D86928