This is an archive of the discontinued LLVM Phabricator instance.

[LegalizeDAG] Fix FCOPYSIGN expansion
ClosedPublic

Authored by lliu0 on Jul 29 2018, 11:32 PM.

Details

Summary

In expansion of FCOPYSIGN, the shift node is missing when the two operands of FCOPYSIGN are of the same size. We should always generate shift node (if the required shift bit is not zero) to put the sign bit into the right position, regardless of the size of underlying types.

Diff Detail

Repository
rL LLVM

Event Timeline

lliu0 created this revision.Jul 29 2018, 11:32 PM
MatzeB accepted this revision.Jul 30 2018, 11:43 AM

LGTM, nice catch!

lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
1492–1496

How about using this pattern here (functionality is the same):

EVT ShiftVT = IntVT;
if (SignBit.getValueSizeInBits() < ClearedSign.getValueSizeInBits()) {
  SignBit = DAG.getNode(ISD::ZERO_EXTEND, DL, MagVT, SignBit);
  ShiftVT = MagVT;
}
This revision is now accepted and ready to land.Jul 30 2018, 11:43 AM
This revision was automatically updated to reflect the committed changes.
lliu0 added a comment.Aug 1 2018, 6:55 PM

Thanks. I have checked in a fix with your sugguested change.

hans added a subscriber: hans.Aug 2 2018, 2:10 AM

This landed shortly after the 7.0 release branch was created. Should we merge it?

bogner added a comment.Aug 3 2018, 2:36 PM

This landed shortly after the 7.0 release branch was created. Should we merge it?

I think we should merge this.

hans added a comment.Aug 6 2018, 11:23 PM

This landed shortly after the 7.0 release branch was created. Should we merge it?

I think we should merge this.

Merged in r339098. Thanks!