Re: [PATCH, PR50527] Don't assume alignment of vla-related allocas.

[Tom de Vries <Tom_deVries at mentor dot com>]

On 10/04/2011 08:51 PM, Richard Henderson wrote:
> On 10/04/2011 09:28 AM, Tom de Vries wrote:
>> Well, that was the idea. But now I wonder, isn't it better to do this in
>> expand_builtin_alloca:
>> ...
>>    /* Compute the argument.  */
>>    op0 = expand_normal (CALL_EXPR_ARG (exp, 0));
>>
>> +  align =
>> +    (DECL_FUNCTION_CODE (get_callee_fndecl (exp)) ==
>> +     BUILT_IN_ALLOCA_WITH_ALIGN)
>> +    ? TREE_INT_CST_LOW (CALL_EXPR_ARG (exp, 1))
>> +    : BIGGEST_ALIGNMENT;
>> +
>> +
>>    /* Allocate the desired space.  */
>> -  result = allocate_dynamic_stack_space (op0, 0, BIGGEST_ALIGNMENT,
>> -					 cannot_accumulate);
>> +  result = allocate_dynamic_stack_space (op0, 0, align, cannot_accumulate);
> 
> Yes, this is better.
> 
> I've lost track of what you're trying to do with "folding" alloca?
> 

In general, to fold vlas (which are lowered to allocas) to normal declarations,
if the alloca argument is constant.

This particular patch tries 2 addres 2 issues:
- A bug in the current implementation (PR50527). We assume a different alignment
  while propagating in ccp, than during folding the alloca to a decl.
- We try to use the DECL_ALIGN of the vla as the DECL_ALIGN of the folded alloca
  decl. We try to carry the DECL_ALIGN from lowering to folding and later using
  the newly introduced __builtin_alloca_with_align. There are other ways to do
  this carrying:
  - introduce a vla_decl field in the call gimple
  - use a composition of __builtin_alloca and __builtin_assume_aligned
  I chose for __builtin_alloca_with_align to because it's simpler to use than
  the latter. The former is the simplest, but I think
  __builtin_alloca_with_align might have a use beyond vlas.

Any guidance on what to do if we have to expand the __builtin_alloca_with_align
to a function call, specifically, with the second argument? This is currently
handled like this, which is not very nice:
...
@@ -5559,11 +5573,21 @@ expand_builtin (tree exp, rtx target, rt
 	return XEXP (DECL_RTL (DECL_RESULT (current_function_decl)), 0);

     case BUILT_IN_ALLOCA:
+    case BUILT_IN_ALLOCA_WITH_ALIGN:
       /* If the allocation stems from the declaration of a variable-sized
 	 object, it cannot accumulate.  */
       target = expand_builtin_alloca (exp, CALL_ALLOCA_FOR_VAR_P (exp));
       if (target)
 	return target;
+      if (DECL_FUNCTION_CODE (get_callee_fndecl (exp))
+	  == BUILT_IN_ALLOCA_WITH_ALIGN)
+	{
+	  tree new_call = build_call_expr_loc (EXPR_LOCATION (exp),
+					       built_in_decls[BUILT_IN_ALLOCA],
+					       1, CALL_EXPR_ARG (exp, 0));
+	  CALL_ALLOCA_FOR_VAR_P (new_call) = CALL_ALLOCA_FOR_VAR_P (exp);
+	  exp = new_call;
+	}
       break;

     case BUILT_IN_STACK_SAVE:
...

Thanks,
- Tom

> 
> r~