[PATCH] D15280: [ARMv8-M] [7/9] Add ARMv8-M security extension instructions to ARMv8-M Baseline/Mainline

[Tim Northover via llvm-commits]

t.p.northover added a comment.

Some minor points here, though again I'm having to trust you on matching the specification.

Cheers.

Tim.


================
Comment at: lib/Target/ARM/ARMInstrThumb2.td:4335-4338
@@ +4334,6 @@
+
+def t2SG : T2I<(outs), (ins), NoItinerary, "sg", "", []>,
+           Requires<[Has8MSecExt]> {
+  let Inst = 0xe97fe97f;
+}
+
----------------
Does this need hasSideEffects or similar? Otherwise (with no inputs or outputs) if it ever gets to CodeGen the compiler is going to nuke it out of hand.

================
Comment at: lib/Target/ARM/ARMInstrVFP.td:207
@@ +206,3 @@
+//
+def VLLDM : AXSI4<(outs), (ins GPRnopc:$Rn, pred:$p), IndexModeNone,
+                  IIC_fpLoad_m, "vlldm${p}\t$Rn", "", []>,
----------------
These look unrelated to the security extensions. Probably also need mayLoad/mayStore.

================
Comment at: test/MC/ARM/thumbv8m.s:168
@@ +167,3 @@
+// Invalid operand tests
+// UNDEF: error: invalid operand for instruction
+sg #0
----------------
Written like this, it becomes just a count of "invalid operand for instruction" errors. It's easier to track down future regressions if the problematic input line gets checked too (as in other error tests).


Repository:
  rL LLVM

http://reviews.llvm.org/D15280