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

[Cameron McInally via Phabricator via llvm-commits]

cameron.mcinally marked an inline comment as done.
cameron.mcinally added inline comments.


================
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:
> cameron.mcinally wrote:
> > 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.
> But still, I don't see any place where common code would even call the back-end's getOperationAction on a STRICT_FSETCC.
> 
> E.g. when calling getStrictFPOperationAction with a STRICT_FSETCC, it will actually call the back-end with SETCC (not STRICT_FSETCC).  So this should already return Custom for you ...
> 
>But still, I don't see any place where common code would even call the back-end's getOperationAction on a STRICT_FSETCC.

I agree with this statement. We call getStrictFPOperationAction(...) which will return Custom from SETCC (for the STRICT_FSETCC node).

> E.g. when calling getStrictFPOperationAction with a STRICT_FSETCC, it will actually call the back-end with SETCC (not STRICT_FSETCC). So this should already return Custom for you ...

I also agree with this, but when the backend returns Custom for SETCC, we have to then Custom lower the STRICT_FSETCC node. We do not mutate the STRICT_FSETCC node into SETCC node in LegalizeOp(), where we call getStrictFPOperationAction().

I'm not sure I fully understand your point though (although I may admittedly be making a fundamental error). If you comment out this line in the code, you'll see the backtrace leading up to the problem.



Repository:
  rL LLVM

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

https://reviews.llvm.org/D54649