[PATCH] [CodeGen] Introduce a FAULTING_LOAD_OP pseudo-op.

Pat Gavlin pagavlin at microsoft.com
Tue Jun 9 09:56:07 PDT 2015


================
Comment at: include/llvm/CodeGen/FaultMaps.h:2
@@ +1,3 @@
+//===------------------- FaultMaps.h - StackMaps ----------------*- C++ -*-===//
+
+//
----------------
Extra blank line.

================
Comment at: include/llvm/CodeGen/FaultMaps.h:15
@@ +14,3 @@
+#include "llvm/ADT/DenseMap.h"
+
+#include <vector>
----------------
Extra blank line.

================
Comment at: include/llvm/CodeGen/FaultMaps.h:54
@@ +53,3 @@
+
+  typedef std::vector<FaultInfo> FunctionFaultingInfo;
+  DenseMap<const MCSymbol *, FunctionFaultingInfo> FunctionInfos;
----------------
Nit: s/FunctionFaultingInfo/FunctionFaultInfos/

================
Comment at: lib/CodeGen/FaultMaps.cpp:11
@@ +10,3 @@
+#include "llvm/CodeGen/FaultMaps.h"
+
+#include "llvm/CodeGen/AsmPrinter.h"
----------------
Extra blank line.

================
Comment at: lib/CodeGen/FaultMaps.cpp:75-86
@@ +74,14 @@
+
+  // We emit the functions sorted by name for easier testing.
+
+  std::vector<const MCSymbol *> FunctionNames;
+  FunctionNames.reserve(FunctionInfos.size());
+
+  for (const auto &FFI : FunctionInfos)
+    FunctionNames.push_back(FFI.first);
+
+  std::sort(FunctionNames.begin(), FunctionNames.end(),
+            [](const MCSymbol *L, const MCSymbol *R) {
+              return L->getName() < R->getName();
+            });
+
----------------
What specifically about testing are you trying to make easier here? Are you just looking for determinism in the output order? If so, perhaps it would be better if `FunctionInfos` was a `std::map`, which should give you this property without the extra work.

FWIW, I think determinism here is useful beyond the scope of testing, so it would be nice to be able to provide it relatively cheaply.

================
Comment at: lib/MC/MCObjectFileInfo.cpp:523
@@ -516,3 +522,3 @@
 
 void MCObjectFileInfo::InitCOFFMCObjectFileInfo(Triple T) {
   bool IsWoA = T.getArch() == Triple::arm || T.getArch() == Triple::thumb;
----------------
Could you also add code to initialize `FaultMapSection` for COFF objects?

================
Comment at: lib/Target/X86/X86AsmPrinter.cpp:686
@@ -685,3 +686,3 @@
 
   if (TT.isOSBinFormatCOFF()) {
     // Necessary for dllexport support
----------------
Please serialize the fault map section for COFF objects as well.

http://reviews.llvm.org/D10197

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list