[PATCH] D93610: [LegalizeType] When LegalizeType procedure widens a masked_gather, set MemoryType equal to ResultType

Bing Yu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 21 17:56:35 PST 2020


yubing added a comment.

In D93610#2466389 <https://reviews.llvm.org/D93610#2466389>, @craig.topper wrote:

> In D93610#2465689 <https://reviews.llvm.org/D93610#2465689>, @yubing wrote:
>
>> In D93610#2465382 <https://reviews.llvm.org/D93610#2465382>, @craig.topper wrote:
>>
>>> In D93610#2465300 <https://reviews.llvm.org/D93610#2465300>, @yubing wrote:
>>>
>>>> In D93610#2465295 <https://reviews.llvm.org/D93610#2465295>, @craig.topper wrote:
>>>>
>>>>> Fix WidenVecOp_MSCATTER as well?
>>>>>
>>>>> May should just take the element type from the original memory VT and the element count from result? Other targets have extending gathers I think which I think would have different element sizes between result and memory?
>>>>
>>>> Yeah, Besides If OpNo == 4 in WidenVecOp_MSCATTER, I guess we shouldn't widen MemType since we only widen the index?
>>>
>>> I think we should keep the same number of elements in the memory VT as the operands so I think it should be widened.
>>>
>>> I'm starting to wonder if the real mistake was making memory VT a vector for gather/scatter. Maybe it should have just been a scalar for the element type?
>>
>> Besides, do we use the information of MemType after LegalizeType procedure? I found X86InstrFragmentsSIMD.td may use it that info but mgather is handled outside X86DAGToDAGISel::SelectCode.
>
> X86 doesn't use it, but a target the support extending gather would need the memory VT to figure out how much it extends.





================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:4906
+    // Widen the MemoryType
+    WideMemVT = EVT::getVectorVT(*DAG.getContext(),
+                                 MSC->getMemoryVT().getScalarType(), NumElts);
----------------
craig.topper wrote:
> This needs to be widened for OpNo == 4 as well otherwise it would have a different element count.
Eh, You mean we should make MemType's EltNum equal to index's EltNum instead of ResultType's EltNum?
But in fact, index and result can have different element number and index's upper element is ignored as mentioned in https://reviews.llvm.org/D51337.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93610



More information about the llvm-commits mailing list