This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Support constrained fp operation for setcc
ClosedPublic

Authored by steven.zhang on Jun 12 2020, 3:05 AM.

Details

Summary

The constrained fp operation fcmp was added by https://reviews.llvm.org/D69281. This patch is trying to add the support for PowerPC backend. For now, only scalar type is supported.

Diff Detail

Event Timeline

steven.zhang created this revision.Jun 12 2020, 3:05 AM
uweigand requested changes to this revision.Jun 30 2020, 2:59 AM
uweigand added inline comments.
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
409

Shouldn't all f128 support be guarded by EnableQuadPrecision?

llvm/lib/Target/PowerPC/PPCInstrInfo.td
3896

This looks incorrect; you cannot use the same FCMPUS operation for both signaling and quiet compares. If I read the PowerISA correctly, the "fcmpo" instruction (not "fcmpu") is the correct one to use to implement signaling compares.

This revision now requires changes to proceed.Jun 30 2020, 2:59 AM
steven.zhang planned changes to this revision.Jul 2 2020, 3:51 AM

Address reviewer's great comments.

steven.zhang marked 2 inline comments as done.Jul 16 2020, 2:37 AM
steven.zhang added inline comments.
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
409

Yes, it should be. But as this option has been removed, we don't need it anymore.

llvm/lib/Target/PowerPC/PPCInstrInfo.td
3896

You are completely right. Thank you for point out this.

uweigand accepted this revision.Jul 16 2020, 5:19 AM

The one issue I still see is that when falling back to library calls on older platforms, signaling and non-signaling compares both are treated the same. But that's really a pre-existing problem because this is not fully supported by libgcc (see the comment in TargetLowering::softenSetCCOperands), so it's not really a problem with this patch.

The patch itself LGTM.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
409

Ah, I missed that. OK, that's fine then.

This revision is now accepted and ready to land.Jul 16 2020, 5:19 AM
This revision was landed with ongoing or failed builds.Aug 6 2020, 10:18 PM
This revision was automatically updated to reflect the committed changes.
llvm/lib/Target/PowerPC/PPCInstrInfo.td