[PATCH] Remove static initializers from MCSchedModel

[Pete Cooper]

> On Aug 29, 2014, at 3:27 PM, Andrew Trick <atrick at apple.com> wrote:
> 
>> 
>> On Aug 29, 2014, at 2:26 PM, Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
>> 
>>> 
>>> On Aug 29, 2014, at 2:18 PM, Andrew Trick <atrick at apple.com <mailto:atrick at apple.com>> wrote:
>>> 
>>>> 
>>>> On Aug 29, 2014, at 2:14 PM, Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
>>>> 
>>>>> 
>>>>> On Aug 29, 2014, at 2:10 PM, Andrew Trick <atrick at apple.com <mailto:atrick at apple.com>> wrote:
>>>>> 
>>>>> 
>>>>>> On Aug 29, 2014, at 9:38 AM, Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
>>>>>> 
>>>>>> Hi all
>>>>>> 
>>>>>> Please review this patch to change MCSchedModel to avoid static initializers.  This makes it behavior match other structs with similar uses, e.g., MCWriteProcResEntry and MCProcResourceDesc.
>>>>>> 
>>>>>> This is part of the work to remove all the static initializers from LLVM.
>>>>> 
>>>>> 
>>>>> Great. I can’t remember why we needed a default ctor on MCSchedModel any more. If we can get away with constant static initialization it’s much better.
>>>>> 
>>>>> I would define an LLVM_DELETED_FUNCTION default ctor to make sure we’re not silently breaking some target.
>>>>> 
>>>>> LGTM.
>>>> Thanks Andy.  I’ll add the deleted constructor.
>>>> 
>>>> I’m preparing an updated patch now based on Chandler’s feedback so that we can see how we want to proceed with this.
>>> 
>>> Unfortunately, I don’t think the “delete” keyword will work because you can’t define a default ctor if you want to initialize it with an initializer list. Maybe someone who knows C++11 rules has an idea for how to catch default ctor uses.
>> Yeah, i’m not sure how to do that either.
>>> 
>>> Or you could just do an experiment where you delete the default ctor without changing the initialization and make sure all targets still build. That would be good enough for me.
>> Experiment done.  Everything builds with the attached patch.
>> 
>> For some details as to how this differs from before.  I changed it from a static field to a function to return a default initialized model.  Unfortunately this requires more source changes as now we can’t have pointers to the default model.  In fact cleaning this up basically removes pointers to MCSchedModel from all the compiler.  
>> 
>> Perhaps this is what we want.  I’m open to discuss whether this is a good idea.
> 
> Pete, this is moving in a good direction.
> 
> I think you should remove isDefaultModel. I don’t see any purpose and it’s not maintainable.
Actually, I think you’re right.  I was looking to replace the code in Disassembler.cpp which used to do ‘if (!SCModel…)’.  However, a null model in the old code didn’t even mean default, but instead uninitialized.

Would it be fair to add another field which tells us if a model is initialized?  Otherwise i’m not going to be able to handle this case?  Perhaps I can fold it in to the ‘CompleteModel’ bool and have an enum for whether a model is ‘Uninitialized, Complete, InComplete’.

> 
> It would be nice to move the default values from MCSchedule into targetSchedule.td. Then we can get rid of the confusing "-1 == use default” logic and consolidate comments. Hopefully that’s possible after your changes.
Yes, that change can be done if we can find a way to construct a default model in tablegen instead of the GetDefaultModel method.  Note that GetDefaultModel is really just equivalent to the old default constructor.  So we’re no closer to moving the defaults to td, but no farther either.

Pete
> 
> -Andy

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140829/a2a946ec/attachment.html>