Adjust empty class parameter passing ABI (PR c++/60336)

[Jason Merrill]

On 11/09/2017 10:26 AM, Richard Biener wrote:
>> Moving TYPE_EMPTY_P to finalize_type_size revealed a bug in Cilk+, it was
>> bogusly (I believe) setting TREE_ADDRESSABLE, so the assert fired.  Since
>> Cilk+ is being deprecated and the Cilk+ testsuite still passes, I'm not
>> too worried about this change.

Ah, presumably C++ classes aren't hitting the assert just because they 
set TREE_ADDRESSABLE after finalize_type_size, so the assert is wrong. 
Let's drop the assert and the Cilk+ change.

>> So if you're fine with the DECL_PADDING_P change, all that remains is to
>> check whether this patch is not changing the ABI for other languages than
>> C++.  I suppose one way to check that could be to do sth like
>>
>>    if (strncmp (lang_hooks.name, "GNU C++", 7))
>>      {FILE *f=fopen("/tmp/A", "a");fprintf(f,"%s\n",main_input_filename);fclose(f);}
>>
>> and compare the assembly of the files it prints?  But there were thousands of
>> them I think :(.
> 
> Shouldn't most of GCCs own object files (and target library object files)
> compare 1:1 before and after the patch?  After all it should _only_ affect
> parameter passing of empty aggregates (and the files the patch changes
> of course)?

That makes sense to me.

>> @@ -9295,6 +9315,10 @@ ix86_return_in_memory (const_tree type, const_tree fntype ATTRIBUTE_UNUSED)
...
>> +  /* Empty records are never passed in memory.  */
>> +  if (type && TYPE_EMPTY_P (type))
>> +    return false;

Since the TYPE_EMPTY_P flag is in target-independent code, let's check 
it in aggregate_value_p rather than here.

>> +ix86_warn_parameter_passing_abi (cumulative_args_t cum_v, tree type)
...
>> +  tree ctx = cum->decl ? DECL_CONTEXT (cum->decl) : NULL_TREE;
>> +  if (ctx != NULL_TREE
>> +      && TREE_CODE (ctx) == TRANSLATION_UNIT_DECL
>> +      && !TRANSLATION_UNIT_WARN_EMPTY_P (ctx))
>> +    return;

This doesn't handle empty classes within a namespace/class/function. 
Maybe move get_ultimate_context out of dwarf2out.c and use that?

>> +/* Used in a FIELD_DECL to indicate that this field is padding for
>> +   the purpose of determining whether the record this field is a member
>> +   of is considered empty.  */

I wouldn't mention "empty" here; this indicates that the field is 
padding, not data.  A target might use this information to avoid passing 
this field even if the class contains data as well.

Jason