[PATCH] Tablegen binary literals should be sized

[Pete Cooper]

On Jul 31, 2014, at 3:03 PM, Sean Silva <chisophugis at gmail.com> wrote:

> Did you see anywhere in the docs that needed updating?
Yeah, here’s what I’ve got so far.  I’m not 100% sure on the how well i’ve organized the changes in LangRef.rst, but I think its close to explaining that we can now nest { } values.

> 
> 
> Regarding patch 0005, thankfully those were benign since they just had leading 0's. Can you doublecheck the diagnostic quality? We need to give a clear indication that what is wrong so that users don't get left scratching their heads.
I added a better error message in r214374.  As an example from those that were fixed in patch 5, this is what I saw before I made a fix:

lib/Target/Sparc/SparcInstrVIS.td:260:16: error: Value 'VISInst:opfval' of type 'bits<9>' is incompatible with initializer '{ 0, 0, 0, 0, 0, 1, 0, 1, 1, 0 }' of type bit initializer with length 10'
def UMULXHI  : VISInst<0b0000010110, "umulxhi", I64Regs>;
               ^
> 
> 
> +  /// resolveListElementReference - This method is used to implement
> +  /// VarListElementInit::resolveReferences.  If the list element is resolvable
> +  /// now, we return the resolved value, otherwise we return null.
> +  Init *resolveListElementReference(Record &R, const RecordVal *RV,
> +                                    unsigned Elt) const override {
> +    llvm_unreachable("Illegal element reference off bits<n>");
> +  }
> 
> This saddens me... all this TableGen code really needs to be refactored...
> There may be a simple refactor for this resolveListElementReference situation that does not require plunging into the murky depths of the resolution machinery.
I’m open to suggestions :)  Otherwise, is there anything you can think of thats better than having BitsInit now being a TypedInit subclass?

Thanks,
Pete
> 
> 
> -- Sean Silva
> 
> 
> 
> On Thu, Jul 31, 2014 at 2:48 PM, Pete Cooper <peter_cooper at apple.com> wrote:
> Hi all
> 
> Moving this from LLVMDev (http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-July/075223.html) now that i have patches i’d welcome reviews of.  The following is the initial summary from that email thread.
> 
> "Currently tablegen parses binary literals such as 0b011 and immediately turns them in to integers internally.  Due to this, 0b011 is a 2-bit value because tablegen will happily drop leading zeroes on integers.
> 
> I propose that we change this, and store binary literals with a size.  I think this is much more natural, as when the user writes a value with 3 bits, I think they expect to get a value with 3 bits, and not a 2-bit value which is silently truncated and later silently zero extended.
> 
> Given this, I would then like to extend bits initializers to accept these new sized binary literals.  For example, I would like to be able to write
> 
> bits<2> opc;
> bits<8> x = { opc, 0b1100, rd{1}, rs{0} }
> 
> This would let us write some encodings much more concisely, instead of having to put each initializer on its own line.”
> 
> For those interested in the low level details, i’ve changed binary literals such as 0b011 from creating an integer ‘3’, to creating a BitsInit ‘{ 0, 1, 1}’.  As Sean suggested, this was better than introducing yet another type just for binary literals.
> 
> As part of this, BitsInit values now had to be able to be typed.  This is because we had uses of binary literals which expected to be typed (for example list<bits<2>> [0b00, 0b01]).  I’ve moved BitsInit from being a subclass of Init to a subclass of TypedInit to allow this.
> 
> Note that patches 2 and 3 really need to be committed together.  I can squash them before commit, but just wanted to present the changes independently.
> 
> Patch 5 can also be committed without any of the others, but just shows that this did actually catch some useful cases where we mismatched sizes of binary expressions.
> 
> Thanks,
> Pete
> 
> 
> 
> 
> 
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140731/27fed779/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: docs.diff
Type: application/octet-stream
Size: 1705 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140731/27fed779/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140731/27fed779/attachment-0001.html>