[PATCH] Changing the Back-end when Target Options Change

[Bill Wendling]

On Jun 18, 2013, at 1:35 AM, Bill Wendling <wendling at apple.com> wrote:

> On Jun 5, 2013, at 4:39 PM, Dan Gohman <dan433584 at gmail.com> wrote:
> 
>> Hi Bill,
>> 
>>> +    TM.cloneWithOptions(mf);
>> 
>> cloneWithOptions is declared to return a pointer. Since this is the only place where it's called, should it return void instead?
>> 
> I made a few changes after sending the initial email. I'll see if 'void' now makes sense. :)
> 
>> Also, the word "clone" has the connotation that a new object is being created, but what this actually appears to do is reconstruct an object in place.
>> 
> I agree. :-) I changed it to "regenerate" instead. What do you think?
> 
>>> +  this->~ARMTargetMachine();
>>> +  TargetMachine *NewTM = new (this)
>>> +    ARMTargetMachine(TheTarget, targetTriple, targetCPU, targetFS,
>>> +                     Opts, RM, CM, OptLvl);
>>> +
>>> +  if (TPC)
>>> +    PassConfig = TPC->cloneWithTargetMachine(NewTM);
>> 
>> There is some disagreement about whether this code is valid (specifically, whether it's valid to reference members of this after the placement new). I think this code would be clearer and safer if TargetMachine and its subclasses merely had a plain method which could re-initialize all needed fields in place, instead of using placement new. Any reference members that they contain would need to be changed to pointers, so that they can be reassigned, but I think the code would be cleaner, and it'd have less risk of violating C++ rules.
>> 
> I can check into that, but it's a bit tricky. Basically, *so* much caching of objects is happening that it's hard to track them all down. I don't like using the placement 'new' hack here. But it at least gets me unstuck for the time being. Moving to a cleaner system is vastly preferable of course.
> 
> As for it being valid or invalid...I need the wisdom of a language lawyer for that. :) I'll see if there's a better way to do this without potentially violating the standard.
> 
Here is the updated patch. It should address all of your concerns above. It also appears to work...

-bw

-------------- next part --------------
A non-text attachment was scrubbed...
Name: changing-target-options.patch
Type: application/octet-stream
Size: 227330 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130618/0ab054aa/attachment.obj>
-------------- next part --------------