This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Failures in branch range calculation

I sent the following response to Richard Earnshaw, but forgot to CC the gcc-patches list. Sorry.

-- Caroline Tice

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:

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

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

I'll give this a little time (but not too much) for discussion...


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]