Since ISA 3.0, we have test data class instruction (x(s|v)tstdc(s|d|q)p) to classify a floating point value, which can be used to combine common FP classify operations like isinf or zero comparison.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Time | Test | |
---|---|---|
60,030 ms | x64 debian > libFuzzer.libFuzzer::minimize_crash.test |
Event Timeline
Thanks for working on this, I think it should be workable. However seems adding a new PPCISD would be preferable, like PPCISD::FP_CLASS? You can find same handling in AMDGPU arch FP_CLASS node.
With this new ISD:
1: we can leverage the table-gen to select the instruction.
2: we can do further combine
3: maybe this node can also be used for the fp class intrinsics like int_ppc_test_data_class_f and int_ppc_test_data_class_d?
llvm/lib/Target/PowerPC/PPCISelLowering.cpp | ||
---|---|---|
14145 | Can we make these enums be ordered? | |
14159 | Any reason we don't handle NAN and denormal inputs? |
llvm/lib/Target/PowerPC/PPCISelLowering.cpp | ||
---|---|---|
14353 | I have same concern as previous patch: any reason for the nan and negdenormal/posdenormal are excluded here? |
llvm/lib/Target/PowerPC/PPCISelLowering.cpp | ||
---|---|---|
14353 | isnan uses cond code SETUO against arbitary non-nan number, whose codegen is also a single fcmpu. C std or LLVM intrinsics don't have issubnormal/isdenormal. For isnormal, the pattern is (abs(x) uge CONSTANT1) and (abs(x) olt CONSTANT2). Optimizing it in backend may be profitable, but (1) is out of scope of this patch; (2) clang will directly generate is_fpclass call in the future |
Actually, now that I've looked over the whole patch, it is not clear to me why this is in the PPC back end? There doesn't seem to be any PPC-specific requirements here. Why is it not OK to put the combine in DAGCombiner.cpp and if we don't want the combine to fire on all targets/subtargets, we can just add TargetLoweringInfo::shouldCombineToIsFPClass().
I've requested changes to require that either this be moved to target independent code or a justification be provided as to why this is PPC specific.
llvm/lib/Target/PowerPC/PPCISelLowering.cpp | ||
---|---|---|
14337 | Nit: name this something like IsFABS or IsAbsVal (my preference would be the first one). |
Can we make these enums be ordered?