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

Daniel Sanders daniel.sanders at imgtec.com
Fri Jun 12 06:21:48 PDT 2015


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2301
@@ +2300,3 @@
+
+  unsigned FstLbuDstReg = LoadedOffsetInAT ? DstReg : ATReg;
+  unsigned SndLbuDstReg = LoadedOffsetInAT ? ATReg : DstReg;
----------------
Nit: I believe Fst should be First and Snd should be Second. Likewise below

FWIW, I'd usually spell them 'TmpReg1' and 'TmpReg2'. There's often little description you can give to scratch registers.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2181-2182
@@ +2180,4 @@
+  unsigned ATReg = getATReg(IDLoc);
+  if (!ATReg)
+    return true;
+
----------------
tomatabacu wrote:
> dsanders wrote:
> > For '.set noat' mode we currently give up here before discovering whether we need AT or not.
> We always need AT for ULHU, as it is always used as the source register for one of the LBu's.
> I added a code comment with this explanation.
Ah yes, it looked like one path didn't use it but that's not the case.

================
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));
----------------
tomatabacu wrote:
> dsanders wrote:
> > I'm not sure why this is needed.
> Because otherwise we would always generate ADDu, and GAS generates a DADDu for MIPS64.
> The reason why it's in this patch is because this is where we have a concrete test case for it.
Hmm.. This patch uses loadImmediate() is to load an offset into a register. In this context, we don't want to test for 64-bit GPR's but rather for 64-bit pointers (see MipsABIInfo::GetPtrAddiuOp()). However, loadImmediate() is also used for the 'li' instruction where we do want the test to be for 64-bit GPR's.

You'll need to separate the two contexts somehow. I think line 2197 ought to be:
  if (loadImmediate(OffsetValue, ATReg, SrcReg, ABI.ArePtrs64bit(), IDLoc,
                    Instructions))
and the Is32bitImm argument to loadImmediate() needs to determine whether you get ADDu or DADDu

http://reviews.llvm.org/D9671

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






More information about the llvm-commits mailing list