[AST] Pack CallExpr

[riccibruno]

Summary

Use the newly available space in the bit-fields of Stmt to store some
data from CallExpr. This saves 8 bytes per CallExpr. This is a straightforward
patch, except that it limits the maximum number of arguments to 2^14 - 1.

The maximum number of arguments to a function call is already
limited to 16 bits because of FunctionTypeBitfields::NumParams,
which used to be 15 bits until a few month ago.

This also do not leave any space for additional bits, but after looking at
the history of CallExpr it seems that people are not adding data
here very frequently.

It would be possible to reuse some bits of the SubExprs pointer, but ideally
this pointer would be removed by storing the arguments in a trailing array.
Alternatively it would be possible to store the number of arguments in a
trailing object when it is too large.

[rjmccall]

I don't think we should be reducing the number of call arguments we can support, sorry, even if 16K is a fairly absurd number that would probably trip stack overflow protections if you actually executed it. Let's try to keep it at least 64K-ish.

[riccibruno]

In D54676#1302510, @rjmccall wrote:

I don't think we should be reducing the number of call arguments we can support, sorry, even if 16K is a fairly absurd number that would probably trip stack overflow protections if you actually executed it. Let's try to keep it at least 64K-ish.

Would 15 bits be alright ? NumParams in FunctionTypeBitfields was 15 bits wide since early 2013.

[rjmccall]

You can have more arguments than parameters because of varargs. Even putting that aside, no, I think we generally shouldn't go backwards on these limits. Anyway, packing right up to the limits imposed by NumExprBits probably isn't a great idea.

[riccibruno]

In D54676#1303176, @rjmccall wrote:

You can have more arguments than parameters because of varargs. Even putting that aside, no, I think we generally shouldn't go backwards on these limits. Anyway, packing right up to the limits imposed by NumExprBits probably isn't a great idea.

Alright, I think it make sense to wait until it is possible to store the arguments in a trailing array.
There are a few places where Sema resize the array of arguments in CallExpr.
From a quick look it seems it should be possible to get rid of them. For now let's mark
this "changes planned".

[riccibruno]

Abandoned in favor of D55771

include/clang/AST/Expr.h

 class CallExpr : public Expr {
   enum { FN=0, PREARGS_START=1 };
   Stmt **SubExprs;
-  unsigned NumArgs;
-  SourceLocation RParenLoc;
 
   void updateDependenciesFromArg(Expr *Arg);
 

   }
 
   /// getNumArgs - Return the number of actual arguments to this call.
-  ///
-  unsigned getNumArgs() const { return NumArgs; }
+  unsigned getNumArgs() const { return CallExprBits.NumArgs; }
 
   /// Retrieve the call arguments.
   Expr **getArgs() {

 
   /// getArg - Return the specified argument.
   Expr *getArg(unsigned Arg) {
-    assert(Arg < NumArgs && "Arg access out of range!");
+    assert(Arg < getNumArgs() && "Arg access out of range!");
     return cast_or_null<Expr>(SubExprs[Arg + getNumPreArgs() + PREARGS_START]);
   }
   const Expr *getArg(unsigned Arg) const {
-    assert(Arg < NumArgs && "Arg access out of range!");
+    assert(Arg < getNumArgs() && "Arg access out of range!");
     return cast_or_null<Expr>(SubExprs[Arg + getNumPreArgs() + PREARGS_START]);
   }
 
   /// setArg - Set the specified argument.
   void setArg(unsigned Arg, Expr *ArgExpr) {
-    assert(Arg < NumArgs && "Arg access out of range!");
+    assert(Arg < getNumArgs() && "Arg access out of range!");
     SubExprs[Arg+getNumPreArgs()+PREARGS_START] = ArgExpr;
   }
 

 
   /// getNumCommas - Return the number of commas that must have been present in
   /// this function call.
-  unsigned getNumCommas() const { return NumArgs ? NumArgs - 1 : 0; }
+  unsigned getNumCommas() const { return getNumArgs() ? getNumArgs() - 1 : 0; }
 
   /// getBuiltinCallee - If this is a call to a builtin, return the builtin ID
   /// of the callee. If not, return 0.

   /// type.
   QualType getCallReturnType(const ASTContext &Ctx) const;
 
-  SourceLocation getRParenLoc() const { return RParenLoc; }
-  void setRParenLoc(SourceLocation L) { RParenLoc = L; }
+  SourceLocation getRParenLoc() const { return CallExprBits.RParenLoc; }
+  void setRParenLoc(SourceLocation L) { CallExprBits.RParenLoc = L; }
 
   SourceLocation getBeginLoc() const LLVM_READONLY;
   SourceLocation getEndLoc() const LLVM_READONLY;

 
   // Iterators
   child_range children() {
-    return child_range(&SubExprs[0],
-                       &SubExprs[0]+NumArgs+getNumPreArgs()+PREARGS_START);
+    return child_range(&SubExprs[0], &SubExprs[0] + getNumArgs() +
+                                         getNumPreArgs() + PREARGS_START);
   }
 
   const_child_range children() const {
-    return const_child_range(&SubExprs[0], &SubExprs[0] + NumArgs +
+    return const_child_range(&SubExprs[0], &SubExprs[0] + getNumArgs() +
                                                getNumPreArgs() + PREARGS_START);
   }
 };

include/clang/AST/Stmt.h

     unsigned : NumExprBits;
 
     unsigned NumPreArgs : 1;
+    unsigned NumArgs : 14;
+
+    SourceLocation RParenLoc;
   };
 
   class MemberExprBitfields {

lib/AST/Expr.cpp

                    ExprValueKind VK, SourceLocation rparenloc)
     : Expr(SC, t, VK, OK_Ordinary, fn->isTypeDependent(),
            fn->isValueDependent(), fn->isInstantiationDependent(),
-           fn->containsUnexpandedParameterPack()),
-      NumArgs(args.size()) {
+           fn->containsUnexpandedParameterPack()) {
+  unsigned NumArgs = args.size();
+  CallExprBits.NumArgs = NumArgs;
+  assert((getNumArgs() == NumArgs) && "NumArgs overflow!");
 
   unsigned NumPreArgs = preargs.size();
-  SubExprs = new (C) Stmt *[args.size()+PREARGS_START+NumPreArgs];
+  CallExprBits.NumPreArgs = NumPreArgs;
+
+  SubExprs = new (C) Stmt *[NumArgs+PREARGS_START+NumPreArgs];
   SubExprs[FN] = fn;
   for (unsigned i = 0; i != NumPreArgs; ++i) {
     updateDependenciesFromArg(preargs[i]);
     SubExprs[i+PREARGS_START] = preargs[i];
   }
-  for (unsigned i = 0; i != args.size(); ++i) {
+  for (unsigned i = 0; i != NumArgs; ++i) {
     updateDependenciesFromArg(args[i]);
     SubExprs[i+PREARGS_START+NumPreArgs] = args[i];
   }
 
-  CallExprBits.NumPreArgs = NumPreArgs;
-  RParenLoc = rparenloc;
+  setRParenLoc(rparenloc);
 }
 
 CallExpr::CallExpr(const ASTContext &C, StmtClass SC, Expr *fn,

 
 CallExpr::CallExpr(const ASTContext &C, StmtClass SC, unsigned NumPreArgs,
                    EmptyShell Empty)
-  : Expr(SC, Empty), SubExprs(nullptr), NumArgs(0) {
+  : Expr(SC, Empty), SubExprs(nullptr) {
+  CallExprBits.NumArgs = 0;
   // FIXME: Why do we allocate this?
   SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs]();
   CallExprBits.NumPreArgs = NumPreArgs;

 
   // If shrinking # arguments, just delete the extras and forgot them.
   if (NumArgs < getNumArgs()) {
-    this->NumArgs = NumArgs;
+    CallExprBits.NumArgs = NumArgs;
     return;
   }
 

 
   if (SubExprs) C.Deallocate(SubExprs);
   SubExprs = NewSubExprs;
-  this->NumArgs = NumArgs;
+  CallExprBits.NumArgs = NumArgs;
 }
 
 /// getBuiltinCallee - If this is a call to a builtin, return the builtin ID. If