[PATCH] D23179: [SystemZ] Add some missing instruction classes and instructions.

Ulrich Weigand via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 5 03:59:26 PDT 2016


uweigand accepted this revision.
uweigand added a comment.
This revision is now accepted and ready to land.

Looks basically good to me.  Some of the instructions don't have the appropriate flags set -- see inline comments.  This is important even for instructions not used for codegen, since those flags can be queried via the disassembler interface (e.g. LLDB uses this to detect branches).  Patch OK with those changes.


================
Comment at: lib/Target/SystemZ/SystemZInstrInfo.td:341
@@ +340,3 @@
+def BRXLE : InstRSI<0x85, (outs), (ins GR64:$R1, GR64:$R3, brtarget16:$RI2),
+                    "brxle\t$R1, $R3, $RI2", []>;
+
----------------
These should have the isBranch and isTerminator flags set.  Maybe we should have a BranchBinaryRSI helper similar to BranchUnaryRI ...

================
Comment at: lib/Target/SystemZ/SystemZInstrInfo.td:1689
@@ +1688,3 @@
+let Defs = [CC] in {
+  def PR : InstE<0x0101, (outs), (ins), "pr", []>;
+  def MVCK : InstSSd<0xD9, (outs),
----------------
PR actually changes a lot of registers and has other side effects.  It probably doesn't matter too much since this instruction isn't usable on Linux anyway, but at least we should have the hasSideEffects flag.

================
Comment at: lib/Target/SystemZ/SystemZInstrInfo.td:1693
@@ +1692,3 @@
+                                GR64:$R3),
+                     "mvck\t$XBD1, $BD2, $R3", []>;
+}
----------------
Should have the mayLoad and mayStore flags.

================
Comment at: lib/Target/SystemZ/SystemZInstrInfo.td:1697
@@ +1696,3 @@
+def STRAG : InstSSE<0xE502, (outs), (ins bdaddr12only:$BD1, bdaddr12only:$BD2),
+                    "strag\t$BD1, $BD2", []>;
+
----------------
Should have the mayStore flag.

================
Comment at: lib/Target/SystemZ/SystemZInstrInfo.td:1702
@@ -1683,1 +1701,3 @@
+                            (ins bdaddr12only:$BD1, bdaddr12only:$BD2, GR64:$R3),
+                     "ectg\t$BD1, $BD2, $R3", []>;
 
----------------
Should have the mayLoad flag.


https://reviews.llvm.org/D23179





More information about the llvm-commits mailing list