[PATCH,RFC] allow signext attribute (for Power).
Hal Finkel
hfinkel at anl.gov
Mon Jun 2 09:35:00 PDT 2014
----- Original Message -----
> From: "Will Schmidt" <will_schmidt at vnet.ibm.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Ulrich Weigand" <Ulrich.Weigand at de.ibm.com>, "William J. Schmidt" <wschmidt at linux.vnet.ibm.com>, "llvm cfe"
> <cfe-commits at cs.uiuc.edu>
> Sent: Monday, June 2, 2014 9:44:17 AM
> Subject: Re: [PATCH,RFC] allow signext attribute (for Power).
>
> On Fri, 2014-05-30 at 17:02 -0500, Hal Finkel wrote:
> > ----- Original Message -----
> > > From: "Will Schmidt" <will_schmidt at vnet.ibm.com>
> > > To: "Hal Finkel" <hfinkel at anl.gov>
> > > Cc: "Ulrich Weigand" <Ulrich.Weigand at de.ibm.com>, "William J.
> > > Schmidt" <wschmidt at linux.vnet.ibm.com>, "llvm cfe"
> > > <cfe-commits at cs.uiuc.edu>
> > > Sent: Friday, May 30, 2014 4:41:08 PM
> > > Subject: Re: [PATCH,RFC] allow signext attribute (for Power).
> > >
> > > On Fri, 2014-05-30 at 14:21 -0500, Hal Finkel wrote:
> > > > Hi Will,
> > > >
> > > > Thanks for updating this. One minor remark:
> > > >
> > > > I find 'i32 {{[a-z].*}}5' and '{{[a-z].*}}i32' slightly
> > > > confusing,
> > > > because it gives the
> > > > impression that we're matching some kind of prefix (and we're
> > > > taking advantage of FileCheck's flexible whitespace matching
> > > > in a fairly odd way). I think it would be better to write:
> > > > {{[a-z].*}} i32 and i32 {{[a-z].*}} 5
> > >
> > > Agreed, and thats the first way I tried.. :-) That causes the
> > > tests
> > > on
> > > x86* (and elsewhere) to fail, due to the additional space in the
> > > resulting "i32 5" string.
> >
> > Ah, interesting. Okay, well then how about {{[a-z]*[ ]?}} ?
>
> If I go as far as searching for an optional space, I could probably
> pull
> the i32 substring directly into the curly braces. so either of the
> two
> variations like so:
> // CHECK-LABEL: define {{[a-z]*[ ]?}}i32 @main(i32 {{[a-z]*[
> ]?}}%argc, i8** %argv)
> // CHECK-LABEL: define {{[a-z]*[ ]?i32}} @main({{i32[ ]?[a-z]*}}
> %argc, i8** %argv)
You're right, maybe pulling the i32 into the curly braces is a bit cleaner. Either way, it now looks like we're matching an optional preceding keyword, and I'm happy :-)
Thanks again,
Hal
>
> and for contrast, here is the initial variation.
> // CHECK-LABEL: define {{.*}}i32 @main(i32 {{.*}}%argc, i8** %argv)
>
> Pick one...
>
>
> >
> > -Hal
> >
> > >
> > > >
> > > > Otherwise, LGTM.
> > >
> > > The period "." in the wildcard changes actually causes a fail on
> > > anything that doesn't have an attribute, so there will be one
> > > more
> > > rev
> > > to this patch without that. Once I verify locally that I
> > > didn't
> > > regress anything, I'll likely commit this one on Monday..
> > >
> > > Thanks,
> > > -Will
> > >
> > > > -Hal
> > > >
> > > > ----- Original Message -----
> > > > > From: "Will Schmidt" <will_schmidt at vnet.ibm.com>
> > > > > To: "Hal Finkel" <hfinkel at anl.gov>
> > > > > Cc: "Ulrich Weigand" <Ulrich.Weigand at de.ibm.com>, "William J.
> > > > > Schmidt" <wschmidt at linux.vnet.ibm.com>, "llvm cfe"
> > > > > <cfe-commits at cs.uiuc.edu>
> > > > > Sent: Friday, May 30, 2014 2:12:40 PM
> > > > > Subject: Re: [PATCH,RFC] allow signext attribute (for Power).
> > > > >
> > > > > > I think this is a reasonable point, but I don't want to go
> > > > > > too
> > > > > > far.
> > > > > > How about using [a-z]*.
> > > > >
> > > > > RFC/Revised patch attached. This also includes changes to
> > > > > handle
> > > > > the
> > > > > debug-info-same-line.cpp testcase failure.
> > > > >
> > > > > Update tests to handle a signext, zeroext, or other
> > > > > unexpected
> > > > > attribute. Use wildcard [a-z].* form.
> > > > >
> > > > >
> > > > > Thanks,
> > > > > -Will
> > > > >
> > > > >
> > > > >
> > > > > diff --git
> > > > > a/tools/clang/test/CodeGenCXX/debug-info-same-line.cpp
> > > > > b/tools/clang/test/CodeGenCXX/debug-info-same-line.cpp
> > > > > index c6216b3..b3e0614 100644
> > > > > --- a/tools/clang/test/CodeGenCXX/debug-info-same-line.cpp
> > > > > +++ b/tools/clang/test/CodeGenCXX/debug-info-same-line.cpp
> > > > > @@ -35,8 +35,8 @@ int main() {
> > > > >
> > > > > // Check that we don't give column information (and thus end
> > > > > up
> > > > > with
> > > > > distinct
> > > > > // line entries) for two non-inlined calls on the same line.
> > > > > -// CHECK: call {{.*}}noinline{{.*}}(i32 5, i32 6), !dbg
> > > > > [[NOINLINE:![0-9]*]]
> > > > > -// CHECK: call {{.*}}noinline{{.*}}(i32 7, i32 8), !dbg
> > > > > [[NOINLINE]]
> > > > > +// CHECK: call {{.*}}noinline{{.*}}(i32 {{[a-z].*}}5, i32
> > > > > {{[a-z].*}}6), !dbg [[NOINLINE:![0-9]*]]
> > > > > +// CHECK: call {{.*}}noinline{{.*}}(i32 {{[a-z].*}}7, i32
> > > > > {{[a-z].*}}8), !dbg [[NOINLINE]]
> > > > >
> > > > > // FIXME: These should be separate locations but because the
> > > > > two
> > > > > calls have the
> > > > > // same line /and/ column, they get coalesced into a single
> > > > > inlined
> > > > > call by
> > > > > @@ -50,8 +50,8 @@ int main() {
> > > > > // Even if the functions are marked inline but do not get
> > > > > inlined,
> > > > > they
> > > > > // shouldn't use column information, and thus should be at
> > > > > the
> > > > > same
> > > > > debug
> > > > > // location.
> > > > > -// CHECK: call {{.*}}inlsum{{.*}}(i32 13, i32 14), !dbg
> > > > > [[INL_FIRST:![0-9]*]]
> > > > > -// CHECK: call {{.*}}inlsum{{.*}}(i32 15, i32 16), !dbg
> > > > > [[INL_SECOND:![0-9]*]]
> > > > > +// CHECK: call {{.*}}inlsum{{.*}}(i32 {{[a-z].*}}13, i32
> > > > > {{[a-z].*}}14), !dbg [[INL_FIRST:![0-9]*]]
> > > > > +// CHECK: call {{.*}}inlsum{{.*}}(i32 {{[a-z].*}}15, i32
> > > > > {{[a-z].*}}16), !dbg [[INL_SECOND:![0-9]*]]
> > > > >
> > > > > // [[FIRST_INLINE]] =
> > > > > // [[SECOND_INLINE]] =
> > > > > diff --git a/tools/clang/test/Modules/cxx-irgen.cpp
> > > > > b/tools/clang/test/Modules/cxx-irgen.cpp
> > > > > index 7c680f8..2a37650 100644
> > > > > --- a/tools/clang/test/Modules/cxx-irgen.cpp
> > > > > +++ b/tools/clang/test/Modules/cxx-irgen.cpp
> > > > > @@ -4,7 +4,7 @@
> > > > >
> > > > > @import cxx_irgen_top;
> > > > >
> > > > > -// CHECK-DAG: call i32 @_ZN8CtorInitIiE1fEv(
> > > > > +// CHECK-DAG: call {{[a-z].*}}i32 @_ZN8CtorInitIiE1fEv(
> > > > > CtorInit<int> x;
> > > > >
> > > > > @import cxx_irgen_left;
> > > > > diff --git a/tools/clang/test/OpenMP/parallel_codegen.cpp
> > > > > b/tools/clang/test/OpenMP/parallel_codegen.cpp
> > > > > index 28505ab..b678ec2 100644
> > > > > --- a/tools/clang/test/OpenMP/parallel_codegen.cpp
> > > > > +++ b/tools/clang/test/OpenMP/parallel_codegen.cpp
> > > > > @@ -34,14 +34,14 @@ int main (int argc, char **argv) {
> > > > > return tmain(argv);
> > > > > }
> > > > >
> > > > > -// CHECK-LABEL: define i32 @main(i32 %argc, i8** %argv)
> > > > > +// CHECK-LABEL: define {{[a-z].*}}i32 @main(i32
> > > > > {{[a-z].*}}%argc,
> > > > > i8** %argv)
> > > > > // CHECK: [[AGG_CAPTURED:%.+]] = alloca %struct.anon
> > > > > // CHECK: [[ARGC_REF:%.+]] = getelementptr inbounds
> > > > > %struct.anon* [[AGG_CAPTURED]], i32 0, i32 0
> > > > > // CHECK-NEXT: store i32* {{%[a-z0-9.]+}}, i32**
> > > > > [[ARGC_REF]]
> > > > > // CHECK-NEXT: [[BITCAST:%.+]] = bitcast %struct.anon*
> > > > > [[AGG_CAPTURED]] to i8*
> > > > > // CHECK-NEXT: call void (%ident_t*, i32, void (i32*, i32*,
> > > > > ...)*,
> > > > > ...)* @__kmpc_fork_call(%ident_t* [[DEF_LOC_2]], i32 1, void
> > > > > (i32*,
> > > > > i32*, ...)* bitcast (void (i32*, i32*, %struct.anon*)*
> > > > > @__captured_stmt to void (i32*, i32*, ...)*), i8*
> > > > > [[BITCAST]])
> > > > > // CHECK-NEXT: [[ARGV:%.+]] = load i8*** {{%[a-z0-9.]+}}
> > > > > -// CHECK-NEXT: [[RET:%.+]] = call i32
> > > > > [[TMAIN:@.+tmain.+]](i8**
> > > > > [[ARGV]])
> > > > > +// CHECK-NEXT: [[RET:%.+]] = call {{[a-z].*}}i32
> > > > > [[TMAIN:@.+tmain.+]](i8** [[ARGV]])
> > > > > // CHECK-NEXT: ret i32 [[RET]]
> > > > > // CHECK-NEXT: }
> > > > > // CHECK-DEBUG-LABEL: define i32 @main(i32 %argc, i8**
> > > > > %argv)
> > > > > @@ -68,7 +68,7 @@ int main (int argc, char **argv) {
> > > > > // CHECK-NEXT: [[ARGC_PTR_REF:%.+]] = getelementptr
> > > > > inbounds
> > > > > %struct.anon* [[CONTEXT_PTR]], i32 0, i32 0
> > > > > // CHECK-NEXT: [[ARGC_REF:%.+]] = load i32**
> > > > > [[ARGC_PTR_REF]]
> > > > > // CHECK-NEXT: [[ARGC:%.+]] = load i32* [[ARGC_REF]]
> > > > > -// CHECK-NEXT: invoke void [[FOO:@.+foo.+]](i32 [[ARGC]])
> > > > > +// CHECK-NEXT: invoke void [[FOO:@.+foo.+]](i32
> > > > > {{[a-z].*}}[[ARGC]])
> > > > > // CHECK: ret void
> > > > > // CHECK: call void @{{.+terminate.*}}(
> > > > > // CHECK-NEXT: unreachable
> > > > > @@ -86,12 +86,12 @@ int main (int argc, char **argv) {
> > > > > // CHECK-DEBUG-NEXT: unreachable
> > > > > // CHECK-DEBUG-NEXT: }
> > > > >
> > > > > -// CHECK-DAG: define linkonce_odr void [[FOO]](i32 %argc)
> > > > > +// CHECK-DAG: define linkonce_odr void [[FOO]](i32
> > > > > {{[a-z].*}}%argc)
> > > > > // CHECK-DAG: declare void @__kmpc_fork_call(%ident_t*, i32,
> > > > > void
> > > > > (i32*, i32*, ...)*, ...)
> > > > > // CHECK-DEBUG-DAG: define linkonce_odr void [[FOO]](i32
> > > > > %argc)
> > > > > // CHECK-DEBUG-DAG: declare void
> > > > > @__kmpc_fork_call(%ident_t*,
> > > > > i32,
> > > > > void (i32*, i32*, ...)*, ...)
> > > > >
> > > > > -// CHECK: define linkonce_odr i32 [[TMAIN]](i8**
> > > > > %argc)
> > > > > +// CHECK: define linkonce_odr {{[a-z].*}}i32
> > > > > [[TMAIN]](i8**
> > > > > %argc)
> > > > > // CHECK: [[AGG_CAPTURED:%.+]] = alloca %struct.anon.0
> > > > > // CHECK: [[ARGC_REF:%.+]] = getelementptr inbounds
> > > > > %struct.anon.0* [[AGG_CAPTURED]], i32 0, i32 0
> > > > > // CHECK-NEXT: store i8*** {{%[a-z0-9.]+}}, i8****
> > > > > [[ARGC_REF]]
> > > > >
> > > > >
> > > > >
> > > >
> > >
> > >
> > >
> >
>
>
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
More information about the cfe-commits
mailing list