Page MenuHomePhabricator

[SelectionDAG] Remove unused FP constant in getNegatedExpression
ClosedPublic

Authored by qiucf on Sep 14 2020, 7:52 AM.

Details

Summary

960cbc53 immediately removes nodes that won't be used to avoid compilation time explosion. This patch adds the removal to constants to fix PR47517.

Diff Detail

Event Timeline

qiucf created this revision.Sep 14 2020, 7:52 AM
qiucf requested review of this revision.Sep 14 2020, 7:52 AM

Have you tried to reduce the test case any further?

qiucf updated this revision to Diff 291598.Sep 14 2020, 9:57 AM

Reduce test case

steven.zhang added a reviewer: Restricted Project.Sep 14 2020, 5:47 PM
steven.zhang added inline comments.Sep 14 2020, 5:54 PM
llvm/test/CodeGen/X86/pr47517.ll
2

Please update the test with FileCheck

qiucf updated this revision to Diff 291801.Sep 14 2020, 10:57 PM
qiucf marked an inline comment as done.

Update test using script

steven.zhang accepted this revision.Sep 14 2020, 11:57 PM

LGTM and thank you for doing this. But please for a while to see if RKSimon has more comments.

This revision is now accepted and ready to land.Sep 14 2020, 11:57 PM
RKSimon accepted this revision.Sep 15 2020, 1:52 AM

LGTM

qiucf added a comment.Sep 15 2020, 6:06 AM

After running sanitizer, I think the bug is not totally covered by this patch.

In D86689, we introduced such pattern:

// Negate the X if its cost is less or equal than Y.
if (NegX && (CostX <= CostY)) {
  Cost = CostX;
  SDValue N = DAG.getNode(ISD::FSUB, DL, VT, NegX, Y, Flags);
  RemoveDeadNode(NegY);
  return N;
}

Somewhere in this test case, N=3.0, NegY=3.0, NegY->use_size()=0, so NegY is just deleted but N also gets deleted.