This is an archive of the discontinued LLVM Phabricator instance.

CodeGen, SelectionDAG: Increase width of NumOperands and NumValues
Needs ReviewPublic

Authored by majnemer on Nov 7 2014, 1:59 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Something like:
%a = type { [65536 x i8] }
store %a zeroinitializer, %a* undef

can generate an SDNode with more than USHRT_MAX results. In this
particular case, we will end up thinking we have zero values.

When it would come time to make an SDValue, we would assert because we
wouldn't think we had any values at all.

This fixes PR21513.

N.B. The smallest, most minimal test case I could write takes three
minutes on a very powerful machine.

Diff Detail

Event Timeline

majnemer updated this revision to Diff 15914.Nov 7 2014, 1:59 AM
majnemer retitled this revision from to CodeGen, SelectionDAG: Increase width of NumOperands and NumValues.
majnemer updated this object.
majnemer added a subscriber: Unknown Object (MLST).
hfinkel added a subscriber: hfinkel.Nov 7 2014, 7:16 AM

Does this actually solve the problem? How about adding a better assert and then making sure that we don't combine token-factors nodes when the result would have too many operands (I'm assuming the token factor node is actually the problem)? Making the structure bigger is not something to be done lightly.

We discussed this on IRC but I'll repeat the conclusion our conclusion for the benefit of all.

SelectionDAGBuilder will attempt to create a MERGE_VALUES node with more than USHRT_MAX operands in order to turn our ConstantArray into an SDNode.

It has been determined that this is a correct fix but possible a costly one as it grows the size of SDNode.

For now, I will just add assertions to SDNode's constructor to catch this a little eagerly instead of asserting later on in SDValue.

Assertion added in rL221560.

qiucf added a subscriber: qiucf.Aug 26 2020, 7:42 PM

Is this still relevant?