[llvm-commits] Move Module::getTypeByName to LLVMContext::getTypeByName
Chris Lattner
clattner at apple.com
Thu Jan 10 17:39:59 PST 2013
On Jan 10, 2013, at 4:03 PM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:
> LGTM. Chris, I don't understand why getTypeByName() belongs to Module?
It was put there as a convenience. Moving it to LLVMContext makes perfect sense.
-Chris
>
> Mael, please update and resend your patch along trunk, thank you.
> LLVMContext.h has been moved to include/llvm/IR.
>
>
>> --- a/include/llvm/LLVMContext.h
>> +++ b/include/llvm/LLVMContext.h
>> @@ -86,6 +87,10 @@ public:
>> void emitError(const Instruction *I, const Twine &ErrorStr);
>> void emitError(const Twine &ErrorStr);
>>
>> + /// getTypeByName - Return the type with the specified name, or null if there
>> + /// is none by that name.
>> + StructType * getTypeByName(StringRef Name) const;
>
> We prefer;
>> + StructType *getTypeByName(StringRef Name) const;
>
>> +
>> private:
>> // DO NOT IMPLEMENT
>> LLVMContext(LLVMContext&);
>
>> --- a/lib/VMCore/Type.cpp
>> +++ b/lib/VMCore/Type.cpp
>> @@ -631,11 +631,7 @@ bool StructType::isLayoutIdentical(StructType *Other) const {
>> /// getTypeByName - Return the type with the specified name, or null if there
>> /// is none by that name.
>> StructType *Module::getTypeByName(StringRef Name) const {
>> - StringMap<StructType*>::iterator I =
>> - getContext().pImpl->NamedStructTypes.find(Name);
>> - if (I != getContext().pImpl->NamedStructTypes.end())
>> - return I->second;
>> - return 0;
>> + return getContext().getTypeByName(Name);
>
> We prefer here;
>> + return getContext().getTypeByName(Name);
>
>> }
>
> 2013/1/11 Maël Nison <nison.mael at gmail.com>:
>> Could it be possible to have a feedback, even negative, please ?
>>
>>
>> On 5 August 2012 14:44, Maël Nison <nison.mael at gmail.com> wrote:
>>>
>>> As a workaround, creating a temporary stack-alloced Module to use its
>>> getTypeByName() method works.
>>>
>>> But that's a workaround, it requires useless additional instructions. : /
>>>
>>>
>>> On 2 August 2012 18:40, Maël Nison <nison.mael at gmail.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> This very short patch moves Module's getTypeByName into LLVMContext,
>>>> because :
>>>>
>>>> - Every types are linked to a Context, so it makes sense to get types
>>>> from the context and not from a module
>>>> - The Module::getTypeByName function doesn't actually use anything owned
>>>> by the Module class
>>>> - It is not possible to specialize the TypeBuilder to return a named
>>>> structure (we can only returns literal structures). By moving getTypeByName
>>>> into the LLVMContext class, we are able to do it (TypeBuilder's
>>>> specializations are called with a LLVMContext reference as parameter).
>>>>
>>>> In order to keep backward compatibility, Module::getTypeByName now
>>>> forwards to the new method.
>>>>
>>>> It's my first patch so .. am I doing it wrong ?
>>>>
>>>> --
>>>> Maël Nison
>>>> Epitech 2014, Paris - Astek
>>>>
>>>
>>>
>>>
>>> --
>>> Maël Nison
>>> Epitech 2014, Paris - Astek
>>>
>>
>>
>>
>> --
>> Maël Nison
>> JS Github hipster, Assistant C++ chez Epitech
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list