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

[Matt Arsenault via llvm-commits]

> On Feb 21, 2017, at 17:36, Friedman, Eli <efriedma at codeaurora.org> wrote:
> 
> 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 <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 <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

I’m not aware of any explicit documentation. There’s an implicit contract. I’ve debugged many infinite loops between DAGCombines and target custom combines.

-Matt
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170221/4749c93c/attachment-0001.html>