This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Represent array members in new-format TBAA type descriptors
ClosedPublic

Authored by kosarev on Dec 19 2017, 8:55 AM.

Details

Summary

Now that in the new TBAA format we allow access types to be of any object types, including aggregate ones, it becomes critical to specify types of all sub-objects such aggregates comprise as their members. In order to meet this requirement, this patch enables generation of field descriptors for members of array types.

This patch requires D41394 to be landed first.

Diff Detail

Repository
rL LLVM

Event Timeline

kosarev created this revision.Dec 19 2017, 8:55 AM
hfinkel added inline comments.Dec 19 2017, 6:13 PM
test/CodeGen/tbaa-array.cpp
24 ↗(On Diff #127537)

Shouldn't this access have a size of 4, and an access for c->x[2] have a size of 4 and a specific offset and c->x[j] have a size of 12 and an offset of zero? Why does this list a size of 16?

In any case, please add tests for:

int *bar2(C *c) {
  return c->x;
}

int bar3(C *c) {
  return c->x[2];
}

int bar4(C *c, int j) {
  return c->x[j];
}
kosarev added inline comments.Dec 20 2017, 8:08 AM
test/CodeGen/tbaa-array.cpp
24 ↗(On Diff #127537)

Indeed, the access size is wrong as we mistakenly inherit it from the base type. D41452 fixes this. Thanks for catching.

kosarev added inline comments.Dec 20 2017, 8:47 AM
test/CodeGen/tbaa-array.cpp
24 ↗(On Diff #127537)

Hal, in bar2() we don't really access memory. What do we want to check with it?

hfinkel added inline comments.Dec 20 2017, 9:00 AM
test/CodeGen/tbaa-array.cpp
24 ↗(On Diff #127537)

Hal, in bar2() we don't really access memory. What do we want to check with it?

Ah, good point. Ignore that one.

kosarev updated this revision to Diff 127742.Dec 20 2017, 9:54 AM
  • Fixed the access size.
  • Added the suggested tests.
hfinkel accepted this revision.Dec 20 2017, 8:29 PM

LGTM. The array accesses here are just being represented as their scalar-access types. In the future, we should update this to represent them as fields in their parent structs, but this is fine for now.

This revision is now accepted and ready to land.Dec 20 2017, 8:29 PM
This revision was automatically updated to reflect the committed changes.