[PATCH] ARM IAS: support .inst directive

Renato Golin renato.golin at linaro.org
Mon Dec 16 04:53:54 PST 2013


  Hi Saleem,

  Thanks for the context, it's a lot easier to understand the diffs that way. ;)

  The patch looks good to me, too (apart from a few comments below), and the tests are really good, thanks!

  cheers,
  --renato


================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:8362
@@ +8361,3 @@
+    } else {
+      return Error(Loc, "expected constant expression");
+    }
----------------
Nit pick on style, this should be before the block to avoid extra indentation:

  const MCConstantExpr *Value = dyn_cast_or_null<MCConstantExpr>(Expr);
  if (!Value)
    return Error(Loc, "expected constant expression");

  switch (Width) {
  ...

================
Comment at: lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp:386
@@ +385,3 @@
+
+    switch (Suffix) {
+    case '\0':
----------------
Is it possible to put an assert here to make sure we're in the right mode instead of assuming the parser did a good job?


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



More information about the llvm-commits mailing list