[PATCH] D22751: AMDGPU Device Libs pass.

[Tom Stellard via llvm-commits]

tstellarAMD added inline comments.

================
Comment at: lib/Target/AMDGPU/AMDGPUDeviceLibs.h:1
@@ +1,2 @@
+#ifndef AMDGPU_DEVICE_LIBS_H
+#define AMDGPU_DEVICE_LIBS_H
----------------
Header comment is missing.

================
Comment at: lib/Target/AMDGPU/AMDGPUDeviceLibs.h:5
@@ +4,3 @@
+enum AMDGPUOptionMask {
+  AMD_OPT = 1,
+  UNSAFE_FP_MATH = 2,
----------------
Can you add a comment explaining what this option is for.

================
Comment at: lib/Target/AMDGPU/AMDGPUDeviceLibsPass.cpp:1
@@ +1,2 @@
+#include "AMDGPU.h"
+#include "llvm/IR/Constants.h"
----------------
This file needs a header comment with an explanation of what it does.

================
Comment at: lib/Target/AMDGPU/AMDGPUDeviceLibsPass.cpp:38
@@ +37,3 @@
+  uint64_t mask = 0;
+  mask |= AMD_OPT; // Always set.
+  if (IsAttributeSet(F, "unsafe-fp-math")) { mask |= UNSAFE_FP_MATH; }
----------------
Do we want to set this even at -O0 ?

================
Comment at: lib/Target/AMDGPU/AMDGPUDeviceLibsPass.cpp:57
@@ +56,3 @@
+                                              ArrayRef<NewArgInfo> NArgs)
+{
+  unsigned NumNArgs = NArgs.size();
----------------
Coding style: brace shouldn't be on its own line.

================
Comment at: lib/Target/AMDGPU/AMDGPUDeviceLibsPass.cpp:76
@@ +75,3 @@
+                                    unsigned Shift)
+{
+  SmallVector<AttributeSet, 8> Attributes;
----------------
Coding Style.

================
Comment at: lib/Target/AMDGPU/AMDGPUDeviceLibsPass.cpp:90
@@ +89,3 @@
+                                         ArrayRef<NewArgInfo> NArgs)
+{
+  unsigned NumNArgs = NArgs.size();
----------------
Coding Style.

================
Comment at: lib/Target/AMDGPU/AMDGPUDeviceLibsPass.cpp:137
@@ +136,3 @@
+                            Value *Callee)
+{
+  Instruction *Call = CS.getInstruction();
----------------
Coding Style.


https://reviews.llvm.org/D22751