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

[Justin Holewinski]

On Thu, 2014-07-17 at 10:44 -0700, Eric Christopher wrote:
> 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.

Since it was a minor change not affecting functionality, just fixing a
crash, I left the review open for a day for objections and then
committed.

> 
> -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
> >
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------