> On Sep 29, 2020, at 10:18 AM, Hohensee, Paul <hohen...@amazon.com> wrote:
> Code that calls initialize_shared_locs is inconsistent even within itself.
> E.g., in c1_Compilation.cpp, we have
Agreed there seems to be a bit of a mess around that function.
> Anyway, I just wanted to make the compiler warning go away, not figure out
> why the code is the way it is. Matthias and Kim would like to separate out
> the CSystemColors.m patch into a separate bug, which is fine by me (see
> separate email).
>
> So, for the sharedRuntime patch, would it be ok to just use
>
> short locs_buf[84];
>
> to account for possible alignment in initialize_shared_locs? I'm using 84
> because 80 is exactly 5 records plus 5 sizeof(relocInfo)s, allowing for
> alignment adds 3, and rounding the total array size up to a multiple of 8
> adds 1.
Your analysis looks plausible. But consider instead
struct { double data[20]; } locs_buf;
and change next line to use &locs_buf
This doesn't change the size or alignment from pre-existing code. I can't
test whether this will suppress the warning, but I'm guessing it will. And the
rest
of below is off if that’s wrong.
Changing the size and type and worrying about alignment changes seems beyond
what's needed "to make the compiler warning go away, not figure out why the
code is the way it is.” I dislike making weird changes to suppress compiler
warnings,
but changing the type and size seems more weird to me here. I mean, 84 looks
like
a number pulled out of a hat, unless you are going to add a bunch of commentary
that probably really belongs in a bug report to look at this stuff more
carefully.
And file an RFE to look at initialize_shared_locs and its callers more
carefully.
