[llvm] r295782 - DAG: Check if extract_vector_elt is legal or custom

[Friedman, Eli via llvm-commits]

On 2/21/2017 4:47 PM, Matt Arsenault wrote:
>
>> On Feb 21, 2017, at 16:39, Friedman, Eli via llvm-commits 
>> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>>
>> On 2/21/2017 2:47 PM, Matt Arsenault via llvm-commits wrote:
>>> Author: arsenm
>>> Date: Tue Feb 21 16:47:27 2017
>>> New Revision: 295782
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=295782&view=rev
>>> Log:
>>> DAG: Check if extract_vector_elt is legal or custom
>>>
>>> Avoids test regressions in future AMDGPU commits when
>>> more vector types are custom lowered.
>>>
>>> Modified:
>>>     llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>>
>>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=295782&r1=295781&r2=295782&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Tue Feb 21 
>>> 16:47:27 2017
>>> @@ -7882,7 +7882,7 @@ SDValue DAGCombiner::visitTRUNCATE(SDNod
>>>      EVT SrcVT = VecSrc.getValueType();
>>>      if (SrcVT.isVector() && SrcVT.getScalarType() == VT &&
>>>          (!LegalOperations ||
>>> -         TLI.isOperationLegal(ISD::EXTRACT_VECTOR_ELT, SrcVT))) {
>>> +         TLI.isOperationLegalOrCustom(ISD::EXTRACT_VECTOR_ELT, 
>>> SrcVT))) {
>>>        SDLoc SL(N);
>>>          EVT IdxVT = TLI.getVectorIdxTy(DAG.getDataLayout());
>>>
>>
>> This doesn't work.  If LegalOperations is true, we're creating an 
>> operation which isn't legal after the legalizer has run, so a 
>> operation marked CUSTOM could fail to match.  (For example, on 
>> AArch64, we custom-lower EXTRACT_VECTOR_ELT on 64-bit vectors by 
>> widening them to 128-bit vectors.)
>>
>> If you're running into trouble with this, maybe it makes sense to add 
>> a specific helper here, along the lines of isShuffleMaskLegal?
>>
>> -Eli
>
> I don’t think that’s true since r214020? The pattern of checking 
> isLegalOrCustom is repeated in a variety of places (I’m not sure we 
> even really want to use isLegal anywhere anymore)
>

Is there documentation somewhere of exactly which illegal nodes 
DAGCombine is allowed to generate after legalization?  Or is there just 
an implicit contract that targets aren't allowed to custom-lower nodes 
in ways which contradict DAGCombine transforms?

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170221/195085af/attachment.html>