[PATCH] MC: support different sized constants in constant pools
David Peixotto
dpeixott at codeaurora.org
Thu Jul 17 10:50:57 PDT 2014
Thanks, this is looking much better. I had one minor suggestion and a question about the APInt usage.
Also, for future patches it would be best to include more context in the patch as described here: http://llvm.org/docs/Phabricator.html. It makes it easier to review.
Thanks for all your work.
================
Comment at: include/llvm/MC/ConstantPools.h:44
@@ -36,3 +43,3 @@
// \param Value is the new entry to put in the constant pool.
//
// \returns a MCExpr that references the newly inserted value
----------------
Add comment for Size param (size in bytes)
================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:3120
@@ +3119,3 @@
+ // check whether the immediate is a (un)signed 32-bit value
+ if (!IsXReg && Simm.getNumSignBits() - Simm.isNegative() < 32)
+ return Error(Loc, "Immediate too large for register");
----------------
This test still looks a bit hard to understand for me. Are you just checking if the value fits into 32 bits? If so I think APInt::isIntN(32) would be more readable.
http://reviews.llvm.org/D4279
More information about the llvm-commits
mailing list