[PATCH] D48149: Expand constrained FP POWI

[Cameron McInally via Phabricator via llvm-commits]

cameron.mcinally added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:729
     return ExpandCTTZ_ZERO_UNDEF(Op);
   case ISD::STRICT_FSQRT:
   case ISD::STRICT_FMA:
----------------
craig.topper wrote:
> Should STRICT_FMUL/DIV/ADD/SUB be here even though I don't know how to test them? If some target did it someday, we would end up the default case which would do the wrong thing.
Ah, you're right. That's a typo.

There could definitely be a RISC-like target without vector arith ops.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:1154
                                  EltVT, Op.getOperand(j), Idx);
       Opers.push_back(Oper);
     }
----------------
craig.topper wrote:
> Why couldn't we just detect that the type of Op.getOperand(j) is a scalar type and skip the extract? Then you could handle POWI in in this code without the separate function. Maybe with some asserts to ensure that type is vector everything but the last operand of POWI.
Good call. I was hung up on checking the FPOWI opcode and missed the obvious...


================
Comment at: test/CodeGen/X86/vector-constrained-fp-intrinsics.ll:337
 
 declare <2 x double> @llvm.experimental.constrained.fdiv.v2f64(<2 x double>, <2 x double>, metadata, metadata)
 declare <2 x double> @llvm.experimental.constrained.fmul.v2f64(<2 x double>, <2 x double>, metadata, metadata)
----------------
craig.topper wrote:
> FMA is missing? I didn't catch it in the other patch.
I'll add those...


================
Comment at: test/CodeGen/X86/vector-constrained-fp-intrinsics.ll:342
 declare <2 x double> @llvm.experimental.constrained.sqrt.v2f64(<2 x double>, metadata, metadata)
 declare <4 x double> @llvm.experimental.constrained.sqrt.v4f64(<4 x double>, metadata, metadata)
 declare <2 x double> @llvm.experimental.constrained.pow.v2f64(<2 x double>, <2 x double>, metadata, metadata)
----------------
craig.topper wrote:
> sqrt.v4f64 doesn't appear to be used.
It's debugging junk. I'll remove it.


Repository:
  rL LLVM

https://reviews.llvm.org/D48149