[PATCH] [mips] [IAS] Unify common functionality of LA and LI.

Daniel Sanders daniel.sanders at imgtec.com
Wed May 13 03:33:48 PDT 2015


LGTM with/without an optional nit.


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1747
@@ -1738,1 +1746,3 @@
+    tmpInst.addOperand(MCOperand::CreateReg(DstReg));
+    tmpInst.addOperand(MCOperand::CreateReg(UseSrcReg ? SrcReg : Mips::ZERO));
     tmpInst.addOperand(MCOperand::CreateImm(ImmValue));
----------------
tomatabacu wrote:
> dsanders wrote:
> > Mips::ZERO should be Mips::ZERO64 on 64-bit targets.
> > 
> > Likewise for the bitwise operations below but not for ADDiu.
> > 
> > Also, I'd suggest having SrcReg default to Mips::ZERO or Mips::ZERO64 and renaming UseSrcReg to SrcRegIsNotZero.
> Essentially done.
> I didn't make SrcReg default to Mips::ZERO or Mips::ZERO64 because there are only 2 places where it could be used with those values and they are slightly different, as one of them should only get Mips::ZERO (the ADDiu).
> This also means that there's no reason to rename UseSrcReg to SrcRegIsNotZero.
Good point. I'd slightly prefer it for tidyness but it's a very slight preference so consider it optional.

http://reviews.llvm.org/D9290

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






More information about the llvm-commits mailing list