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
