This is the mail archive of the gcc-bugs@gcc.gnu.org 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]

Re: i386 code label alignment patch (version 2)


> You seem to be looking at version numbers for what is essentially a
> feature test.  I think you should run the feature test instead.  The
> only case in which you need to check the version number is for a
> Canadian Cross.  That should be a fallback, not the default.
> 
> In fact, I see that you actually run the assembler to get the version
> number.  Instead, run the assembler to see if it supports what you
> need.

I can change this to use feature tests if desired.  It may been harder
to reliably determine if a feature is available using feature tests
instead of gas version numbers in some cases.  Feature tests certainly
make sense if supporting features that are expected to be implemented
in the same fashion by more than one assembler vendor.  At the moment
I'm only concerned with taking advantage of newer gas specific features.
It seems cleaner in this scenario to merely check GAS_MAJOR_VERSION and
GAS_MINOR_VERSION.

> You also seem to be confusing the issue of how to set the alignment
> correctly with the issue of aligning code labels.  Those are separate
> issues.  gcc should always be able to use ASM_OUTPUT_ALIGN.  The issue
> of defining ASM_OUTPUT_ALIGN correctly is independent of the issue of
> when to use it.  I think this patch will be easier to understand if
> you break it into two independent patches.

I don't mean to be confusing them.  You're right that changing
ASM_OUTPUT_ALIGN is not directly related to aligning code labels, except
for the fact that them both rely on knowing what forms of .p2align
are supported by the assembler.  I can certainly remove the
ASM_OUTPUT_ALIGN change from the patch if desired.

>  + elif [[ -f ../gas/Makefile.in ]]; then
>  + 	gcc_cv_gas_major_version=`sed -n 's/^VERSION=\([[0-9]]*\)\..*/\1/p' < ../gas/Makefile.in`
>  + 	gcc_cv_gas_minor_version=`sed -n 's/^VERSION=[[0-9]]*\.\([[0-9]]*\).*/\1/p' < ../gas/Makefile.in`
> 
> That does not work with the current gas sources.  As of binutils 2.9,
> the version number is actually found in gas/configure.in, not
> gas/Makefile.in.

Actually I changed this to ../gas/Makefile after sending out version
2 of the patch since I believe that ../gas/Makefile.in only works if
srcdir == objdir.  It looks like binutils 2.9.1 still has VERSION in
gas/Makefile even though it's not set in gas/Makefile.in.  Does this
sound correct / how would you suggest doing this?

Keep in mind that:

  1) In the cross compiler cases the only thing I may have to work with
     is the assembler VERSION number.

  2) It's not the end of the world if the assembler version / features
     can't be determined.  It only means that certain features will not
     be used ... the result will be the current status quo.

Let me know what the consensus is on these issues and I'll be happy
to look at making the necessary changes.

-- John
-------------------------------------------------------------------------
|   Feith Systems  |   Voice: 1-215-646-8000  |  Email: john@feith.com  |
|    John Wehle    |     Fax: 1-215-540-5495  |                         |
-------------------------------------------------------------------------



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