[PATCH] D70226: Add an option to disable strict float node mutating to an normal float node

Ulrich Weigand via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 20 09:38:32 PST 2019


uweigand added a comment.

> X86 has FP_ROUND marked Custom, but most type combinations are Legal. I had to mark STRICT_FP_ROUND as Custom to get it past this code. But now I can’t get it past the mutation code in SelectionDAGIsel because it’s not “Legal”.

Ah, so you mark STRICT_FP_ROUND Custom, but in the custom expander still leave it as STRICT_FP_ROUND, expecting it to be matched?  I see.  I believe this code in SelectionDAGISel:

  if (Node->isStrictFPOpcode() &&
      (TLI->getOperationAction(Node->getOpcode(), Node->getValueType(0))
       != TargetLowering::Legal))

should really be:

  if (Node->isStrictFPOpcode() &&
      (TLI->getOperationAction(Node->getOpcode(), Node->getValueType(0))
       == TargetLowering::Expand))

That should fix your problem.   (Or else, once we get this patch committed, you could also set isStrictFPEnabled for your target, and the problem would also be gone.)

> Scalar FADD on X86 is also marked Custom but most cases go through unmodified. STRICT_FADD is marked Expand currently. And only doesn’t get turned into a lib call because I don’t think there is STRICT_FADD libcall support yet. But that needs to be added to support strict ops on f128 for X86-64. The moment that happens then every other target that hasn’t implemented strict fp yet will generate a libcall for STRICT_FADD.

Right.  But as I said, I personally would prefer this behavior: at least the compiler doesn't silently ignore strict semantics that it promised to implement ...


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

https://reviews.llvm.org/D70226





More information about the llvm-commits mailing list