Page MenuHomePhabricator

[CodeGen] Propagate may-alias'ness of lvalues with TBAA info
ClosedPublic

Authored by kosarev on Oct 17 2017, 9:27 AM.

Details

Summary

This patch fixes various places in clang to propagate may-alias TBAA access descriptors during construction of lvalues, thus eliminating the need for the LValueBaseInfo::MayAlias flag.

This is part of D38126 reworked to be a separate patch to simplify review.

Diff Detail

Repository
rL LLVM

Event Timeline

kosarev created this revision.Oct 17 2017, 9:27 AM
rjmccall accepted this revision.Oct 23 2017, 12:40 AM

Well, I've been agreeing so far that it makes sense to propagate TBAA information separately from LValueBaseInfo, and this seems like a logical part of that, so okay.

This revision is now accepted and ready to land.Oct 23 2017, 12:40 AM

Now that D38796 is comitted and added the tbaa-cast.cpp test case to the mainline, we fail on it with this patch applied. The reason is that we have no special TBAA type node for may-alias accesses, so everything that ever been a potential access to a character type turns into a may-alias access and propagates as such.

The test case reads:

struct V {
  unsigned n;
};

struct S {
  char bytes[4];
};

void foo(S *p) {
  ((V*)p->bytes)->n = 5;
}

Here, p->bytes decays to a pointer to char, meaning the pointee object is considered may-alias and so it is after the explicit cast.

This arises an interesting question: should we introduce a special representation for may-alias accesses to distinct them from character-typed accesses? Or, maybe explicit casts should not derive their may-alias'ness from their source operands? Or, maybe we want to consider all explicit casts of the form (T*)E where T* and typeof(E) point to different types as pointers to may-alias objects?

I suspect this is supposed to be in this diff:

I think probably the best solution is to track may-alias-ness explicitly in the TBAAAccessInfo instead of relying in the frontend on being able to distinguish things in the LLVM metadata.

Yes, I don't like it too when it comes to comparing pointers to metadata nodes in an attempt to figure out what kind of access we are dealing with.

What concerns me is that there may be other kinds of TBAA descriptors that are neither ordinary nor may-alias ones. For example, at some point we will need to distinct TBAA descriptors for accesses to direct and indirect union members, because they require special processing.

What if we replace the may-alias flag with an enumeration? Something like this:

enum class TBAAAccessKind {
  Ordinary,
  MayAlias,
};

Sure, that makes sense to me.

John.

kosarev updated this revision to Diff 120675.Oct 27 2017, 12:34 PM

Reworked to distinct may-alias accesses from ordinary ones with an explicit 'kind' field.

kosarev requested review of this revision.Oct 27 2017, 12:34 PM
kosarev edited edge metadata.

Looks good, but you missed updating equality/hashing for the new Kind field.

lib/CodeGen/CodeGenTBAA.h
71 ↗(On Diff #120675)

This needs to factor in the Kind.

205 ↗(On Diff #120675)

This needs to factor in the Kind.

kosarev updated this revision to Diff 120828.Oct 30 2017, 8:24 AM
  • Fixed comparing and hashing of TBAA access descriptors.
  • Rebased on top of D39177.

Thanks John!

This revision is now accepted and ready to land.Oct 30 2017, 10:52 AM
This revision was automatically updated to reflect the committed changes.