[PATCH] D22751: AMDGPU Device Libs pass.

[Matt Arsenault via llvm-commits]

arsenm added inline comments.

================
Comment at: lib/Target/AMDGPU/AMDGPUDeviceLibsPass.cpp:59-64
@@ +58,8 @@
+  if (IsAttributeSet(F, "unsafe-fp-math")) { mask |= UNSAFE_FP_MATH; }
+  if (IsAttributeSet(F, "no-nans-fp-math")) { mask |= NO_NANS_FP_MATH; }
+  if (IsAttributeSet(F, "no-infs-fp-math")) { mask |= NO_INFS_FP_MATH; }
+  const SISubtarget* ST = static_cast<const SISubtarget*>(TM->getSubtargetImpl(F));
+  if (ST->hasFastFMAF32()) { mask |= FAST_FMA32; }
+  AMDGPU::IsaVersion ISA = ST->getIsaVersion();
+  mask |= (ISA.Major * 100 + ISA.Minor * 10 + ISA.Stepping) << ISA_VERSION_SHIFT;
+
----------------
if bodies on separate lines

================
Comment at: lib/Target/AMDGPU/AMDGPUDeviceLibsPass.cpp:142-146
@@ +141,7 @@
+
+  NF->setCallingConv(F->getCallingConv());
+  if (F->hasGC())
+    NF->setGC(F->getGC());
+  else
+    NF->clearGC();
+
----------------
Why do we have to handle this GC bit? needs tests

================
Comment at: lib/Target/AMDGPU/AMDGPUDeviceLibsPass.cpp:163-180
@@ +162,20 @@
+  Instruction *NCall;
+  if (InvokeInst *II = dyn_cast<InvokeInst>(Call)) {
+    InvokeInst *NII = InvokeInst::Create(Callee, II->getNormalDest(),
+                                         II->getUnwindDest(),
+                                         Args, "", Call);
+    NII->setCallingConv(II->getCallingConv());
+    NII->setAttributes(NPAL);
+
+    NCall = NII;
+  } else {
+    CallInst *CI = cast<CallInst>(Call);
+    CallInst *NCI = CallInst::Create(Callee, Args, "", Call);
+    NCI->setCallingConv(CI->getCallingConv());
+    NCI->setAttributes(NPAL);
+    if (CI->isTailCall())
+      NCI->setTailCall();
+
+    NCall = NCI;
+  }
+  NCall->setDebugLoc(Call->getDebugLoc());
----------------
Should make common with CallSite. Why does invoke need to be handled at all? No library function should ever throw 

================
Comment at: lib/Target/AMDGPU/AMDGPUDeviceLibsPass.cpp:204-205
@@ +203,4 @@
+  while (!Worklist.empty()) {
+    Function* F = *Worklist.begin();
+    Worklist.erase(F);
+    Changed = false;
----------------
pop_back_val?


https://reviews.llvm.org/D22751