This is an archive of the discontinued LLVM Phabricator instance.

[Clang][CodeGen] Add constant array support for __builtin_dump_sturct, and beautify dump format
AbandonedPublic

Authored by yihanaa on Mar 29 2022, 8:44 AM.

Details

Summary
  1. Add constant array support for __builtin_dump_sturct, the style like lldb

struct:

struct S {
  int x;
  int y : 4;
  int : 0;
  int b[2][2];
  float f;
  struct T {
    int i;
  } t;
  struct {
    int i;
  } foo;
  struct Bar {
      int x;
      const char *B;
    } bar[2];
};

output:

struct S {
    int x = 100
    int y : 4 = 1
    int : 0
    int[2][2] b = [
        [0] = [
            [0] = 1
            [1] = 2
        ]
        [1] = [
            [0] = 3
            [1] = 4
        ]
    ]
    float f = 0.000000
    struct T {
        int i = 2022
    }
    struct S::(unnamed) {
        int i = 4096
    }
    struct Bar[2] bar = [
        [0] = {
            int x = 1024
            const char * B = This is struct Bar[0]
        }
        [1] = {
            int x = 2048
            const char * B = This is struct Bar[1]
        }
    ]
}
  1. beautify dump format, add indent.

for example:
struct:

struct A {
  int a;
  struct B {
    int b;
    struct C {
      struct D {
        int d;
        union E {
          int x;
          int y;
        } e;
      } d;
      int c;
    } c;
  } b;
};

Before:

struct A {
int a = 0
struct B {
    int b = 0
struct C {
struct D {
            int d = 0
union E {
                int x = 0
                int y = 0
                }
            }
        int c = 0
        }
    }
}

After:

struct A {
    int a = 0
    struct B {
        int b = 0
        struct C {
            struct D {
                int d = 0
                union E {
                    int x = 0
                    int y = 0
                }
            }
            int c = 0
        }
    }
}
  1. Remove anonymous tag locations, powered by 'PrintingPolicy'

struct:

struct S {
  int a;
  struct /* Anonymous*/ {
    int x;
  } b;
  int c;
};

Before:

struct S {
int a = 0
struct S::(unnamed at ./builtin_dump_struct.c:20:3) {
    int x = 0
    }
int c = 0
}

After:

struct S {
    int a = 0
    struct S::(unnamed) {
        int x = 0
    }
    int c = 0
}

Diff Detail

Event Timeline

yihanaa created this revision.Mar 29 2022, 8:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2022, 8:44 AM
yihanaa requested review of this revision.Mar 29 2022, 8:44 AM

This looks to be quite the large patch, can you split this up into individual patches? It isn't clear to me what all is going on everywhere.

clang/docs/ReleaseNotes.rst
93

What is this from/for? Is this left over from a previous patch?

113

"now supports dumping constant arrays" perhaps?

114

This should probably be a different entry than the bitfield one.

155

Probably want a separate patch to fix these.

yihanaa updated this revision to Diff 418895.Mar 29 2022, 8:56 AM

Remove unnecessary code format

Thanks for review @erichkeane ,maybe i should split this patch.

clang/docs/ReleaseNotes.rst
93

The fixed by me in https://reviews.llvm.org/D122248, this modification appeared in multiple places in ReleaseNote, so I removed the redundant.

113

Yes,i add constant array support int this patch.

114

should i separate the two?

155

Okay

yihanaa updated this revision to Diff 418901.Mar 29 2022, 9:13 AM

split another patch to fix ReleaseNotes.

This looks to be quite the large patch, can you split this up into individual patches? It isn't clear to me what all is going on everywhere.

Thanks @erichkeane ,i have splited this patch.

I'd also suggest splitting into the '3' things that you're trying to accomplish above. The CGBuiltin.cpp code has way too much going on to reasonably review.

I'd also suggest splitting into the '3' things that you're trying to accomplish above. The CGBuiltin.cpp code has way too much going on to reasonably review.

These three things are mainly done in a new function dumpArray, and the dumpRecord is modified.

I'd also suggest splitting into the '3' things that you're trying to accomplish above. The CGBuiltin.cpp code has way too much going on to reasonably review.

I'd also suggest splitting into the '3' things that you're trying to accomplish above. The CGBuiltin.cpp code has way too much going on to reasonably review.

Maybe I should split the third thing into another separate patch, the first thing needs to modify the indent when adding constant array support, that is the third thing (modify the indent), what do you think?

I'd also suggest splitting into the '3' things that you're trying to accomplish above. The CGBuiltin.cpp code has way too much going on to reasonably review.

I'd also suggest splitting into the '3' things that you're trying to accomplish above. The CGBuiltin.cpp code has way too much going on to reasonably review.

Maybe I should split the third thing into another separate patch, the first thing needs to modify the indent when adding constant array support, that is the third thing (modify the indent), what do you think?

I would suggest piling them from #3, to #2 to #1. That is: "Change the anonymous printing" in the first, add the 'sub-element-tabbing' in 2nd, and 'add constant array support' in 3rd.

I'd also suggest splitting into the '3' things that you're trying to accomplish above. The CGBuiltin.cpp code has way too much going on to reasonably review.

I'd also suggest splitting into the '3' things that you're trying to accomplish above. The CGBuiltin.cpp code has way too much going on to reasonably review.

Maybe I should split the third thing into another separate patch, the first thing needs to modify the indent when adding constant array support, that is the third thing (modify the indent), what do you think?

I would suggest piling them from #3, to #2 to #1. That is: "Change the anonymous printing" in the first, add the 'sub-element-tabbing' in 2nd, and 'add constant array support' in 3rd.

Thanks for the suggestion @erichkeane , I am trying to do this

yihanaa abandoned this revision.Mar 31 2022, 7:48 AM