fixup! ARM/Decoder: Misc fixes

src/core/arm/decoder/arm.cpp

@@ -44,18 +44,19 @@ namespace Impl {
         std::array<u32, NumArgs> masks = {};
         std::array<size_t, NumArgs> shifts = {};
         Function fn = nullptr;
-        virtual void visit(Visitor *v, u32 inst) override {
+        void visit(Visitor *v, u32 inst) override {
             std::array<u32, NumArgs> values;
-            for (size_t i = 0; i < NumArgs; i++) {
-                values[i] = (inst & masks[i]) >> shifts[i];
-            }
+            std::transform(masks.begin(), masks.begin() + NumArgs, shifts.begin(), values.begin(),
+                [inst](u32 mask, size_t shift) { return (inst & mask) >> shift; });
             call<NumArgs>(v, fn, values);
         }
     };
 }
 
 template<size_t NumArgs, typename Function>
-static std::unique_ptr<ArmMatcher> MakeMatcher(const char format[32], Function fn) {
+static std::unique_ptr<ArmMatcher> MakeMatcher(const char* const format, Function fn) {
+    ASSERT(strlen(format) == 32);
+
     auto ret = Common::make_unique<Impl::MatcherImpl<NumArgs, Function>>();
     ret->fn = fn;
     ret->masks.fill(0);
@@ -64,19 +65,23 @@ static std::unique_ptr<ArmMatcher> MakeMatcher(const char format[32], Function f
     char ch = 0;
     int arg = -1;
 
-    for (int i = 0; i < 32; i++) {
-        const u32 bit = 1 << (31 - i);
+    for (size_t i = 0; i < 32; i++) {
+        const size_t bit_position = 31 - i;
+        const u32 bit = 1 << bit_position;
 
         if (format[i] == '0') {
+            // 0: A zero must be found here
             ret->bit_mask |= bit;
             ch = 0;
             continue;
         } else if (format[i] == '1') {
+            // 1: A one must be found here
             ret->bit_mask |= bit;
             ret->expected |= bit;
             ch = 0;
             continue;
         } else if (format[i] == '-') {
+            // -: Ignore this bit
             ch = 0;
             continue;
         }
@@ -86,14 +91,17 @@ static std::unique_ptr<ArmMatcher> MakeMatcher(const char format[32], Function f
         ASSERT(format[i] != 'l');
         ASSERT(format[i] != 'O');
 
-        if (format[i] != ch){
+        // otherwise: This bit is part of a field to extract
+
+        // strings of the same character make up a field
+        if (format[i] != ch) {
             arg++;
             ASSERT(arg < NumArgs);
             ch = format[i];
         }
 
         ret->masks[arg] |= bit;
-        ret->shifts[arg] = 31 - i;
+        ret->shifts[arg] = bit_position;
     }
 
     ASSERT(arg == NumArgs - 1);

src/core/arm/decoder/decoder.h

@@ -23,14 +23,14 @@ class ThumbInstruction;
 class Visitor;
 
 /**
- * This funtion identifies an ARM instruction and returns the relevant ArmInstruction.
- * Returns boost::none if the instruction could not be deocoded.
+ * This function identifies an ARM instruction and returns the relevant ArmInstruction.
+ * Returns boost::none if the instruction was not recognised.
  */
 boost::optional<const ArmInstruction&> DecodeArm(u32 instruction);
 
 /**
-* This funtion identifies a Thumb instruction and returns the relevant ThumbInstruction.
-* Returns boost::none if the instruction could not be deocoded.
+* This function identifies a Thumb instruction and returns the relevant ThumbInstruction.
+* Returns boost::none if the instruction was not recognised.
 */
 boost::optional<const ThumbInstruction&> DecodeThumb(u16 instruction);
 
@@ -91,7 +91,7 @@ public:
         return name;
     }
 
-    bool Match(u32 instruction) const {
+    bool Match(u16 instruction) const {
         return matcher.Match(instruction);
     }
 

src/core/arm/decoder/thumb.cpp

@@ -22,22 +22,19 @@ ThumbMatcher MakeMatcher(const char* const str, std::function<void(Visitor* v, u
     u16 mask = 0;
     u16 expect = 0;
 
-    for (int i = 0; i < 16; i++) {
-        mask <<= 1;
-        expect <<= 1;
+    for (size_t i = 0; i < 16; i++) {
+        const size_t bit_position = 15 - i;
+        const u16 bit = 1 << bit_position;
 
         switch (str[i]) {
         case '0':
-            mask |= 1;
-            expect |= 0;
+            mask |= bit;
             break;
         case '1':
-            mask |= 1;
-            expect |= 1;
+            mask |= bit;
+            expect |= bit;
             break;
         default:
-            mask |= 0;
-            expect |= 0;
             break;
         }
     }