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

[Dan Gohman]

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?

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.

> +  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.

> -  assert(AsmInfo && "MCAsmInfo not initialized."
> +  assert(AsmInfo && "MCAsmInfo not initialized. "

This probably isn't supposed to be in the patch.

Dan



On Wed, Jun 5, 2013 at 1:05 AM, Bill Wendling <wendling at apple.com> wrote:

> Here's a better patch (I missed some code in one of the 'clones').
>
> -bw
>
>
>
>
> On Jun 5, 2013, at 12:12 AM, Bill Wendling <wendling at apple.com> wrote:
>
> > Hi all!
> >
> > This patch is the culmination of the attributes changes I did last year.
> It changes the back-end when target options change. For instance, if one
> function was compiled with the `-msoft-float' command line option and
> another was compiled without that flag, then we want functions to generate
> the correct code, even if their modules are merged.
> >
> > So what does this patch do?
> >
> > When the target options change, it regenerates the TargetMachine object.
> It does this using a placement-new method, because a reference to the
> TargetMachine object is stored in several places. So it's important to
> reuse the memory address. (The same is true for the TargetConfigPass object
> that the TargetMachine creates.)
> >
> > Why do we need to regenerate the TargetMachine?
> >
> > The back-end code was built around the assumption that once the
> TargetMachine object was generated, none of the state it built up would
> change. For instance, how instruction selection treats specific types may
> depend on the target options. It is a very long-term project to move to a
> system where the function's attributes are checked directly. That said,
> this isn't a "hack", but could be looked upon as an intermediate step from
> what we have to what would be ideal.
> >
> > I ran some tests, and there doesn't appear to be a difference in
> compilation time for the normal case --- one where the module contains
> functions generated with the same command line options.
> >
> > Please review this patch and submit your comments and flamage.
> >
> > -bw
> >
> > <changing-target-options.patch>
> >
> > _______________________________________________
> > 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130605/b8050d29/attachment.html>