[PATCH] [mips] [IAS] Fix expansion of negative 32-bit immediates for LI/DLI.

[Daniel Sanders]

Thanks for reworking this. A couple comments:


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1734-1735
@@ -1733,3 +1733,4 @@
     Instructions.push_back(tmpInst);
-  } else if ((ImmValue & 0xffffffff) == ImmValue) {
+  } else if ((ImmValue & 0xffffffff) == ImmValue ||
+             (ImmValue | ~0x7fffffff) == ImmValue) {
     // For all other values which are representable as a 32-bit integer:
----------------
This condition is a complicated way of saying 'does it fit in a signed/unsigned 32-bit integer?' which can be more simply written as 'isInt<32>(ImmValue) || isUInt<32>(ImmValue)'. A few of the other paths do the same kind of thing, for example the first if-statement could be written 'isUInt<16>(ImmValue).

Also: it isn't correct for the DLI case when ImmValue is 0x80000000 because 'lui' produces a sign-extended (32->64-bit) result giving 0xffffffff80000000 in the register. It's fine for 32-bit though.
You need the condition to be isInt<32>(ImmValue) for the DLI case and 'isInt<32>(ImmValue) || isUInt<32>(ImmValue)' for the LI case.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1755
@@ -1753,3 +1754,3 @@
     // |          |          |          |
     // | 16-bytes | 16-bytes | 16-bytes |
     // |__________|__________|__________|
----------------
I know this isn't from your patch but I just noticed it. Shouldn't these be '16-bits'?

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:1780
@@ -1778,3 +1779,3 @@
     // |          |          |          |          |
     // | 16-bytes | 16-bytes | 16-bytes | 16-bytes |
     // |__________|__________|__________|__________|
----------------
Same here

http://reviews.llvm.org/D8662

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