[llvm-commits] Patch: avoid reanalyze a node twice in LegalizeTypes

[Mon Ping Wang]

Hi Duncan,

Thanks for the info and looking into this further.

  -- Mon Ping


On Jan 13, 2009, at 12:15 AM, Duncan Sands wrote:

> Hi Mon Ping,
>
>> This patch is to fix an assert that we get for the test case below
>>   Assertion failed: (N->getNodeId() !=
>> DAGTypeLegalizer::ReadyToProcess && N->getNodeId() !=
>>   DAGTypeLegalizer::Processed && "Invalid node ID for RAUW
>> analysis!"), function ReplaceValue
>>
>> In the DAG, we get is something like
>>
>> %tmp1 = extract_subvector <2 x double> %tmp0,  <4 x double> %vec, 0
>> %tmp2 = extract_element <2 x double> %tmp1, 0
>> %tmp3 = extract_element <2 x double> %tmp1, 1
>> %tmp4 = build_vector<4 x double> %tmp2, %tmp3, %tmp?, %tmp?
>>
>> We discover when examining the operands of %tmp1 that %vec is not a
>> legal type and we split it.  This causes us to replace all uses of
>> %tmp1 which triggers us update %tmp2 and %tmp3 and then update the
>> %tmp4.  The problem is that when we do the update, we will add %tmp4
>> node twice to UpdateListener (NodesToAnalyze) i..e, we add it when we
>> process %tmp2 and again for %tmp3.  When walking through the nodes to
>> reanalyze, the first time we see %tmp4, we update the NodeId to the
>> NewNode.  When we encounter the node again through the list, we hit
>> the assertion since we have already updated that Node.
>>
>> The following patch fixes this by changing NodesToAnalyze from
>> SmallVector to use SmallPtrSet to avoid duplicates.  Please let me
>> know of any comments.
>
> thanks for looking into this.  I used SmallVector in order to make
> things deterministic - the NodesDeleted set was, as a side-effect,
> supposed to avoid duplicates IIRC.  I will try to analyze this later
> today.
>
> Ciao,
>
> Duncan.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits