These instructions are significantly faster than the corresponding compare/select code.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/Target/PowerPC/PPCInstrAltivec.td | ||
---|---|---|
931 ↗ | (On Diff #148430) | Hi Nemanja, I have one concern on whether these two hardware instructions for vector float point can be perfectly mapped to these two ISDNode. |
lib/Target/PowerPC/PPCInstrVSX.td | ||
1507 ↗ | (On Diff #148430) | I'm not sure why not use similar patterns like VMAXSW in PPCInstrAltivec.td but being located in HasP8Vector scope? Is there some special reasons with COPY_TO_REGCLASS? |
test/CodeGen/PowerPC/vec-min-max.ll | ||
53 ↗ | (On Diff #148430) | Some more cases cover sge/sle seems trivial but the coverage is better? |
@jedilyn Hi Ke Wen, thanks for your comments. This needs some cleanup with regard to which instructions match the semantics of F{MIN|MAX}NUM vs. F{MIN|MAX}NAN. I'll clean that up and re-post this for your review. Thanks.
lib/Target/PowerPC/PPCInstrAltivec.td | ||
---|---|---|
931 ↗ | (On Diff #148430) | I am really sorry about such a long delay in responding to this. You are absolutely right. The correct nodes are FMINNAN and FMAXNAN. I think what I meant to use here are XVMAXDP, XVMINDP, XVMAXSP, XVMINSP. Those have the mentioned semantics (i.e. comparing a value and a NaN returns the value). And I don't think we need to worry about signalling NaNs at this time. |
lib/Target/PowerPC/PPCInstrVSX.td | ||
1507 ↗ | (On Diff #148430) | These instructions are new in ISA 2.07 so they have to be in the P8Vector block. Also, the COPY_TO_REGCLASS is needed because VRRC registers cannot contain v2i64 values. |
This covers PR39130 right?
Maybe worth adding the new vec-min-max.ll test file to trunk with current codegen so this patch shows the improved codegen diff?
test/CodeGen/PowerPC/vec-min-max.ll | ||
---|---|---|
5 ↗ | (On Diff #148430) | You might be better using common prefixes to share (and reduce) checks. --check-prefixes=CHECK,P8VEC |
Yes, this will fix the PR for vector types. Scalar types will come later.
test/CodeGen/PowerPC/vec-min-max.ll | ||
---|---|---|
5 ↗ | (On Diff #148430) | Oh cool. I was not aware of this functionality. Thank you. |
Updated to remove the patterns for the Altivec versions of vector min/max as they have IEEE semantics wrt. handling NaN. A subsequent patch will legalize the _IEEE versions of the nodes for single precision and provide patterns to match them to vmaxfp/vminfp.
Is anything happening with this? We've hit PPC issues with ISD::ABS on https://reviews.llvm.org/D49837 and I noticed that PPCTargetLowering::LowerABS has a reference to this patch.