This is an archive of the discontinued LLVM Phabricator instance.

[AST] Store the callee and argument expressions of CallExpr in a trailing array.
ClosedPublic

Authored by riccibruno on Dec 17 2018, 8:24 AM.

Details

Summary

Since CallExpr::setNumArgs has been removed, it is now possible to store the callee expression and the argument expressions of CallExpr in a trailing array. This saves one pointer per CallExpr, CXXOperatorCallExpr,
CXXMemberCallExpr, CUDAKernelCallExpr and UserDefinedLiteral.

Since CallExpr is used as a base of the above classes we cannot use llvm::TrailingObjects. Instead we store
the offset in bytes from the this pointer to the start of the trailing objects and manually do the casts + arithmetic.

Some notes:

  1. I did not try to fit the number of arguments in the bit-fields of Stmt. This leaves some space for future additions and avoid the discussion about whether x bits are sufficient to hold the number of arguments.
  2. It would be perfectly possible to recompute the offset to the trailing objects before accessing the trailing objects. However the trailing objects are frequently accessed and benchmarks show that it is slightly faster to just load the offset from the bit-fields. Additionally, because of 1), we have plenty of space in the bit-fields of Stmt.

Diff Detail

Repository
rL LLVM

Event Timeline

riccibruno created this revision.Dec 17 2018, 8:24 AM

I agree with not packing the argument count in.

include/clang/AST/Stmt.h
439 ↗(On Diff #178458)

If we're not packing anything into these bits anyway, it would be really nice if this could just be loaded as a byte, i.e. if it were 8 bits wide and allocated at a bit offset that's a multiple of 8. That should be reasonably maintainable with a modest amount of arithmetic and static_assert-ing.

riccibruno marked an inline comment as done.

Make OffsetToTrailingObjects byte sized and aligned to a byte multiple.

rjmccall accepted this revision.Dec 18 2018, 8:18 AM

Thanks, looks great.

This revision is now accepted and ready to land.Dec 18 2018, 8:18 AM
This revision was automatically updated to reflect the committed changes.