Siddhesh Poyarekar has uploaded this change for review. ( https://gem5-review.googlesource.com/8541

Change subject: arm: Fix implicit-fallthrough warnings when building with gcc-7+
......................................................................

arm: Fix implicit-fallthrough warnings when building with gcc-7+

gcc 7 onwards have additional heuristics to detect implicit
fallthroughs and it fails the build with warnings for ARM as a result.
There was one gcc bug[1] that I fixed but the rest are cases that gcc
cannot detect due to the point at which it does the fallthrough check.
Most of this patch adds __builtin_unreachable() hints in places that throw
this warning to indicate to gcc that the fallthrough will never
happen.

The remaining cases are actually possible fallthroughs due to
incorrect code running on the simulator; in which case an Unknown
instruction is returned.

[1] https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01105.html

Change-Id: I1baa9fa0ed15181c10c755c0bd777f88b607c158
---
M src/arch/arm/insts/pred_inst.hh
M src/arch/arm/isa/formats/aarch64.isa
M src/arch/arm/isa/formats/data.isa
M src/arch/arm/isa/formats/fp.isa
M src/arch/arm/isa/formats/m5ops.isa
M src/arch/arm/isa/formats/mem.isa
M src/arch/arm/isa/formats/mult.isa
M src/arch/arm/isa/formats/neon64.isa
8 files changed, 154 insertions(+), 58 deletions(-)



diff --git a/src/arch/arm/insts/pred_inst.hh b/src/arch/arm/insts/pred_inst.hh
index b4186c8..ce9304f 100644
--- a/src/arch/arm/insts/pred_inst.hh
+++ b/src/arch/arm/insts/pred_inst.hh
@@ -150,7 +150,7 @@
                 break;
             }
         }
-        // Fall through, immediate encoding is invalid.
+        // Fall through
       default:
         immValid = false;
         break;
diff --git a/src/arch/arm/isa/formats/aarch64.isa b/src/arch/arm/isa/formats/aarch64.isa
index 68f6006..0718ebe 100644
--- a/src/arch/arm/isa/formats/aarch64.isa
+++ b/src/arch/arm/isa/formats/aarch64.isa
@@ -108,6 +108,8 @@
                 return new SubXImm(machInst, rdsp, rnsp, imm);
               case 0x3:
                 return new SubXImmCc(machInst, rdzr, rnsp, imm);
+              default:
+                __builtin_unreachable();
             }
           }
           case 0x4:
@@ -150,6 +152,8 @@
                 return new EorXImm(machInst, rdsp, rn, imm);
               case 0x3:
                 return new AndXImmCc(machInst, rdzr, rn, imm);
+              default:
+                __builtin_unreachable();
             }
           }
           case 0x5:
@@ -167,6 +171,8 @@
                 return new Movz(machInst, rdzr, imm16, hw * 16);
               case 0x3:
                 return new Movk(machInst, rdzr, imm16, hw * 16);
+              default:
+                __builtin_unreachable();
             }
           }
           case 0x6:
@@ -181,6 +187,8 @@
                 return new Ubfm64(machInst, rdzr, rn, immr, imms);
               case 0x3:
                 return new Unknown64(machInst);
+              default:
+                __builtin_unreachable();
             }
           case 0x7:
           {
@@ -405,6 +413,8 @@
                     }
                   }
                   break;
+                  default:
+                    __builtin_unreachable();
                 }
             } else if (bits(machInst, 25) == 0x1) {
                 uint8_t opc = bits(machInst, 24, 21);
@@ -429,8 +439,11 @@
                     if (rn != 0x1f)
                         return new Unknown64(machInst);
                     return new FailUnimplemented("dret", machInst);
+                  default:
+                    return new Unknown64(machInst);
                 }
             }
+          // Fall through
           default:
             return new Unknown64(machInst);
         }
@@ -470,6 +483,8 @@
                         return new STXRW64(machInst, rt, rnsp, rs);
                       case 0x3:
                         return new STXRX64(machInst, rt, rnsp, rs);
+                      default:
+                        __builtin_unreachable();
                     }
                   case 0x1:
                     switch (size) {
@@ -481,6 +496,8 @@
                         return new STLXRW64(machInst, rt, rnsp, rs);
                       case 0x3:
                         return new STLXRX64(machInst, rt, rnsp, rs);
+                      default:
+                        __builtin_unreachable();
                     }
                   case 0x2:
                     switch (size) {
@@ -491,6 +508,8 @@
                         return new STXPW64(machInst, rs, rt, rt2, rnsp);
                       case 0x3:
                         return new STXPX64(machInst, rs, rt, rt2, rnsp);
+                      default:
+                        __builtin_unreachable();
                     }

                   case 0x3:
@@ -502,6 +521,8 @@
                         return new STLXPW64(machInst, rs, rt, rt2, rnsp);
                       case 0x3:
                         return new STLXPX64(machInst, rs, rt, rt2, rnsp);
+                      default:
+                        __builtin_unreachable();
                     }

                   case 0x4:
@@ -514,6 +535,8 @@
                         return new LDXRW64(machInst, rt, rnsp, rs);
                       case 0x3:
                         return new LDXRX64(machInst, rt, rnsp, rs);
+                      default:
+                        __builtin_unreachable();
                     }
                   case 0x5:
                     switch (size) {
@@ -525,6 +548,8 @@
                         return new LDAXRW64(machInst, rt, rnsp, rs);
                       case 0x3:
                         return new LDAXRX64(machInst, rt, rnsp, rs);
+                      default:
+                        __builtin_unreachable();
                     }
                   case 0x6:
                     switch (size) {
@@ -535,6 +560,8 @@
                         return new LDXPW64(machInst, rt, rt2, rnsp);
                       case 0x3:
                         return new LDXPX64(machInst, rt, rt2, rnsp);
+                      default:
+                        __builtin_unreachable();
                     }

                   case 0x7:
@@ -546,6 +573,8 @@
                         return new LDAXPW64(machInst, rt, rt2, rnsp);
                       case 0x3:
                         return new LDAXPX64(machInst, rt, rt2, rnsp);
+                      default:
+                        __builtin_unreachable();
                     }

                   case 0x9:
@@ -558,6 +587,8 @@
                         return new STLRW64(machInst, rt, rnsp);
                       case 0x3:
                         return new STLRX64(machInst, rt, rnsp);
+                      default:
+                        __builtin_unreachable();
                     }
                   case 0xd:
                     switch (size) {
@@ -569,6 +600,8 @@
                         return new LDARW64(machInst, rt, rnsp);
                       case 0x3:
                         return new LDARX64(machInst, rt, rnsp);
+                      default:
+                        __builtin_unreachable();
                     }
                   default:
                     return new Unknown64(machInst);
@@ -1021,9 +1054,13 @@
                         return new Unknown64(machInst);
                     }
                   }
+                  default:
+                    __builtin_unreachable();
                 }
             }
           }
+          default:
+            __builtin_unreachable();
         }
         return new FailUnimplemented("Unhandled Case1", machInst);
     }
@@ -1070,6 +1107,8 @@
                 return new AndXSRegCc(machInst, rdzr, rn, rm, imm6, type);
               case 0x7:
                 return new BicXSRegCc(machInst, rdzr, rn, rm, imm6, type);
+              default:
+                __builtin_unreachable();
             }
           }
           case 0x1:
@@ -1096,6 +1135,8 @@
return new SubXSReg(machInst, rdzr, rn, rm, imm6, type);
                   case 0x3:
return new SubXSRegCc(machInst, rdzr, rn, rm, imm6, type);
+                  default:
+                    __builtin_unreachable();
                 }
             } else {
if (bits(machInst, 23, 22) != 0 || bits(machInst, 12, 10)
0x4)
@@ -1119,6 +1160,8 @@
return new SubXEReg(machInst, rdsp, rnsp, rm, type, imm3);
                   case 0x3:
return new SubXERegCc(machInst, rdzr, rnsp, rm, type, imm3);
+                  default:
+                    __builtin_unreachable();
                 }
             }
           }
@@ -1145,6 +1188,8 @@
                     return new SbcXSReg(machInst, rdzr, rn, rm, 0, LSL);
                   case 0x3:
                     return new SbcXSRegCc(machInst, rdzr, rn, rm, 0, LSL);
+                  default:
+                    __builtin_unreachable();
                 }
               }
               case 0x1:
@@ -1198,6 +1243,8 @@
                     return new Csinv64(machInst, rdzr, rn, rm, cond);
                   case 0x3:
                     return new Csneg64(machInst, rdzr, rn, rm, cond);
+                  default:
+                    __builtin_unreachable();
                 }
               }
               case 0x3:
@@ -1261,8 +1308,12 @@
                         return new Clz64(machInst, rdzr, rn);
                       case 0x5:
                         return new Cls64(machInst, rdzr, rn);
+                      default:
+                        return new Unknown64(machInst);
                     }
                 }
+              default:
+                __builtin_unreachable();
             }
           }
           case 0x3:
@@ -1303,6 +1354,8 @@
                 return new Unknown64(machInst);
             }
           }
+          default:
+            __builtin_unreachable();
         }
         return new FailUnimplemented("Unhandled Case2", machInst);
     }
@@ -1484,6 +1537,8 @@
                   default:
                     return new Unknown64(machInst);
                 }
+              default:
+                return new Unknown64(machInst);
             }
         } else {
             // 30=0, 28:24=11110, 21=1
@@ -1946,6 +2001,8 @@
                 else
                     return new Unknown64(machInst);
               }
+              default:
+                __builtin_unreachable();
             }
         }
         return new FailUnimplemented("Unhandled Case4", machInst);
diff --git a/src/arch/arm/isa/formats/data.isa b/src/arch/arm/isa/formats/data.isa
index eab0818..8b5081d 100644
--- a/src/arch/arm/isa/formats/data.isa
+++ b/src/arch/arm/isa/formats/data.isa
@@ -76,17 +76,17 @@

 def format ArmDataProcReg() {{
     pclr = '''
-        return new %(className)ssRegPclr(machInst, %(dest)s,
-                                        %(op1)s, rm, imm5,
-                                        type);
+                    if (%(dest)s == INTREG_PC) {
+ return new %(className)ssRegPclr(machInst, %(dest)s,
+                                                         %(op1)s, rm, imm5,
+                                                         type);
+                    } else
     '''
     instDecode = '''
           case %(opcode)#x:
             if (immShift) {
                 if (setCc) {
-                    if (%(dest)s == INTREG_PC) {
-                        %(pclr)s
-                    } else {
+                    %(pclr)s {
                         return new %(className)sRegCc(machInst, %(dest)s,
%(op1)s, rm, imm5, type);
                     }
@@ -452,26 +452,26 @@

 def format ArmDataProcImm() {{
     pclr = '''
-        return new %(className)ssImmPclr(machInst, %(dest)s,
-                                        %(op1)s, imm, false);
+                if (%(dest)s == INTREG_PC) {
+                    return new %(className)ssImmPclr(machInst, %(dest)s,
+                                                     %(op1)s, imm, false);
+                } else
     '''
     adr = '''
-        return new AdrImm(machInst, %(dest)s, %(add)s,
-                                     imm, false);
+                if (%(dest)s == INTREG_PC) {
+                    return new AdrImm(machInst, %(dest)s, %(add)s,
+                                      imm, false);
+                } else
     '''
     instDecode = '''
           case %(opcode)#x:
             if (setCc) {
-                if (%(pclrInst)s && %(dest)s == INTREG_PC) {
-                    %(pclr)s
-                } else {
+                %(pclr)s {
return new %(className)sImmCc(machInst, %(dest)s, %(op1)s,
                                                    imm, rotC);
                 }
             } else {
-                if (%(adrInst)s && %(op1)s == INTREG_PC) {
-                    %(adr)s
-                } else {
+                %(adr)s {
return new %(className)sImm(machInst, %(dest)s, %(op1)s,
                                                  imm, rotC);
                 }
@@ -493,13 +493,10 @@
                       "opcode": opcode,
                       "dest": dest,
                       "op1": op1,
-                      "adr": "",
-                      "adrInst": "false" }
+                      "adr": "" }
         if useDest:
-            substDict["pclrInst"] = "true"
             substDict["pclr"] = pclr % substDict
         else:
-            substDict["pclrInst"] = "false"
             substDict["pclr"] = ""
         return instDecode % substDict

@@ -509,9 +506,7 @@
                       "opcode": opcode,
                       "dest": "rd",
                       "op1": "rn",
-                      "add": add,
-                      "pclrInst": "true",
-                      "adrInst": "true" }
+                      "add": add }
         substDict["pclr"] = pclr % substDict
         substDict["adr"] = adr % substDict
         return instDecode % substDict
@@ -611,6 +606,8 @@
                   case 0x7:
                     return new MovRegRegCc(machInst, rd,
                             INTREG_ZERO, rn, rm, ROR);
+                  default:
+                    __builtin_unreachable();
                 }
             } else if (bits(op2, 3) == 0) {
                 return new Unknown(machInst);
@@ -916,6 +913,8 @@
                 } else {
                     return new SubImmCc(machInst, rd, rn, imm3, true);
                 }
+              default:
+                __builtin_unreachable();
             }
           case 0x4:
             if (machInst.itstateMask) {
@@ -937,6 +936,8 @@
             } else {
                 return new SubImmCc(machInst, rd8, rd8, imm8, true);
             }
+          default:
+            __builtin_unreachable();
         }
     }
     '''
@@ -1040,6 +1041,8 @@
             } else {
return new MvnRegCc(machInst, rdn, INTREG_ZERO, rm, 0, LSL);
             }
+          default:
+            __builtin_unreachable();
         }
     }
     '''
@@ -1069,6 +1072,8 @@
(IntRegIndex)(uint32_t)bits(machInst, 6, 3),
                                   COND_UC);
             }
+          default:
+            __builtin_unreachable();
         }
     }
     '''
@@ -1169,6 +1174,8 @@
                     return new Uxth(machInst, rd, 0, rm);
                   case 0x3:
                     return new Uxtb(machInst, rd, 0, rm);
+                  default:
+                    __builtin_unreachable();
                 }
             }
           case 0x1:
@@ -1196,6 +1203,7 @@
                                           ((enable ? 1 : 0) << 9);
                     return new Cps(machInst, mods);
                 }
+                return new Unknown(machInst);
             }
           case 0xa:
             {
@@ -1407,7 +1415,7 @@
                 const uint32_t satImm = bits(machInst, 4, 0);
                 return new Ssat16(machInst, rd, satImm + 1, rn);
             }
-            // Fall through on purpose...
+            // Fall through
           case 0x10:
             {
                 const uint32_t satImm = bits(machInst, 4, 0);
@@ -1440,7 +1448,7 @@
                 const uint32_t satImm = bits(machInst, 4, 0);
                 return new Usat16(machInst, rd, satImm, rn);
             }
-            // Fall through on purpose...
+            // Fall through
           case 0x18:
             {
                 const uint32_t satImm = bits(machInst, 4, 0);
diff --git a/src/arch/arm/isa/formats/fp.isa b/src/arch/arm/isa/formats/fp.isa
index 82d351e..648a1cd 100644
--- a/src/arch/arm/isa/formats/fp.isa
+++ b/src/arch/arm/isa/formats/fp.isa
@@ -130,7 +130,7 @@
                     width = 1;
                     break;
                 }
-                // Fall through on purpose.
+                // Fall through
               default:
                 return new Unknown(machInst);
             }
@@ -404,6 +404,8 @@
                         } else {
return new VbifD<uint64_t>(machInst, vd, vn, vm);
                         }
+                      default:
+                        __builtin_unreachable();
                     }
                 } else {
                     switch (c) {
@@ -445,6 +447,8 @@
                             return new VornD<uint64_t>(
                                     machInst, vd, vn, vm);
                         }
+                      default:
+                        __builtin_unreachable();
                     }
                 }
             }
@@ -1550,6 +1554,8 @@
                     return decodeNeonSTwoMiscReg<NVnegD, NVnegQ>(
                             q, size, machInst, vd, vm);
                 }
+              default:
+                return new Unknown64(machInst);
             }
           case 0x2:
             switch (bits(b, 4, 1)) {
@@ -1859,6 +1865,8 @@
                     // If rn == sp, then this is called vpop.
                     return new VLdmStm(machInst, rn, vd, single,
                                        true, true, true, offset);
+                  default:
+                    __builtin_unreachable();
                 }
             }
           case 0x2:
@@ -1870,7 +1878,7 @@
                 return new VLdmStm(machInst, rn, vd, single,
                                    false, true, true, offset);
             }
-            // Fall through on purpose
+            // Fall through
           case 0x3:
             const bool up = (bits(machInst, 23) == 1);
             const uint32_t imm = bits(machInst, 7, 0) << 2;
diff --git a/src/arch/arm/isa/formats/m5ops.isa b/src/arch/arm/isa/formats/m5ops.isa
index d3db813..6b4112d 100644
--- a/src/arch/arm/isa/formats/m5ops.isa
+++ b/src/arch/arm/isa/formats/m5ops.isa
@@ -68,6 +68,7 @@
             case M5OP_PANIC: return new M5panic(machInst);
             case M5OP_WORK_BEGIN: return new M5workbegin(machInst);
             case M5OP_WORK_END: return new M5workend(machInst);
+            default: return new Unknown(machInst);
         }
    }
    '''
diff --git a/src/arch/arm/isa/formats/mem.isa b/src/arch/arm/isa/formats/mem.isa
index abac270..b44c250 100644
--- a/src/arch/arm/isa/formats/mem.isa
+++ b/src/arch/arm/isa/formats/mem.isa
@@ -470,6 +470,7 @@
                     return new %(imm_puw)s(machInst, RT, RN, true, imm);
                 }
             }
+          return new Unknown(machInst);
         } else {
             return new Unknown(machInst);
         }
@@ -514,6 +515,8 @@
                         return new %(imm_pu)s(machInst, RT, RN, true, imm);
                       case 7:
return new %(imm_puw)s(machInst, RT, RN, true, imm);
+                      default:
+                        __builtin_unreachable();
                     }
                 }
         '''
@@ -978,6 +981,8 @@
             return new %(ldrb)s(machInst, rt, rn, true, 0, LSL, rm);
           case 0x7:
             return new %(ldrsh)s(machInst, rt, rn, true, 0, LSL, rm);
+          default:
+            __builtin_unreachable();
         }
     }
     '''
diff --git a/src/arch/arm/isa/formats/mult.isa b/src/arch/arm/isa/formats/mult.isa
index 73157dd..0fd851c 100644
--- a/src/arch/arm/isa/formats/mult.isa
+++ b/src/arch/arm/isa/formats/mult.isa
@@ -87,6 +87,8 @@
               } else {
                   return new Smlal(machInst, ra, rd, rn, rm);
               }
+            default:
+              __builtin_unreachable();
         }
     }
     '''
@@ -112,6 +114,8 @@
                 return new SmlabtCc(machInst, rd, rn, rm, ra);
               case 0x3:
                 return new SmlattCc(machInst, rd, rn, rm, ra);
+              default:
+                __builtin_unreachable();
             }
           case 0x1:
             if (op) {
@@ -137,6 +141,8 @@
                 return new Smlalbt(machInst, ra, rd, rn, rm);
               case 0x3:
                 return new Smlaltt(machInst, ra, rd, rn, rm);
+              default:
+                __builtin_unreachable();
             }
           case 0x3:
             switch (bits(machInst, 6, 5)) {
@@ -148,7 +154,11 @@
                 return new Smulbt(machInst, rd, rn, rm);
               case 0x3:
                 return new Smultt(machInst, rd, rn, rm);
+              default:
+                __builtin_unreachable();
             }
+          default:
+            __builtin_unreachable();
         }
     }
     '''
@@ -201,6 +211,7 @@
                     return new SmlattCc(machInst, rd, rn, rm, ra);
                 }
             }
+            __builtin_unreachable();
           case 0x2:
             if (ra == 0xf) {
                 if (bits(machInst, 4)) {
@@ -271,6 +282,8 @@
             } else {
                 return new Usada8(machInst, rd, rn, rm, ra);
             }
+          default:
+            __builtin_unreachable();
         }
     }
     '''
diff --git a/src/arch/arm/isa/formats/neon64.isa b/src/arch/arm/isa/formats/neon64.isa
index e0a913a..b92734a 100644
--- a/src/arch/arm/isa/formats/neon64.isa
+++ b/src/arch/arm/isa/formats/neon64.isa
@@ -181,6 +181,8 @@
                     else
                         return new OrnDX<uint64_t>(machInst, vd, vn, vm);
                 }
+              default:
+                __builtin_unreachable();
             }
           case 0x04:
             if (size == 0x3)
@@ -1211,6 +1213,8 @@
                     return new DupGprXQX<uint64_t>(machInst, vd, vn);
                 else
                     return new Unknown64(machInst);
+              default:
+                return new Unknown64(machInst);
             }
           case 0x3:
             index1 = imm5 >> (imm5_pos + 1);
@@ -2065,17 +2069,17 @@
                 return decodeNeonUTwoMiscScFpReg<FcmltZeroScX>(
                     size & 0x1, machInst, vd, vn);
           case 0x14:
-            if (size == 0x3) {
+            switch (size) {
+              case 0x0:
+                return new SqxtnScX<int8_t>(machInst, vd, vn);
+              case 0x1:
+                return new SqxtnScX<int16_t>(machInst, vd, vn);
+              case 0x2:
+                return new SqxtnScX<int32_t>(machInst, vd, vn);
+              case 0x3:
                 return new Unknown64(machInst);
-            } else {
-                switch (size) {
-                  case 0x0:
-                    return new SqxtnScX<int8_t>(machInst, vd, vn);
-                  case 0x1:
-                    return new SqxtnScX<int16_t>(machInst, vd, vn);
-                  case 0x2:
-                    return new SqxtnScX<int32_t>(machInst, vd, vn);
-                }
+              default:
+                __builtin_unreachable();
             }
           case 0x1a:
             if (size < 0x2)
@@ -2145,30 +2149,30 @@
                 return decodeNeonUTwoMiscScFpReg<FcmleZeroScX>(
                     size & 0x1, machInst, vd, vn);
           case 0x32:
-            if (size == 0x3) {
+            switch (size) {
+              case 0x0:
+                return new SqxtunScX<int8_t>(machInst, vd, vn);
+              case 0x1:
+                return new SqxtunScX<int16_t>(machInst, vd, vn);
+              case 0x2:
+                return new SqxtunScX<int32_t>(machInst, vd, vn);
+              case 0x3:
                 return new Unknown64(machInst);
-            } else {
-                switch (size) {
-                  case 0x0:
-                    return new SqxtunScX<int8_t>(machInst, vd, vn);
-                  case 0x1:
-                    return new SqxtunScX<int16_t>(machInst, vd, vn);
-                  case 0x2:
-                    return new SqxtunScX<int32_t>(machInst, vd, vn);
-                }
+              default:
+                __builtin_unreachable();
             }
           case 0x34:
-            if (size == 0x3) {
+            switch (size) {
+              case 0x0:
+                return new UqxtnScX<uint8_t>(machInst, vd, vn);
+              case 0x1:
+                return new UqxtnScX<uint16_t>(machInst, vd, vn);
+              case 0x2:
+                return new UqxtnScX<uint32_t>(machInst, vd, vn);
+              case 0x3:
                 return new Unknown64(machInst);
-            } else {
-                switch (size) {
-                  case 0x0:
-                    return new UqxtnScX<uint8_t>(machInst, vd, vn);
-                  case 0x1:
-                    return new UqxtnScX<uint16_t>(machInst, vd, vn);
-                  case 0x2:
-                    return new UqxtnScX<uint32_t>(machInst, vd, vn);
-                }
+              default:
+                __builtin_unreachable();
             }
           case 0x36:
             if (size != 0x1) {

--
To view, visit https://gem5-review.googlesource.com/8541
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1baa9fa0ed15181c10c755c0bd777f88b607c158
Gerrit-Change-Number: 8541
Gerrit-PatchSet: 1
Gerrit-Owner: Siddhesh Poyarekar <siddhesh.poyare...@gmail.com>
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to