Closed fxcoudert closed 2 years ago
The next step on this is to do side-by-side with clang to make sure that we are producing the correct ABI.
Empty objects on Darwin (notwithstanding C++17+ rules) should occupy 1 byte of storage (for reasons to do with the ld64 atom model) and there's no reason for that to have excess alignment (that I can see) if there's no __attribute__((__align__()))
.
So, unless the rules have changed for aarch64, I'd say the test case expects the Right Thing and we're generating wrong code.
but.. let's check what clang does (section, alignment and size) - noting that you should add -fno-common to the clang flags (since that's what GCC defaults to now).
That test explicitly passes -fcommon
. clang agrees with the expectation from the testcase:
.zerofill __DATA,__bss,_sea,1,0 ; @sea
ah my bad, it's testing that indeed.
OK that's one for the TODO :)
Actually I think for that one, -fcommon
and -fno-common
do the same thing.
hmm .. I cannot quite see where the variable is picking up the excess alignment.
.local sea
.comm sea,0,8
(note that the size is OK to be zero on Linux, it has to be bumped to 1 on darwin because of the ld64 model).
... unfortunately tree dumps are not much use for tracing variables.
prob next is to ask the aarch64 folks whether this is an ABI mandate (I do not recall reading it in the AAPCS64).
patch for testing.
fixed - TODO is maybe to decide if there is ever a case that the extra alignment really brings some performance gain - but IMO that would only be something to do for >= O1 (and not for Os or Oz)
darwin-sections.c
The only difference with the expected assembly is the generation of this:
which gives:
where the testcase expects a different alignment:
I'm a bit puzzled as to whether this is expected or not, actually. The
_Alignof
operator applied tosea
returns 1, which would match what the testcase expects… but maybe being in a common makes it different?