[PATCH] D54121: [FPEnv] Add constrained FCMP intrinsic

[Cameron McInally via Phabricator via llvm-commits]

cameron.mcinally added inline comments.


================
Comment at: include/llvm/IR/Intrinsics.td:600
+                                                      llvm_metadata_ty,
+                                                      llvm_metadata_ty ]>;
 }
----------------
This isn't quite right. The result's vector width should be tied to the operands' vector width. Something like:

```
def int_experimental_constrained_fcmp : Intrinsic<[ llvm_anyint_ty ],
                                                    [ llvm_i8_ty,
                                                      LLVMVectorSameWidth<0, llvm_anyscalarfloat_ty>,
                                                      LLVMMatchType<1>,
                                                      llvm_metadata_ty,
                                                      llvm_metadata_ty ]>;
```

We would need to accommodate vectors of floats and doubles here, but TableGen isn't expressive enough to do that right now. 

Should we extend TableGen to handle a situation like this? That's assuming the "one compare intrinsic" solution I'm proposing is what we decide on, of course. 


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:772
     return ExpandStrictFPOp(Op);
   default:
     return DAG.UnrollVectorOp(Op.getNode());
----------------
ExpandStrictFPOp(...) and friends would need to be updated to handle FCmp's operand types. The operand types do not match the result type, as the other constrained intrinsics do.


Repository:
  rL LLVM

https://reviews.llvm.org/D54121