[LLVMdev] Fwd: Re: [PATCH] [TABLEGEN] Do not crash on intrinsics with names longer than 40 characters

[Eric Christopher]

Hi Alp,

On Thu, Jul 17, 2014 at 6:25 AM, Alp Toker <alp at nuanti.com> wrote:
> Hi Manuel,
>
> Here's another commit authored through the web interface where no discussion
> or reviewership information is apparent on the mailing list.

If you look at the phab review, it's the same there. The only changes
on the review actually went to the mailing list.

-eric

>
> All we see in cases like this are a few unthreaded list posts by the
> original author followed by an SVN revision number:
>
> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140714/226166.html
>
> For any patch that's submitted for review, especially in a core module like
> tablegen, I think it's worth putting the review discussion in a public place
> so we can keep track.
>
>
>
>
>
>
>
> -------- Original Message --------
> Subject:        Re: [PATCH] [TABLEGEN] Do not crash on intrinsics with names
> longer than 40 characters
> Date:   Thu, 17 Jul 2014 11:32:18 +0000
> From:   Justin Holewinski <justin.holewinski at gmail.com>
> Reply-To:       reviews+D4537+public+fddcf5969927efcd at reviews.llvm.org
> To:     justin.holewinski at gmail.com
> CC:     llvm-commits at cs.uiuc.edu
>
>
>
> Closed by commit rL213253 (authored by @jholewinski).
>
> REPOSITORY
>   rL LLVM
>
> http://reviews.llvm.org/D4537
>
> Files:
>   llvm/trunk/test/TableGen/intrinsic-long-name.td
>   llvm/trunk/utils/TableGen/IntrinsicEmitter.cpp
>
> Index: llvm/trunk/utils/TableGen/IntrinsicEmitter.cpp
> ===================================================================
> --- llvm/trunk/utils/TableGen/IntrinsicEmitter.cpp
> +++ llvm/trunk/utils/TableGen/IntrinsicEmitter.cpp
> @@ -129,8 +129,9 @@
>    for (unsigned i = 0, e = Ints.size(); i != e; ++i) {
>      OS << "    " << Ints[i].EnumName;
>      OS << ((i != e-1) ? ", " : "  ");
> -    OS << std::string(40-Ints[i].EnumName.size(), ' ')
> -      << "// " << Ints[i].Name << "\n";
> +    if (Ints[i].EnumName.size() < 40)
> +      OS << std::string(40-Ints[i].EnumName.size(), ' ');
> +    OS << " // " << Ints[i].Name << "\n";
>    }
>    OS << "#endif\n\n";
>  }
> Index: llvm/trunk/test/TableGen/intrinsic-long-name.td
> ===================================================================
> --- llvm/trunk/test/TableGen/intrinsic-long-name.td
> +++ llvm/trunk/test/TableGen/intrinsic-long-name.td
> @@ -0,0 +1,32 @@
> +// RUN: llvm-tblgen -gen-intrinsic %s | FileCheck %s
> +// XFAIL: vg_leak
> +
> +class IntrinsicProperty;
> +
> +class ValueType<int size, int value> {
> +  string Namespace = "MVT";
> +  int Size = size;
> +  int Value = value;
> +}
> +
> +class LLVMType<ValueType vt> {
> +  ValueType VT = vt;
> +}
> +
> +class Intrinsic<string name, list<LLVMType> param_types = []> {
> +  string LLVMName = name;
> +  bit isTarget = 0;
> +  string TargetPrefix = "";
> +  list<LLVMType> RetTypes = [];
> +  list<LLVMType> ParamTypes = param_types;
> +  list<IntrinsicProperty> Properties = [];
> +}
> +
> +def iAny : ValueType<0, 254>;
> +def llvm_anyint_ty : LLVMType<iAny>;
> +
> +// Make sure we generate the long name without crashing
> +// CHECK:
> this_is_a_really_long_intrinsic_name_but_we_should_still_not_crash   //
> llvm.this.is.a.really.long.intrinsic.name.but.we.should.still.not.crash
> +def int_foo : Intrinsic<"llvm.foo", [llvm_anyint_ty]>;
> +def int_this_is_a_really_long_intrinsic_name_but_we_should_still_not_crash
> :
> Intrinsic<"llvm.this.is.a.really.long.intrinsic.name.but.we.should.still.not.crash",
> [llvm_anyint_ty]>;
> +
>
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>