[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