r366076 - fix unnamed fiefield issue and add tests for __builtin_preserve_access_index intrinsic

Eric Christopher via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 15 17:13:28 PDT 2019


I'm going to cheat and make Nick do it :)

-eric

On Mon, Jul 15, 2019 at 5:12 PM Yonghong Song <yhs at fb.com> wrote:
>
> HI, Eric,
>
> No problem!
>
> I just tried the following cmake (removing -DLLVM_ENABLE_ASSERTIONS=ON which is used in my previous build)
>
> cmake -G "Unix Makefiles" -DLLVM_TARGETS_TO_BUILD="BPF;X86" \
>     -DCMAKE_C_COMPILER=/llvm8/bin/clang \
>     -DCMAKE_CXX_COMPILER=/llvm8/bin/clang++ \
>     -DLLVM_TEMPORARILY_ALLOW_OLD_TOOLCHAIN=ON
>     -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=$PWD/install ..
>
> and cannot reproduce the issue. If you could send me the cmake
> command line which caused failure in your environment. That will
> be great!
>
> Thanks,
>
> Yonghong
>
> On 7/15/19, 4:50 PM, "Eric Christopher" <echristo at gmail.com> wrote:
>
>     Just wanted to let you know I've temporarily reverted this here:
>
>     echristo at jhereg ~/s/llvm-project> git llvm push
>     Pushing 1 commit:
>       ba7decf8c91 Temporarily Revert "fix unnamed fiefield issue and add
>     tests for __builtin_preserve_access_index intrinsic"
>     Sending        cfe/trunk/lib/CodeGen/CGExpr.cpp
>     Sending        cfe/trunk/lib/CodeGen/CodeGenFunction.h
>     Deleting       cfe/trunk/test/CodeGen/builtin-preserve-access-index.c
>     Deleting       cfe/trunk/test/Sema/builtin-preserve-access-index.c
>     Transmitting file data ..done
>     Committing transaction...
>     Committed revision 366155.
>     Committed ba7decf8c91 to svn.
>
>     Since the testcase was causing problems in non-assert builds. Feel
>     free to recommit when you fix that :)
>
>     Thanks!
>
>     -eric
>
>     On Mon, Jul 15, 2019 at 8:42 AM Yonghong Song via cfe-commits
>     <cfe-commits at lists.llvm.org> wrote:
>     >
>     > Author: yhs
>     > Date: Mon Jul 15 08:42:41 2019
>     > New Revision: 366076
>     >
>     > URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D366076-26view-3Drev&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=dtjPkUWyM2j7LOCdes9xK0tSCe4BrvYtJpmJu_el7xE&s=LyV9cu6egVjs0MILQtMXrDRw1r2afyEdzsh_tKDRsQE&e=
>     > Log:
>     > fix unnamed fiefield issue and add tests for __builtin_preserve_access_index intrinsic
>     >
>     > This is a followup patch for https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D61809&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=dtjPkUWyM2j7LOCdes9xK0tSCe4BrvYtJpmJu_el7xE&s=zdwdBj89w175wBRDiJ8ts1YTR7VrNyt4IHp-29y6b8E&e= .
>     > Handle unnamed bitfield properly and add more test cases.
>     >
>     > Fixed the unnamed bitfield issue. The unnamed bitfield is ignored
>     > by debug info, so we need to ignore such a struct/union member
>     > when we try to get the member index in the debug info.
>     >
>     > D61809 contains two test cases but not enough as it does
>     > not checking generated IRs in the fine grain level, and also
>     > it does not have semantics checking tests.
>     > This patch added unit tests for both code gen and semantics checking for
>     > the new intrinsic.
>     >
>     > Signed-off-by: Yonghong Song <yhs at fb.com>
>     >
>     > Added:
>     >     cfe/trunk/test/CodeGen/builtin-preserve-access-index.c
>     >     cfe/trunk/test/Sema/builtin-preserve-access-index.c
>     > Modified:
>     >     cfe/trunk/lib/CodeGen/CGExpr.cpp
>     >     cfe/trunk/lib/CodeGen/CodeGenFunction.h
>     >
>     > Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
>     > URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_CodeGen_CGExpr.cpp-3Frev-3D366076-26r1-3D366075-26r2-3D366076-26view-3Ddiff&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=dtjPkUWyM2j7LOCdes9xK0tSCe4BrvYtJpmJu_el7xE&s=wNn-0ZBP1UL0gaIGHoTeMMTdnpNcjwneAK5I8Y8AaJY&e=
>     > ==============================================================================
>     > --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
>     > +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Mon Jul 15 08:42:41 2019
>     > @@ -3892,6 +3892,23 @@ LValue CodeGenFunction::EmitLValueForLam
>     >    return EmitLValueForField(LambdaLV, Field);
>     >  }
>     >
>     > +/// Get the field index in the debug info. The debug info structure/union
>     > +/// will ignore the unnamed bitfields.
>     > +unsigned CodeGenFunction::getDebugInfoFIndex(const RecordDecl *Rec,
>     > +                                             unsigned FieldIndex) {
>     > +  unsigned I = 0, Skipped = 0;
>     > +
>     > +  for (auto F : Rec->getDefinition()->fields()) {
>     > +    if (I == FieldIndex)
>     > +      break;
>     > +    if (F->isUnnamedBitfield())
>     > +      Skipped++;
>     > +    I++;
>     > +  }
>     > +
>     > +  return FieldIndex - Skipped;
>     > +}
>     > +
>     >  /// Get the address of a zero-sized field within a record. The resulting
>     >  /// address doesn't necessarily have the right type.
>     >  static Address emitAddrOfZeroSizeField(CodeGenFunction &CGF, Address Base,
>     > @@ -3931,7 +3948,7 @@ static Address emitPreserveStructAccess(
>     >        CGF.CGM.getTypes().getCGRecordLayout(rec).getLLVMFieldNo(field);
>     >
>     >    return CGF.Builder.CreatePreserveStructAccessIndex(
>     > -      base, idx, field->getFieldIndex(), DbgInfo);
>     > +      base, idx, CGF.getDebugInfoFIndex(rec, field->getFieldIndex()), DbgInfo);
>     >  }
>     >
>     >  static bool hasAnyVptr(const QualType Type, const ASTContext &Context) {
>     > @@ -4048,7 +4065,7 @@ LValue CodeGenFunction::EmitLValueForFie
>     >            getContext().getRecordType(rec), rec->getLocation());
>     >        addr = Address(
>     >            Builder.CreatePreserveUnionAccessIndex(
>     > -              addr.getPointer(), field->getFieldIndex(), DbgInfo),
>     > +              addr.getPointer(), getDebugInfoFIndex(rec, field->getFieldIndex()), DbgInfo),
>     >            addr.getAlignment());
>     >      }
>     >    } else {
>     >
>     > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
>     > URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_CodeGen_CodeGenFunction.h-3Frev-3D366076-26r1-3D366075-26r2-3D366076-26view-3Ddiff&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=dtjPkUWyM2j7LOCdes9xK0tSCe4BrvYtJpmJu_el7xE&s=4q40rgk-KNfA47o6IGYhKniKcB0guzzlsWerlf_-vME&e=
>     > ==============================================================================
>     > --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
>     > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Mon Jul 15 08:42:41 2019
>     > @@ -2652,6 +2652,9 @@ public:
>     >    /// Converts Location to a DebugLoc, if debug information is enabled.
>     >    llvm::DebugLoc SourceLocToDebugLoc(SourceLocation Location);
>     >
>     > +  /// Get the record field index as represented in debug info.
>     > +  unsigned getDebugInfoFIndex(const RecordDecl *Rec, unsigned FieldIndex);
>     > +
>     >
>     >    //===--------------------------------------------------------------------===//
>     >    //                            Declaration Emission
>     >
>     > Added: cfe/trunk/test/CodeGen/builtin-preserve-access-index.c
>     > URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_CodeGen_builtin-2Dpreserve-2Daccess-2Dindex.c-3Frev-3D366076-26view-3Dauto&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=dtjPkUWyM2j7LOCdes9xK0tSCe4BrvYtJpmJu_el7xE&s=vdxrPX0OhC8bZBm-Sdz6KDlEs6zRaTcwF3AltXpLEgs&e=
>     > ==============================================================================
>     > --- cfe/trunk/test/CodeGen/builtin-preserve-access-index.c (added)
>     > +++ cfe/trunk/test/CodeGen/builtin-preserve-access-index.c Mon Jul 15 08:42:41 2019
>     > @@ -0,0 +1,177 @@
>     > +// RUN: %clang -target x86_64 -emit-llvm -S -g %s -o - | FileCheck %s
>     > +
>     > +#define _(x) (__builtin_preserve_access_index(x))
>     > +
>     > +const void *unit1(const void *arg) {
>     > +  return _(arg);
>     > +}
>     > +// CHECK: define dso_local i8* @unit1(i8* %arg)
>     > +// CHECK-NOT: llvm.preserve.array.access.index
>     > +// CHECK-NOT: llvm.preserve.struct.access.index
>     > +// CHECK-NOT: llvm.preserve.union.access.index
>     > +
>     > +const void *unit2(void) {
>     > +  return _((const void *)0xffffffffFFFF0000ULL);
>     > +}
>     > +// CHECK: define dso_local i8* @unit2()
>     > +// CHECK-NOT: llvm.preserve.array.access.index
>     > +// CHECK-NOT: llvm.preserve.struct.access.index
>     > +// CHECK-NOT: llvm.preserve.union.access.index
>     > +
>     > +const void *unit3(const int *arg) {
>     > +  return _(arg + 1);
>     > +}
>     > +// CHECK: define dso_local i8* @unit3(i32* %arg)
>     > +// CHECK-NOT: llvm.preserve.array.access.index
>     > +// CHECK-NOT: llvm.preserve.struct.access.index
>     > +// CHECK-NOT: llvm.preserve.union.access.index
>     > +
>     > +const void *unit4(const int *arg) {
>     > +  return _(&arg[1]);
>     > +}
>     > +// CHECK: define dso_local i8* @unit4(i32* %arg)
>     > +// CHECK-NOT: getelementptr
>     > +// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0i32(i32* %0, i32 0, i32 1)
>     > +
>     > +const void *unit5(const int *arg[5]) {
>     > +  return _(&arg[1][2]);
>     > +}
>     > +// CHECK: define dso_local i8* @unit5(i32** %arg)
>     > +// CHECK-NOT: getelementptr
>     > +// CHECK: call i32** @llvm.preserve.array.access.index.p0p0i32.p0p0i32(i32** %0, i32 0, i32 1)
>     > +// CHECK-NOT: getelementptr
>     > +// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0i32(i32* %2, i32 0, i32 2)
>     > +
>     > +struct s1 {
>     > +  char a;
>     > +  int b;
>     > +};
>     > +
>     > +struct s2 {
>     > +  char a1:1;
>     > +  char a2:1;
>     > +  int b;
>     > +};
>     > +
>     > +struct s3 {
>     > +  char a1:1;
>     > +  char a2:1;
>     > +  char :6;
>     > +  int b;
>     > +};
>     > +
>     > +const void *unit6(struct s1 *arg) {
>     > +  return _(&arg->a);
>     > +}
>     > +// CHECK: define dso_local i8* @unit6(%struct.s1* %arg)
>     > +// CHECK-NOT: getelementptr
>     > +// CHECK: call i8* @llvm.preserve.struct.access.index.p0i8.p0s_struct.s1s(%struct.s1* %0, i32 0, i32 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[STRUCT_S1:[0-9]+]]
>     > +
>     > +const void *unit7(struct s1 *arg) {
>     > +  return _(&arg->b);
>     > +}
>     > +// CHECK: define dso_local i8* @unit7(%struct.s1* %arg)
>     > +// CHECK-NOT: getelementptr
>     > +// CHECK: call i32* @llvm.preserve.struct.access.index.p0i32.p0s_struct.s1s(%struct.s1* %0, i32 1, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[STRUCT_S1]]
>     > +
>     > +const void *unit8(struct s2 *arg) {
>     > +  return _(&arg->b);
>     > +}
>     > +// CHECK: define dso_local i8* @unit8(%struct.s2* %arg)
>     > +// CHECK-NOT: getelementptr
>     > +// CHECK: call i32* @llvm.preserve.struct.access.index.p0i32.p0s_struct.s2s(%struct.s2* %0, i32 1, i32 2), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[STRUCT_S2:[0-9]+]]
>     > +
>     > +const void *unit9(struct s3 *arg) {
>     > +  return _(&arg->b);
>     > +}
>     > +// CHECK: define dso_local i8* @unit9(%struct.s3* %arg)
>     > +// CHECK-NOT: getelementptr
>     > +// CHECK: call i32* @llvm.preserve.struct.access.index.p0i32.p0s_struct.s3s(%struct.s3* %0, i32 1, i32 2), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[STRUCT_S3:[0-9]+]]
>     > +
>     > +union u1 {
>     > +  char a;
>     > +  int b;
>     > +};
>     > +
>     > +union u2 {
>     > +  char a;
>     > +  int :32;
>     > +  int b;
>     > +};
>     > +
>     > +const void *unit10(union u1 *arg) {
>     > +  return _(&arg->a);
>     > +}
>     > +// CHECK: define dso_local i8* @unit10(%union.u1* %arg)
>     > +// CHECK-NOT: getelementptr
>     > +// CHECK: call %union.u1* @llvm.preserve.union.access.index.p0s_union.u1s.p0s_union.u1s(%union.u1* %0, i32 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[UNION_U1:[0-9]+]]
>     > +
>     > +const void *unit11(union u1 *arg) {
>     > +  return _(&arg->b);
>     > +}
>     > +// CHECK: define dso_local i8* @unit11(%union.u1* %arg)
>     > +// CHECK-NOT: getelementptr
>     > +// CHECK: call %union.u1* @llvm.preserve.union.access.index.p0s_union.u1s.p0s_union.u1s(%union.u1* %0, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[UNION_U1]]
>     > +
>     > +const void *unit12(union u2 *arg) {
>     > +  return _(&arg->b);
>     > +}
>     > +// CHECK: define dso_local i8* @unit12(%union.u2* %arg)
>     > +// CHECK-NOT: getelementptr
>     > +// CHECK: call %union.u2* @llvm.preserve.union.access.index.p0s_union.u2s.p0s_union.u2s(%union.u2* %0, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[UNION_U2:[0-9]+]]
>     > +
>     > +struct s4 {
>     > +  char d;
>     > +  union u {
>     > +    int b[4];
>     > +    char a;
>     > +  } c;
>     > +};
>     > +
>     > +union u3 {
>     > +  struct s {
>     > +    int b[4];
>     > +  } c;
>     > +  char a;
>     > +};
>     > +
>     > +const void *unit13(struct s4 *arg) {
>     > +  return _(&arg->c.b[2]);
>     > +}
>     > +// CHECK: define dso_local i8* @unit13(%struct.s4* %arg)
>     > +// CHECK: call %union.u* @llvm.preserve.struct.access.index.p0s_union.us.p0s_struct.s4s(%struct.s4* %0, i32 1, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[STRUCT_S4:[0-9]+]]
>     > +// CHECK: call %union.u* @llvm.preserve.union.access.index.p0s_union.us.p0s_union.us(%union.u* %1, i32 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[UNION_I_U:[0-9]+]]
>     > +// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0a4i32([4 x i32]* %b, i32 1, i32 2)
>     > +
>     > +const void *unit14(union u3 *arg) {
>     > +  return _(&arg->c.b[2]);
>     > +}
>     > +// CHECK: define dso_local i8* @unit14(%union.u3* %arg)
>     > +// CHECK: call %union.u3* @llvm.preserve.union.access.index.p0s_union.u3s.p0s_union.u3s(%union.u3* %0, i32 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[UNION_U3:[0-9]+]]
>     > +// CHECK: call [4 x i32]* @llvm.preserve.struct.access.index.p0a4i32.p0s_struct.ss(%struct.s* %c, i32 0, i32 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[STRUCT_I_S:[0-9]+]]
>     > +// CHECK: call i32* @llvm.preserve.array.access.index.p0i32.p0a4i32([4 x i32]* %2, i32 1, i32 2)
>     > +
>     > +const void *unit15(struct s4 *arg) {
>     > +  return _(&arg[2].c.a);
>     > +}
>     > +// CHECK: define dso_local i8* @unit15(%struct.s4* %arg)
>     > +// CHECK: call %struct.s4* @llvm.preserve.array.access.index.p0s_struct.s4s.p0s_struct.s4s(%struct.s4* %0, i32 0, i32 2)
>     > +// CHECK: call %union.u* @llvm.preserve.struct.access.index.p0s_union.us.p0s_struct.s4s(%struct.s4* %1, i32 1, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[STRUCT_S4]]
>     > +// CHECK: call %union.u* @llvm.preserve.union.access.index.p0s_union.us.p0s_union.us(%union.u* %2, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[UNION_I_U]]
>     > +
>     > +const void *unit16(union u3 *arg) {
>     > +  return _(&arg[2].a);
>     > +}
>     > +// CHECK: define dso_local i8* @unit16(%union.u3* %arg)
>     > +// CHECK: call %union.u3* @llvm.preserve.array.access.index.p0s_union.u3s.p0s_union.u3s(%union.u3* %0, i32 0, i32 2)
>     > +// CHECK: call %union.u3* @llvm.preserve.union.access.index.p0s_union.u3s.p0s_union.u3s(%union.u3* %1, i32 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[UNION_U3]]
>     > +
>     > +// CHECK: ![[STRUCT_S1]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "s1",
>     > +// CHECK: ![[STRUCT_S2]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "s2",
>     > +// CHECK: ![[STRUCT_S3]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "s3",
>     > +// CHECK: ![[UNION_U1]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "u1",
>     > +// CHECK: ![[UNION_U2]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "u2",
>     > +// CHECK: ![[STRUCT_S4]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "s4",
>     > +// CHECK: ![[UNION_I_U]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "u",
>     > +// CHECK: ![[UNION_U3]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "u3",
>     > +// CHECK: ![[STRUCT_I_S]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "s",
>     >
>     > Added: cfe/trunk/test/Sema/builtin-preserve-access-index.c
>     > URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_Sema_builtin-2Dpreserve-2Daccess-2Dindex.c-3Frev-3D366076-26view-3Dauto&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=dtjPkUWyM2j7LOCdes9xK0tSCe4BrvYtJpmJu_el7xE&s=07_SwhXPP90qkyzer0pZ10w4OJJr_rjS7nj0WpnAcdY&e=
>     > ==============================================================================
>     > --- cfe/trunk/test/Sema/builtin-preserve-access-index.c (added)
>     > +++ cfe/trunk/test/Sema/builtin-preserve-access-index.c Mon Jul 15 08:42:41 2019
>     > @@ -0,0 +1,13 @@
>     > +// RUN: %clang_cc1 -x c -triple x86_64-pc-linux-gnu -dwarf-version=4 -fsyntax-only -verify %s
>     > +
>     > +const void *invalid1(const int *arg) {
>     > +  return __builtin_preserve_access_index(&arg[1], 1); // expected-error {{too many arguments to function call, expected 1, have 2}}
>     > +}
>     > +
>     > +void *invalid2(const int *arg) {
>     > +  return __builtin_preserve_access_index(&arg[1]); // expected-warning {{returning 'const void *' from a function with result type 'void *' discards qualifiers}}
>     > +}
>     > +
>     > +const void *invalid3(const int *arg) {
>     > +  return __builtin_preserve_access_index(1); // expected-warning {{incompatible integer to pointer conversion passing 'int' to parameter of type 'const void *'}}
>     > +}
>     >
>     >
>     > _______________________________________________
>     > cfe-commits mailing list
>     > cfe-commits at lists.llvm.org
>     > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=dtjPkUWyM2j7LOCdes9xK0tSCe4BrvYtJpmJu_el7xE&s=T1DJodYLPvglRosJ1a2hRWzLmu_TMofjMYBPulgTS2I&e=
>
>


More information about the cfe-commits mailing list