[llvm] r216757 - Tablegen fixes for new syntax when initializing bits from variables.

[Pete Cooper]

> On Aug 29, 2014, at 1:27 PM, Sean Silva <chisophugis at gmail.com> wrote:
> 
> 
> 
> 
> On Fri, Aug 29, 2014 at 12:41 PM, Jean-Luc Duprat <jduprat at apple.com <mailto:jduprat at apple.com>> wrote:
> Author: jduprat
> Date: Fri Aug 29 14:41:04 2014
> New Revision: 216757
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=216757&view=rev <http://llvm.org/viewvc/llvm-project?rev=216757&view=rev>
> Log:
> Tablegen fixes for new syntax when initializing bits from variables.
> Followup to r215086.
> 
> 
> Modified:
>     llvm/trunk/lib/TableGen/TGParser.cpp
>     llvm/trunk/test/TableGen/BitsInit.td
> 
> Modified: llvm/trunk/lib/TableGen/TGParser.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/TableGen/TGParser.cpp?rev=216757&r1=216756&r2=216757&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/TableGen/TGParser.cpp?rev=216757&r1=216756&r2=216757&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/TableGen/TGParser.cpp (original)
> +++ llvm/trunk/lib/TableGen/TGParser.cpp Fri Aug 29 14:41:04 2014
> @@ -1321,6 +1321,15 @@ Init *TGParser::ParseSimpleValue(Record
>            NewBits.push_back(BI->getBit((e - i) - 1));
>          continue;
>        }
> +      // bits<n> can also come from variable initializers.
> +      if (VarInit *VI = dyn_cast<VarInit>(Vals[i])) {
> +        if (BitsRecTy *BitsRec = dyn_cast<BitsRecTy>(VI->getType())) {
> +          for (unsigned i = 0, e = BitsRec->getNumBits(); i != e; ++i)
> +            NewBits.push_back(VI->getBit((e - i) - 1));
> +          continue;
> +        }
> +        // Fallthrough to try convert this to a bit.
> +      }
> 
> Please add a FIXME to this loop for removing the duplication with the one just above. That is a valuable use case that reduces the problem space for refactoring the Record.h API's.
>  
>        // All other values must be convertible to just a single bit.
>        Init *Bit = Vals[i]->convertInitializerTo(BitRecTy::get());
>        if (!Bit) {
> 
> Modified: llvm/trunk/test/TableGen/BitsInit.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/TableGen/BitsInit.td?rev=216757&r1=216756&r2=216757&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/TableGen/BitsInit.td?rev=216757&r1=216756&r2=216757&view=diff>
> ==============================================================================
> --- llvm/trunk/test/TableGen/BitsInit.td (original)
> +++ llvm/trunk/test/TableGen/BitsInit.td Fri Aug 29 14:41:04 2014
> @@ -16,7 +16,7 @@ def a {
>  // CHECK:   bits<2> opc = { 0, 1 };
>  // CHECK:   bits<2> opc2 = { 1, 0 };
>  // CHECK:   bits<1> opc3 = { 1 };
> -// CHECK:   bits<2> a = { ?, ? };
> +// CHECK:   bits<2> a;
> 
> Can you provide an explanation for why this is printing differently with this patch?
I took a look at this for Jean-Luc.

What I think is happening is that we used to reject constructing { opc, opc2 } because they were both bits<n> variables.  This caused us to hit the error when parsing the contents of { }, i.e.,

Error(BraceLoc, "Element #" + utostr(i) + " (" + Vals[i]->getAsString()+
              ") is not convertable to a bit”);

Now, we don’t hit that error, but instead construct a bits<4> value for the RHS of the assignment.  Later we get to setValue to try assign that to bits<2> a.  This fails because of a size mismatch and reports:

      InitType = (Twine("' of type bit initializer with length ") +
                  Twine(BI->getNumBits())).str();

Now for a little guessing, I believe in the original case, if we threw an error in { } parsing, we have (construct?) a default { ?, ? } value for ‘a’.  With this change, given that we throw an error in setValue, it looks like we no longer attach any initializer to ‘a’ as this is the method that was going to actually set it.

Now whether we want to put { ?, ? } on ‘a’ in either or both cases is a valid question.  Given that there are errors in either case, i’m not too worried, as users are going to have to fix the errors anyway.

Thanks,
Pete
> 
> -- Sean Silva
>  
>  // CHECK:   bits<2> b = { 1, 0 };
>  // CHECK:   bits<2> c = { 1, 1 };
>  // CHECK: }
> @@ -51,6 +51,7 @@ def {
> 
>    bits<16> H;
>    let H{15-0} = { { 0b11001100 }, 0b00110011 };
> +  bits<16> I = { G1, G2 };
> 
>    // Make sure we can initialise ints with bits<> values.
>    int J = H;
> @@ -78,6 +79,7 @@ def {
>  // CHECK: bits<8> G2 = { 0, 1, 1, 0, 0, 1, 0, 0 };
>  // CHECK: bits<8> G3 = { 0, 1, 1, 0, 0, 1, 0, 0 };
>  // CHECK: bits<16> H = { 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1 };
> +// CHECK: bits<16> I = { 0, 1, 1, 0, 0, 1, 0, 0, 0, 1, 1, 0, 0, 1, 0, 0 };
>  // CHECK: int J = 52275;
>  // CHECK: int K = 1;
>  // CHECK: }
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140829/62350032/attachment.html>