[PATCH] D139902: IR: Add nofpclass parameter attribute

[Nikita Popov via Phabricator via llvm-commits]

nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/include/llvm/IR/Attributes.h:21
 #include "llvm/ADT/BitmaskEnum.h"
+#include "llvm/ADT/FloatingPointMode.h"
 #include "llvm/ADT/SmallString.h"
----------------
Please forward-declare instead.


================
Comment at: llvm/include/llvm/IR/Attributes.h:152
   static Attribute getWithMemoryEffects(LLVMContext &Context, MemoryEffects ME);
+  static Attribute getWithNoFPClass(LLVMContext &Context, unsigned Mask);
 
----------------
Should accept `FPClassTest` instead?


================
Comment at: llvm/include/llvm/IR/Attributes.h:254
 
+  // Return the FPClassTest for nofpclass
+  FPClassTest getNoFPClass() const;
----------------
`///`


================
Comment at: llvm/include/llvm/IR/Attributes.h:890
+  /// Get the disallowed floating-point classes of the argument value.
+  FPClassTest getParamNoFPClass(unsigned Index) const;
+
----------------
`Index` -> `ArgNo`


================
Comment at: llvm/include/llvm/IR/Attributes.h:1252
+  // Add nofpclass attribute
+  AttrBuilder &addNoFPClassAttr(unsigned NoFPClassMask);
+
----------------
Same here, pass `FPClassTest` at API level?


================
Comment at: llvm/include/llvm/IR/InstrTypes.h:1838
+  /// return value.
+  FPClassTest getRetNoFPClass() const {
+    unsigned Mask = Attrs.getRetNoFPClass();
----------------
Move to C++ file.


================
Comment at: llvm/include/llvm/IR/InstrTypes.h:1843
+    if (const Function *F = getCalledFunction())
+      Mask |= static_cast<unsigned>(F->getAttributes().getRetNoFPClass());
+    return static_cast<FPClassTest>(Mask);
----------------
Unrelated, but why isn't it a bitmask enum?


================
Comment at: llvm/lib/IR/Attributes.cpp:472
+      if (EmitSeparator)
+        OS << ' ';
+      OS << Name;
----------------
Use `ListSeparator` instead.


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

https://reviews.llvm.org/D139902