[llvm-commits] [PATCH] Add APInt(numBits, ArrayRef<uint64_t> bigVal) constructor (issue4749046)

Jeffrey Yasskin jyasskin at google.com
Mon Jul 18 11:42:34 PDT 2011


On Mon, Jul 18, 2011 at 11:33 AM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Mon, Jul 18, 2011 at 9:57 AM, Jeffrey Yasskin <jyasskin at google.com> wrote:
>> Here's the patch.
>>
>> On Sat, Jul 16, 2011 at 10:51 PM,  <jyasskin at gmail.com> wrote:
>>> Reviewers: eli.friedman_gmail.com,
>>>
>>> Message:
>>> Initial diff at
>>> http://codereview.appspot.com/download/issue4749046_1.diff
>>>
>>> Description:
>>> Add APInt(numBits, ArrayRef<uint64_t> bigVal) constructor to prevent
>>> future ambiguity errors like the one corrected by r135261.  Migrate all
>>> LLVM callers of the old constructor to the new one.
>>>
>>>
>>> Please review this at http://codereview.appspot.com/4749046/
>>>
>>> Affected files:
>>>   M include/llvm/ADT/APInt.h
>>>   M lib/AsmParser/LLLexer.cpp
>>>   M lib/Bitcode/Reader/BitcodeReader.cpp
>>>   M lib/CodeGen/SelectionDAG/FastISel.cpp
>>>   M lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
>>>   M lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>>>   M lib/ExecutionEngine/ExecutionEngine.cpp
>>>   M lib/Support/APFloat.cpp
>>>   M lib/Support/APInt.cpp
>>>   M lib/VMCore/ConstantFold.cpp
>>>   M lib/VMCore/Core.cpp
>>>   M unittests/ADT/APIntTest.cpp
>
> @@ -342,7 +353,8 @@ public:
>
>     if (isSingleWord())
>       return isUIntN(N, VAL);
> -    return APInt(N, getNumWords(), pVal).zext(getBitWidth()) == (*this);
> +    return APInt(N, ArrayRef<uint64_t>(pVal,
> getNumWords())).zext(getBitWidth())
> +      == (*this);
>   }
>
>   /// @brief Check if this APInt has an N-bits signed integer value.
>
> makeArrayRef just landed; use it. :)  Same in a couple other places.

Silly codebase, changing out from under me. ;-) Will do.

> Otherwise, looks fine.
>
> As a followup, could you eliminate both the "APInt(unsigned numBits,
> unsigned numWords, const uint64_t bigVal[])" and the "APInt(unsigned
> numBits, uint64_t val, bool isSigned = false)" constructors?  (It
> should just require ArrayRef-izing and adding an
> "APInt::getSigned(unsigned numBits, uint64_t val)" helper.)

How do you want to handle dependent codebases? I was thinking to leave
APInt(...bigVal[]) in for a version to let people switch, but I guess
LLVM does tend to remove things more aggressively than that. I'm only
willing to commit to fixing clang myself if you want me to both fix
things and remove the deprecated constructors.




More information about the llvm-commits mailing list