[PATCH] Patch that adds address-masking instructions before loads and stores, and after the instructions that change stack-pointer.

Mark Seaborn mseaborn at chromium.org
Sun Mar 9 11:23:49 PDT 2014


  LGTM, thanks


================
Comment at: lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp:99
@@ -69,2 +98,3 @@
   virtual void EmitInstruction(const MCInst &Inst, const MCSubtargetInfo &STI) {
-    if (isIndirectJump(Inst))
+    unsigned AddrIdx;
+    bool IsStore;
----------------
Nit: It would be more usual C++ style to declare these immediately before they're used...

================
Comment at: lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp:111
@@ +110,3 @@
+    // Sandbox loads, stores and SP changes.
+    IsMemAccess = isBasePlusOffsetMemoryAccess(Inst.getOpcode(), &AddrIdx,
+                                               &IsStore);
----------------
Can be "bool IsMemAccess = ..."

================
Comment at: lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp:137
@@ +136,3 @@
+                                  bool *IsStore) {
+  if (IsStore != NULL)
+    *IsStore = false;
----------------
I think usual LLVM style is to omit "!= NULL"

================
Comment at: lib/Target/Mips/MipsDelaySlotFiller.cpp:19
@@ -18,2 +18,3 @@
 #include "MipsTargetMachine.h"
+#include "MCTargetDesc/MipsMCNaCl.h"
 #include "llvm/ADT/BitVector.h"
----------------
Nit: sort #includes?

================
Comment at: test/MC/Mips/nacl-mask.s:215
@@ +214,3 @@
+# For stores where $sp is destination and base, check that mask is added neither
+# before not after.
+
----------------
"not" -> "nor"


http://llvm-reviews.chandlerc.com/D2904



More information about the llvm-commits mailing list