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

[Eric Christopher]

On Thu, Jul 17, 2014 at 10:43 AM, Alp Toker <alp at nuanti.com> wrote:
>
> On 17/07/2014 20:27, Eric Christopher wrote:
>>
>> 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.
>
>
> Does that mean no review comments were posted in the first place, or did
> they go missing?

Unknown unknowns. There are no comments on the review in phab.

-eric

>
>
>> The only changes
>> on the review actually went to the mailing list.
>
>
> Sure, Manuel asked to forward incidents involving the web review system
> instead of directly checking in the individual contributor as we were doing
> before.
>
>
>
>>
>> -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
>>>
>
> --
> http://www.nuanti.com
> the browser experts
>