[PATCH] D43515: More math intrinsics for conservative math handling

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/CodeGen/StrictFP.cpp:76
+
+  auto *TLI = TM->getSubtargetImpl(F)->getTargetLowering();
+
----------------
I believe you should be getting TM by doing this.

```
auto *TPC = getAnalysisIfAvailable<TargetPassConfig>();
if (!TPC)
  return false;

auto &TM = TPC->getTM<TargetMachine>();
```

There is only one other pass that takes TargetMachine in its constructor. The others use what I've put above.  So I believe that is the preferred way.


================
Comment at: lib/CodeGen/StrictFP.cpp:84
+  for (auto *I : IntrinsicWorkList) {
+    Changed |= processIntrinsicCall(Context, I);
+  }
----------------
There is little reason to pass Context around. Value has a getContext as does Type. So you can get the context easily whenever you need it. 


================
Comment at: lib/CodeGen/StrictFP.cpp:99
+  case Intrinsic::experimental_constrained_fptoui:
+    Value *IntDst = cast<Value>(I);
+    Type *IntDstType = IntDst->getType();
----------------
This cast is unnecessary. IntrinsicInst is a subclass of Value


================
Comment at: lib/CodeGen/StrictFP.cpp:101
+    Type *IntDstType = IntDst->getType();
+    EVT VT = EVT::getEVT(IntDstType, true);
+
----------------
This should use TLI->getValueType.


================
Comment at: lib/CodeGen/StrictFP.cpp:166
+
+  SmallVector<Value *, 4> Operands(I->arg_operands());
+  Value *IntDst = cast<Value>(I);
----------------
Why do you need a SmallVector here? Why can't you just call getArgOperand?


================
Comment at: lib/CodeGen/StrictFP.cpp:167
+  SmallVector<Value *, 4> Operands(I->arg_operands());
+  Value *IntDst = cast<Value>(I);
+  Value *FPSrc = Operands[0];
----------------
This cast is unecessary.


================
Comment at: lib/CodeGen/StrictFP.cpp:171
+
+  auto *t = cast<IntegerType>(I->getType());
+  APInt IntMaxAP(APInt::getSignedMinValue(DL->getTypeStoreSize(t) * 8));
----------------
What if the intrinsic uses a vector type?


https://reviews.llvm.org/D43515