Page MenuHomePhabricator

[DAGCombine] Unchecked calls to DAGCombiner::*ExtPromoteOperand

Authored by jacobly on May 28 2017, 12:03 PM.



Other calls to DAGCombiner::*PromoteOperand check the result, but here it could cause an assertion in getNode. Falling back to any extend in this case instead of failing outright seems correct to me.

Diff Detail


Event Timeline

jacobly created this revision.May 28 2017, 12:03 PM
spatel edited edge metadata.Jun 3 2017, 9:36 AM

Did you find a place where this fails? If so, it would be better to add that test case.

I would have, but it was triggered by an out of tree backend. In order to trigger it, a backend would need to overload TargetLowering::IsDesirableToPromoteOp to return true for a type for which ISD::SIGN_EXTEND_INREG is marked illegal. In tree, only X86 overloads and sometimes returns true for MVT::i16 yet it marks setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::i16 , Legal);.

spatel accepted this revision.Jun 4 2017, 7:45 AM

Ok - thanks for the explanation. LGTM.

This revision is now accepted and ready to land.Jun 4 2017, 7:45 AM

I don't have commit access, could someone commit this for me please.

This revision was automatically updated to reflect the committed changes.