[PATCH] D54649: [FPEnv] Rough out constrained FCmp intrinsics

[Cameron McInally via Phabricator via llvm-commits]

cameron.mcinally added inline comments.


================
Comment at: include/llvm/CodeGen/ValueTypes.td:153
+// Pseudo valuetype to represent "bool of any format"
+def bAny  : ValueType<0  , 249>;
 
----------------
I'm not sure if it's safe to reorder these values. I had to cluster these entries together since there's a Duff's device in `utils/TableGen/IntrinsicEmitter.cpp` that depends on it.

Unfortunately, the new bAny entry could not be added to the end of the *Any defs list, since the last value is 255 and the container is an 8-bit unsigned int.


================
Comment at: include/llvm/IR/Intrinsics.td:163
+  LLVMType ElTy = elty;
+}
 class LLVMPointerTo<int num> : LLVMMatchType<num>;
----------------
This should probably be more strict. We could end up with something like <2 <2 x double>> as an argument. There's some C++ code later that should catch that case, but we could probably come up with a better class to extend here.


================
Comment at: include/llvm/Support/MachineValueType.h:219
+      // This is only for tblgen's consumption!
+      bAny          = 249,
 
----------------
This is the same value reordering mentioned in the comment for `include/llvm/CodeGen/ValueTypes.td`.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:1231
       LLVM_FALLTHROUGH;
+    }
     case TargetLowering::Expand:
----------------
Has anyone given any thought to how we will Custom lower Strict nodes? This is punting too soon, but I wanted to get the code out there to review before the changeset became too large.


Repository:
  rL LLVM

https://reviews.llvm.org/D54649