[PATCH] D156707: AMDGPU: Handle multiple uses when matching sincos

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added a comment.

Do you think it's worth running a pass like InstSimplify after the tests to check the instruction does get eliminated?
Maybe with a different CHECK line?

Otherwise I think it's error prone, if someone makes a change that makes the inst not dead anymore all we would see in the test diff is an operand that's renamed



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp:94
 
-  bool insertSinCos(CallInst *Sin, CallInst *Cos, IRBuilder<> &B,
-                    const FuncInfo &FInfo);
+  std::tuple<Value *, Value *, Value *> insertSinCos(Value *Arg,
+                                                     FastMathFlags FMF,
----------------
nit: add docs, or use a small struct, otherwise it's difficult to tell what the tuple contains


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp:1249-1260
+    bool Handled = false;
+
+    if (UCallee->getName() == SinName) {
+      Handled = true;
+      SinCalls.push_back(XI);
+    } else if (UCallee->getName() == CosName) {
+      Handled = true;
----------------
small nit: avoids repetition


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

https://reviews.llvm.org/D156707