[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