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

Mon Dec 3 08:09:16 PST 2018

cameron.mcinally marked 2 inline comments as done.
cameron.mcinally added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:81
+  case ISD::STRICT_FSETCC: 
+                         Res = PromoteIntRes_StrictFSETCC(N); break;
   case ISD::SMIN:
----------------
uweigand wrote:
> Just to clarify: the reason why we need to handle STRICT_FSETCC here but not any of the other STRICT_ opcodes is that only FSETCC involves an integer type (as return type), right?
That's correct. 

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:26106
   case ISD::SETCC:              return LowerSETCC(Op, DAG);
+  case ISD::STRICT_FSETCC:      return LowerSTRICT_FSETCC(Op, DAG);
   case ISD::SETCCCARRY:         return LowerSETCCCARRY(Op, DAG);
----------------
uweigand wrote:
> Why do you have this here?   It seems that this can never be called right now (at least without something like my D45576, right?)
It actually will be called. Some SETCCs are always custom lowered on x86, even for scalars. 

The other constrained vector operations are all legal or expanded to legal scalar operations. SETCC is the first operation that needs the special lowering.

Just to be pedantic, I do realize this code is punting right now. The TableGen changes in this patch are quite involved. So I'd like for us to focus on those first. I will handle the STRICT_FSETCC peculiarities in a future patch.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54649/new/

https://reviews.llvm.org/D54649