[PATCH] [tablegen] Resolve complex def identifiers when instanciating multiclass

[deadal nix]

No that wouldn't work, and I tried to explain it, but apparently I wasn't
very clear. Sorry for that.

The name is of the def is computed the following way:
 - If it is a simple string, then prepend the prefix and use that.
 - If it is something more complex, then resolve that thing and use that.

If you eagerly resolve arguments, then everything become a string, and the
prefix is added every time. I do think that this would have been a sensible
thing to do when initially working on tablegen, but that would break most
backend, so i don't think that is is something we want to do now.

Is that clearer ?



2014-06-30 21:15 GMT-07:00 Hal Finkel <hfinkel at anl.gov>:

> ----- Original Message -----
> > From: "deadal nix" <deadalnix at gmail.com>
> > To: "Hal Finkel" <hfinkel at anl.gov>
> > Cc: "llvm-commits" <llvm-commits at cs.uiuc.edu>
> > Sent: Monday, June 30, 2014 10:49:28 PM
> > Subject: Re: [PATCH] [tablegen] Resolve complex def identifiers when
> instanciating multiclass
> >
> >
> > As per comment, I removed the {}.
> >
> >
> >
> >
> > 2014-06-30 20:24 GMT-07:00 deadal nix < deadalnix at gmail.com > :
> >
> >
> >
> >
> > The resolution is done after the name resolution because the name
> > creation depend on it. If the name is a simple string, then the
> > multiclass prefix is not added. That seems like a bizarre design to
> > me but it is already workign that way, so I just extended the
> > existing design.
>
> I don't understand what you mean. To be more specific, what if you:
>
> 1. Removed the call to ResolveMulticlassDefArgs in TGParser::ParseDefm
>
> 2. Changed your patch so that the logic read:
>
> +    ResolveMulticlassDefArgs(MC, CurRec, DefmPrefixRange.Start,
> Lex.getLoc(),
> +                             TArgs, TemplateVals, false/*Delete args*/);
> +
> +    DefName = CurRec->getNameInit();
> +    DefNameString = dyn_cast<StringInit>(DefName);
> +
> +    if (!DefNameString)
> +      DefName = DefName->convertInitializerTo(StringRecTy::get());
> ...
>
> so that we always call ResolveMulticlassDefArgs and then try to resolve
> the name in InstantiateMulticlassDef. Is there some reason that would not
> work? It seems as though it would be cleaner.
>
>  -Hal
>
> >
> > Changing this design would probably break most backends, and it is
> > probably not worth it. Removing the {} and coming back with an
> > updated patch.
> >
> >
> >
> >
> > 2014-06-30 11:39 GMT-07:00 Hal Finkel < hfinkel at anl.gov > :
> >
> >
> >
> >
> > ----- Original Message -----
> > > From: "deadal nix" < deadalnix at gmail.com >
> > > To: llvm-commits at cs.uiuc.edu
> > > Sent: Monday, June 30, 2014 1:24:44 PM
> > > Subject: Re: [PATCH] [tablegen] Resolve complex def identifiers
> > > when instanciating multiclass
> > >
> > >
> > >
> > > PING ?
> > >
> >
> > Please wait 1 week in between pings for non-urgent patches. That
> > having been said, thanks for working on this.
> >
> > + DefName = CurRec->getNameInit();
> > + DefNameString = dyn_cast<StringInit>(DefName);
> > +
> > + // OK the pattern is more complex than simply using NAME.
> > + // Let's use the heavy weaponery.
> > + if (!DefNameString) {
> > + ResolveMulticlassDefArgs(MC, CurRec, DefmPrefixRange.Start,
> > Lex.getLoc(),
> > + TArgs, TemplateVals, false/*Delete args*/);
> > +
> > + DefName = CurRec->getNameInit();
> > + DefNameString = dyn_cast<StringInit>(DefName);
> >
> > Why are we only calling ResolveMulticlassDefArgs when we can't
> > resolve the name? It seems somewhat funny to have the argument
> > resolution state depend on the kind of name that is used. Does
> > something break if you always call this?
> >
> >
> > + if (!DefNameString) {
> > + DefName = DefName->convertInitializerTo(StringRecTy::get());
> > + }
> >
> > Remove the {} here.
> >
> > Thanks,
> > Hal
> >
> >
> >
> > >
> > >
> > >
> > > 2014-06-26 23:27 GMT-07:00 deadal nix < deadalnix at gmail.com > :
> > >
> > >
> > >
> > > ping !!?!
> > >
> > >
> > >
> > >
> > > 2014-06-24 16:02 GMT-07:00 deadal nix < deadalnix at gmail.com > :
> > >
> > >
> > >
> > >
> > > ping \o/
> > >
> > >
> > >
> > >
> > > 2014-06-23 0:22 GMT-07:00 deadal nix < deadalnix at gmail.com > :
> > >
> > >
> > >
> > >
> > > As per title. I ran into tablegen limitations that are annoying,
> > > namely that I couldn't generate custom name for def inside
> > > multiclass. So here is the fix.
> > >
> > >
> > >
> > >
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > >
> >
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
> >
> >
> >
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140701/c729d0b5/attachment.html>