RPCS3/llvm-mirror
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2024-11-23 19:23:23 +01:00
Code Issues Projects Releases Wiki Activity

[x86] Don't add nodes to the combined set (and prune subsequent

Browse Source 
combines) until they are legal.

Doing it the old way could, when the stars align *just* right, cause
a node to get into the combine set prior to being legalized. Then, when
the same node showed up as an operand to another node later on (but not
so much later on that it had been deleted as dead) we would fail to add
it back to the worklist thinking it had already been combined. This
would in turn cause it to not be legalized. Fortunately, we can also
walk the operands looking for uncombined (and thus potentially
un-legalized) nodes late. It will still ensure that we walk all operands
of all nodes and send all of them through both the legalizer without
changes and the combiner at least once. (Which was the original goal of
this).

I have a test case for this bug, but it is terribly brittle. For
example, it will stop finding the bug the moment I enable the new
shuffle lowering. I don't yet have any test case that reliably exercises
this bug, and it isn't clear that it will be possible to craft one. It
is entirely possible that with the new shuffle lowering the two forms of
doing this are precisely equivalent. That doesn't mean we shouldn't take
the more conservative approach of insisting on things in the combined
set having survived the legalizer.

llvm-svn: 214673
This commit is contained in:
Chandler Carruth 2014-08-03 23:10:59 +00:00
parent 1a36f331c8
commit 9918929946
1 changed files with 8 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
View File

@ -1152,14 +1152,6 @@ void DAGCombiner::Run(CombineLevel AtLevel) {
if (recursivelyDeleteUnusedNodes(N))
continue;
// Add any operands of the new node which have not yet been combined to the
// worklist as well. Because the worklist uniques things already, this
// won't repeatedly process the same operand.
CombinedNodes.insert(N);
for (unsigned i = 0, e = N->getNumOperands(); i != e; ++i)
if (!CombinedNodes.count(N->getOperand(i).getNode()))
AddToWorklist(N->getOperand(i).getNode());
WorklistRemover DeadNodes(*this);
// If this combine is running after legalizing the DAG, re-legalize any
@ -1178,6 +1170,14 @@ void DAGCombiner::Run(CombineLevel AtLevel) {
DEBUG(dbgs() << "\nCombining: "; N->dump(&DAG));
// Add any operands of the new node which have not yet been combined to the
// worklist as well. Because the worklist uniques things already, this
// won't repeatedly process the same operand.
CombinedNodes.insert(N);
for (unsigned i = 0, e = N->getNumOperands(); i != e; ++i)
if (!CombinedNodes.count(N->getOperand(i).getNode()))
AddToWorklist(N->getOperand(i).getNode());
SDValue RV = combine(N);
if (!RV.getNode())