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

Bill Wendling wendling at apple.com
Wed Jun 19 15:47:05 PDT 2013


For those not interested in slugging their way through a ton of refactoring code, here's a cleaner patch.

The executive summary:

This regenerates the TargetMachine object if the any of the values in TargetOptions changes. The regeneration is recursive, because creating TargetMachine pretty much creates all of the other back-end objects that are needed for code generation.

-bw

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


On Jun 18, 2013, at 2:16 PM, Bill Wendling <wendling at apple.com> wrote:

> 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
> 
> <changing-target-options.patch>
> 
> 
> _______________________________________________
> 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