[llvm-commits] [PATCH] Use GetUnderlyingObjects in misched

[Andrew Trick]

On Dec 10, 2012, at 10:54 AM, Hal Finkel <hfinkel at anl.gov> wrote:

> ----- Original Message -----
>> From: "Andrew Trick" <atrick at apple.com>
>> To: "Hal Finkel" <hfinkel at anl.gov>
>> Cc: llvm-commits at cs.uiuc.edu
>> Sent: Friday, December 7, 2012 6:16:50 PM
>> Subject: Re: [llvm-commits] [PATCH] Use GetUnderlyingObjects in misched
>> 
>> 
>> On Dec 6, 2012, at 10:28 AM, Andrew Trick <atrick at apple.com> wrote:
>> 
>>> Hal,
>>> 
>>> I'm hoping to do some compile time measurements soon. I'll apply
>>> your patch and verify that it isn't a problem for the usual
>>> benchmarks. I agree that GetUnderlyingObjects should use its
>>> MaxLookup parameter correctly. That's a more general problem that
>>> doesn't need to hold up the patch.
>> 
>> I don't see any alarming compile times with your patch. Actually I
>> tested a modified version of the patch where I removed the inner
>> loop that revisits all found objects at each step.
> 
> As we've discussed, this could lead to duplicate DAG edges. As you've asserted that this is not actually a correctness problem, I'm certainly fine with it.
> 
>> Take a look at
>> the modifications. If you agree, then please check in.
> 
> Thanks! r169744.

Thanks for the fix. Summarizing the code you checked in:

There is only one visited set.

+  SmallPtrSet<const Value*, 16> Visited;
...

Objs may contain duplicates.

+    for (SmallVector<Value *, 4>::iterator I = Objs.begin(), IE = Objs.end();
+         I != IE; ++I) {
+      V = *I;
+      if (!Visited.insert(V))
+        continue;
...

But a value is only pushed on the result list the first time it is marked Visited.

+      Objects.push_back(const_cast<Value *>(V));
+    }
+  } while (!Working.empty());

So all Objects should be unique.

-Andy