Failures in branch range calculation
Caroline Tice
ctice@apple.com
Tue May 25 06:03:00 GMT 2004
I sent the following response to Richard Earnshaw, but forgot to CC the
gcc-patches list. Sorry.
-- Caroline Tice
ctice@apple
I have a patch I am currently working on for updating/fixing a lot of
small problems with
the hot/cold partitioning stuff. Currently, in my patch, the code for
the function
text_section looks like this:
void
text_section (void)
{
if (in_section != in_text)
{
in_section = in_text;
fprintf (asm_out_file, "%s\n", TEXT_SECTION_ASM_OP);
assemble_align (FUNCTION_BOUNDARY);
}
}
Will this be okay? Will it fix your problem? If not, what do you
think would be better? I do
not know when I will be able to submit my full patch, because I am
running into some
very obscure, difficult-to-track-down bugs when I try to do a profiled
bootstrap on an x86/Linux
machine, doing partitioning. But I would be happy to submit (or have
you submit) a patch
containing the above fix, if you think that would be appropriate.
Please let me know what
you think.
-- Caroline Tice
ctice@apple.com
On May 22, 2004, at 5:08 AM, Richard Earnshaw wrote:
> I've been trying to track down the reason why the compiler has started
> occasionally generating out-of-range branches on the thumb target. It
> turns out to be as a result of the hot-and-cold code regions changes.
>
> One of the changes in that patch was to make text_section() add an
> unconditional alignment. This started off as:
>
>>>> + #define SECTION_FORMAT_STRING".section\t\"%s\"\n\t.align 2\n"
>>>
>>> What is this for?
>>
>> To make sure the correct alignments, "dots", etc. end up being
>> output for whichever architecture
>> we are on, when writing out the assembly directive for
>> changing between hot/cold sections. Yes, this
>> *is* necessary. Without it I get bugs on both architectures
>> I've tried this on.
>
> and was subsequently changed to be an explicit alignment statement in
> text_section() by Andrew.
>
> This causes a problem on Thumb because we output switch tables in the
> read-only data section and then switch back to the text section and get
> an unaccounted alignment statement in the output stream. Thumb
> instructions are only 2 bytes long, so a 4-byte alignment at this point
> is bad news.
>
> The change to add an unconditional alignment is just wrong.
> Conceptually, and in practice.
>
> - Just because we've switched to a new section doesn't automatically
> imply that things in it need aligning.
>
> - It's really wrong that the alignment is a manifest constant...
>
> - And it's really really wrong that branch range calculation doesn't
> take this into account.
>
> Almost invariably if we've just switched to the text section (and we
> weren't there previously) the the next thing that will happen is that a
> label will be emitted. The alignment belongs on that label not on the
> section change.
>
> Whatever, the alignment doesn't belong here, so I really think the only
> correct patch here is to back this bit out.
>
> However, I don't know what the particular problem was that was
> supposedly solved by this change, so it's impossible for me to evaluate
> exactly what it was intended to fix and what the most appropriate fix
> is
> elsewhere.
>
> I'll give this a little time (but not too much) for discussion...
>
> R.
>
More information about the Gcc-patches
mailing list