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

[Hal Finkel]

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