[LLVMdev] Tablegen binary literals

[Sean Silva]

On Thu, Jul 31, 2014 at 10:08 AM, Pete Cooper <peter_cooper at apple.com>
wrote:

>
> On Jul 31, 2014, at 9:02 AM, Sean Silva <chisophugis at gmail.com> wrote:
>
> I think it would make sense to change this behavior to follow the behavior
> that Pete describes. I don't have the code handy but my guess is that what
> the code is doing here is requesting each element in the braces of a
> bits<n> initializer to be casted to `bit`; in that case, we would want to
> change that code to first try casting to bits and give that preference (and
> issues relevant diagnostics). Pete, is that what your patch does? Can you
> attach your patches?
>
> Yeah, thats exactly it.  Here’s a WIP patch which fixes that particular
> issue.  I’m building the rest of the compiler now to see if anything breaks
> with that applied.
>
>
>
>
> Of course, before changing the behavior, we would probably want to place
> an assert on that codepath to catch all existing cases where the behavior
> would change.
>
> Sounds like a good plan.  I’ll try that and see what happens.
>
> Also it is important to scan all the docs in docs/TableGen/ and make any
> necessary updates (if there is nowhere that needs to be updated, then
> documentation needs to be *added* covering this behavior).
>
> I haven’t done that yet, but good point.  Depending on what changes you
> all think are acceptable, i’ll update the documentation appropriately.
>
> Finally, we should post a notice to LLVMdev for out-of-tree maintainers
> including information about how to add the necessary assert to check for
> places where this behavior would change their .td files.
>
> Sure thing.
>
> And here’s the other 2 patches.  The first sizes binary literals.
>

Regarding this patch, does it need to create a whole new Init class? I
would try to avoid that if at all possible. Could we just have 0b0111 be a
shorthand for a bits<4> BitsInit? Or maybe store some extra data in IntInit?


>  The second allows them to initialize multiple bits inside { }.  Note that
> i’m only handling certain cases inside the { } initializer.  I’m happy to
> refactor the code and handle other cases if necessary.
>

I don't like the special casing going on in this patch. Could we just
switch BitsInit to store pointers to *both* BitInit and BitsInit (it
actually can already sort of do that since it has a std::vector<Init*>)?

I just feel like this is adding a lot of complexity to TableGen where the
behavior that you want could largely be accomplished by removing complexity.

-- Sean Silva


>
>
>
>
> Thanks,
> Pete
>
>
> -- Sean Silva
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140731/907a9301/attachment.html>