[PATCH] D64746: Add constrained intrinsics for lrint and lround

[Kevin P. Neal via Phabricator via llvm-commits]

kpn added inline comments.


================
Comment at: lib/CodeGen/TargetLoweringBase.cpp:784
+  // is an integer type.
+  for (unsigned I = MVT::FIRST_INTEGER_VALUETYPE;
+       I <= MVT::LAST_INTEGER_VALUETYPE;
----------------
craig.topper wrote:
> What places check the result? Can they be fixed?
Hmmm, in LegalizeDAG.cpp we're checking the value in LegalizeOp around line 1153. We're checking the value in ExpandNode around 3692. 

If these aren't libcalls then we'll do the mutation in SelectionDAGISel.cpp, but that checks the value as well. And it comes too late for mutating if it will become a libcall. That's what i'm seeing: a failure during instruction selection because mutation to LRINT happened too late.

I'm not sure, though, that they are broken. Operation actions don't seem to be defined exclusively for operands or exclusively for values. And there's oodles of code checking operands, but oodles of code checking values. Some by definition, since, for example, the Expand code must be looking at values. 

Anything that isn't marked will default to Legal and cause us problems. I'm not sure we want to be playing whack-a-mole forever adding exceptions here and there. This is a bit of a shotgun approach, but it does cover all the bases we need covered.

I'll experiment a bit more and see what if I can get this thing to behave without this block of code here.


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

https://reviews.llvm.org/D64746