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

[deadal nix]

Updated patch with explanations in comments.


2014-07-02 2:37 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: Wednesday, July 2, 2014 1:50:31 AM
> > Subject: Re: [PATCH] [tablegen] Resolve complex def identifiers when
> instanciating multiclass
> >
> > 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 ?
> >
>
> Yes, please add to the patch a detailed comment to the code explaining
> this.
>
>  -Hal
>
> >
> > 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
> >
> >
>
> --
> 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/20140704/a46a2a84/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-tablegen-Resolve-complex-def-identifiers-when-instan.patch
Type: text/x-patch
Size: 5646 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140704/a46a2a84/attachment.bin>