[PATCH] [mips] [IAS] Add partial support for the ULHU pseudo-instruction.

Daniel Sanders daniel.sanders at imgtec.com
Tue May 12 08:18:40 PDT 2015


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2164-2165
@@ +2163,4 @@
+
+  if (!AssemblerOptions.back()->isMacro())
+    Warning(IDLoc, "macro instruction expanded into multiple instructions");
+
----------------
Nit: We're going to end up writing this a lot. It would be good to factor it out.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2181-2182
@@ +2180,4 @@
+  unsigned ATReg = getATReg(IDLoc);
+  if (!ATReg)
+    return true;
+
----------------
For '.set noat' mode we currently give up here before discovering whether we need AT or not.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2188
@@ +2187,3 @@
+  bool LoadedOffsetInAT = false;
+  if (!isInt<16>(OffsetValue + 1)) {
+    LoadedOffsetInAT = true;
----------------
What happens if OffsetValue is -32769? OffsetValue+1 will fit but we still need AT for OffsetValue

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2203-2215
@@ +2202,15 @@
+  MCInst TmpInst;
+  TmpInst.setOpcode(Mips::LBu);
+  TmpInst.addOperand(MCOperand::CreateReg(LoadedOffsetInAT ? DstReg : ATReg));
+  TmpInst.addOperand(MCOperand::CreateReg(LoadedOffsetInAT ? ATReg : SrcReg));
+  TmpInst.addOperand(MCOperand::CreateImm(LoadedOffsetInAT ? 0 : OffsetValue));
+  Instructions.push_back(TmpInst);
+
+  TmpInst.clear();
+  TmpInst.setOpcode(Mips::LBu);
+  TmpInst.addOperand(MCOperand::CreateReg(LoadedOffsetInAT ? ATReg : DstReg));
+  TmpInst.addOperand(MCOperand::CreateReg(LoadedOffsetInAT ? ATReg : SrcReg));
+  TmpInst.addOperand(
+      MCOperand::CreateImm(LoadedOffsetInAT ? 1 : (OffsetValue + 1)));
+  Instructions.push_back(TmpInst);
+
----------------
Shouldn't endianness have some effect on the expansion?

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2204-2220
@@ +2203,19 @@
+  TmpInst.setOpcode(Mips::LBu);
+  TmpInst.addOperand(MCOperand::CreateReg(LoadedOffsetInAT ? DstReg : ATReg));
+  TmpInst.addOperand(MCOperand::CreateReg(LoadedOffsetInAT ? ATReg : SrcReg));
+  TmpInst.addOperand(MCOperand::CreateImm(LoadedOffsetInAT ? 0 : OffsetValue));
+  Instructions.push_back(TmpInst);
+
+  TmpInst.clear();
+  TmpInst.setOpcode(Mips::LBu);
+  TmpInst.addOperand(MCOperand::CreateReg(LoadedOffsetInAT ? ATReg : DstReg));
+  TmpInst.addOperand(MCOperand::CreateReg(LoadedOffsetInAT ? ATReg : SrcReg));
+  TmpInst.addOperand(
+      MCOperand::CreateImm(LoadedOffsetInAT ? 1 : (OffsetValue + 1)));
+  Instructions.push_back(TmpInst);
+
+  TmpInst.clear();
+  TmpInst.setOpcode(Mips::SLL);
+  TmpInst.addOperand(MCOperand::CreateReg(LoadedOffsetInAT ? DstReg : ATReg));
+  TmpInst.addOperand(MCOperand::CreateReg(LoadedOffsetInAT ? DstReg : ATReg));
+  TmpInst.addOperand(MCOperand::CreateImm(8));
----------------
Nit: There's some repetition in the ternary operators. Could you factor out the repeated ones into variables?

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2254
@@ -2170,3 +2253,3 @@
   MCInst AdduInst;
-  AdduInst.setOpcode(Mips::ADDu);
+  AdduInst.setOpcode(isGP64bit() ? Mips::DADDu : Mips::ADDu);
   AdduInst.addOperand(MCOperand::CreateReg(DstReg));
----------------
I'm not sure why this is needed.

http://reviews.llvm.org/D9671

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list