This is an archive of the discontinued LLVM Phabricator instance.

InstCombine: Fold is.fpclass(x, fcInf) to fabs+fcmp
ClosedPublic

Authored by arsenm on Aug 29 2023, 4:45 AM.

Details

Summary

This is a better canonical form. fcmp and fabs are more widely
understood and fabs can fold for free into some sources.

Addresses todo from D146170

Diff Detail

Event Timeline

arsenm created this revision.Aug 29 2023, 4:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2023, 4:45 AM
arsenm requested review of this revision.Aug 29 2023, 4:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2023, 4:45 AM
Herald added a subscriber: wdng. · View Herald Transcript
kpn accepted this revision.Aug 29 2023, 11:54 AM

LGTM.

This revision is now accepted and ready to land.Aug 29 2023, 11:54 AM

If ISD::IS_FPCLASS is legal or custom on some targets, do we still need the transform?

For example on riscv64,

// test.c
_Bool check_isfpclass_inf(float x) {
  return __builtin_isfpclass(x, 516);
}

clang -S -O2 test.c --target=riscv64 -march=rv64g -o -

Before this change, fclass.s is generated:

fclass.s        a0, fa0
andi    a0, a0, 129
snez    a0, a0
ret

After this change, one more insn is generated:

fabs.s  fa5, fa0
lui     a0, 522240
fmv.w.x fa4, a0
feq.s   a0, fa5, fa4
ret

If ISD::IS_FPCLASS is legal or custom on some targets, do we still need the transform?

Regardless of codegen I think this is the better canonical form. Codegen should implement the reverse transform based on target preference