On Nov 24, 2007, at 12:55 AM, Duncan Sands wrote:
> Hi Chris,
>
>> /// RemapNode - If the specified value was already legalized to
>> another value,
>> /// replace it by that value.
>> void DAGTypeLegalizer::RemapNode(SDOperand &N) {
>> - DenseMap<SDOperand, SDOperand>::iterator I =
>> ReplacedNodes.find(N);
>> - if (I != ReplacedNodes.end()) {
>> - RemapNode(I->second);
>> + for (DenseMap<SDOperand, SDOperand>::iterator I =
>> ReplacedNodes.find(N);
>> + I != ReplacedNodes.end(); I = ReplacedNodes.find(N))
>> N = I->second;
>> - }
>> }
>
> in the original version this updated the map too (or at least it was
> supposed
> to). For example, suppose we remap N and the map contained:
> N -> A
> A -> B
> B -> C
> then afterwards we would get N = C, but also the map would be
> modified to:
> N -> C
> A -> C
> B -> C
> This was supposed to speed up later lookups.
Urg, I completely missed that. Restored, sorry!
>> + if (SDNode *P = TLI.ExpandOperationResult(N, DAG)) {
>> + // Everything that once used N now uses P. P had better not
>> require
>> + // custom expansion.
>
> How about an assertion that it doesn't require custom expansion?
I don't know what I was thinking with that comment, I updated it to
better describe the constraints.
Thanks for the review Duncan!
-Chris
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
