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

[David Blaikie]

On Jun 18, 2013 1:36 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.

No dou t Dan will chime in with his own thoughts, but I don't understand
why caching of objects would make the placement new strategy any more
reliable than the 'mutate all the members' option. Could you explain by way
of an example of the outcome you're concerned might occur?

It seems to me if something else is caching a reference/pointer to a member
of TargetMachine then mutating that member would be at least as useful as
constructing a new object in its place, no? And any indirect members (eg,
where TM points to something and some other code caches that  pointer too)
is going to be as broken in both (presuming the ctor constructed a new
thing to point to, leaving the other code holding a dangling pointer) and
maybe less broken in the non-placement case because the update function
could potentially update the existing pointed-to thing.

>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.
>
> -bw
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130620/a744d5a9/attachment.html>