diff mbox

PATCH: Add OPTION_MASK_ISA_X86_64 and support TARGET_BI_ARCH == 2

Message ID CAMe9rOo3k1jXT4rEu3E4+JRhG4CndepVkCpUsW3+WBY8hNyM1A@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu April 8, 2012, 2:54 p.m. UTC
On Sun, Apr 8, 2012 at 4:38 AM, Iain Sandoe <idsandoe@googlemail.com> wrote:
> Hi H.J.
>
>
> On 31 Mar 2012, at 20:24, Jack Howarth wrote:
>>
>>
>>  The latest gcc-pr52784-2.patch patch also allows current gcc trunk to
>> bootstrap on i386-apple-darwin10.
>
>
> Despite the fact that bootstrap is restored, there remain problems with this
> patch and some more work is needed.
>
> (a) [trivial] the option 'mx32' is in i386.opt, which means it is exposed to
> all sub-targets, even if they don't support it.
>
> $ ./gcc/xgcc -Bgcc ../tests/hello.c -mx32 -o hc
> /var/folders/OW/OW-PGOtgHbKakssxFpJpkU++-0E/-Tmp-//ccIP9e4Z.s:10:bad
> register name `%rbp'
> /var/folders/OW/OW-PGOtgHbKakssxFpJpkU++-0E/-Tmp-//ccIP9e4Z.s:14:bad
> register name `%rsi'
> etc. etc.

It is useful for bootstrap and allows you check out what
X32 code looks like even if your OS doesn't support it.

> (b) [serious] the m64 ObjC multi-lib is broken on i?86-darwin* (and likely
> there are other more subtle effects).
>
> This is because the code in config/darwin.c that Joseph pointed out (earlier
> in this thread) is called for SUBSUBTARGET_OVERRIDE_OPTIONS.
>
> That code sets defaults for, and checks errors for, flags that apply to
> *-*-darwin* (and are needed for LTO as well as c-family).
>
> In the case of the ObjC ABI (fobjc-abi-version=) we need to default it to
> "2" @ m64 (and default it to 0 or 1 depending on the darwin version @ m32).
>
> I accept that some of this could possibly be done in driver-self-specs;
> however, we allow m32/m64 to be unspecified on the c/l and to default for
> the target.  I'm also not yet sure whether %:version-compare() would be
> applicable to fobjc-abi-version.
>
> Thus, the current trunk implementation is broken by your patch and we need
> to address that pending other solutions (I'm also very short of free time
> for Darwin right now - to experiment with the specs solution).

Please try this



> =--=
>
> It is possible that there is an options handling issue, (although I might
> also have misunderstood) viz:
>
> =========
> (current) i386.opt:
>
> ;; ISA support
>
>
> m32
> Target RejectNegative Negative(m64) Report InverseMask(ISA_64BIT)
> Var(ix86_isa_flags) Save
> Generate 32bit i386 code
>
> m64
> Target RejectNegative Negative(mx32) Report Mask(ABI_64) Var(ix86_isa_flags)
> Save
>
> Generate 64bit x86-64 code
>
> mx32
> Target RejectNegative Negative(m32) Report Mask(ABI_X32) Var(ix86_isa_flags)
> Save
> Generate 32bit x86-64 code
>
> =======
> from gccint.pdf (section 8.2):
>
> Negative(othername )
> The option will turn off another option othername, which is the option name
> with the leading “-” removed. This chain action will propagate through the
> Negative property of the option to be turned off.
> As a consequence, if you have a group of mutually-exclusive options, their
> Negative properties should form a circular chain. For example, if options
> ‘-a ’, ‘-b ’ and ‘-c ’ are mutually exclusive, their respective Negative
> properties
> should be ‘Negative(b )’, ‘Negative(c )’ and ‘Negative(a )’.
>
> ======
>
> I read this as "if the User specifies -a on the command line the inverse of
> -b *and* the inverse of -c will be applied".
>
> so that when -m64 is issued, the *inverse* of Mask(ABI_X32) should be
> applied and then the *inverse* of InverseMask(ISA_64BIT) - which would
> (correctly, for the case we're considering) set MASK_ISA_64BIT.
>
> However, in the example above this does NOT happen - if I set a breakpoint
> at the entry of x86_internal_override_options - ISA_64BIT ends up as 0 when
> -m64 is specified on the c/l for i?86-darwin*.
>
> The x86_isa_explicit stuff doesn't appear to get set either, although
> global_options_set.x_x86_isa... does, which I've used in the attached patch.
>
> so is this an options bug or misunderstanding on my part?
>

There is no problem here.  ISA_64BIT is turned on by ABI_64:

  else if (TARGET_LP64)
    {
      /* Always turn on OPTION_MASK_ISA_64BIT and turn off
         OPTION_MASK_ABI_X32 for TARGET_LP64.  */
      ix86_isa_flags |= OPTION_MASK_ISA_64BIT;
      ix86_isa_flags &= ~OPTION_MASK_ABI_X32;
    }

Comments

Iain Sandoe April 8, 2012, 6:09 p.m. UTC | #1
On 8 Apr 2012, at 15:54, H.J. Lu wrote:
>> Despite the fact that bootstrap is restored, there remain problems  
>> with this
>> patch and some more work is needed.
>>
>> (a) [trivial] the option 'mx32' is in i386.opt, which means it is  
>> exposed to
>> all sub-targets, even if they don't support it.
>>
>> $ ./gcc/xgcc -Bgcc ../tests/hello.c -mx32 -o hc
>> /var/folders/OW/OW-PGOtgHbKakssxFpJpkU++-0E/-Tmp-//ccIP9e4Z.s:10:bad
>> register name `%rbp'
>> /var/folders/OW/OW-PGOtgHbKakssxFpJpkU++-0E/-Tmp-//ccIP9e4Z.s:14:bad
>> register name `%rsi'
>> etc. etc.
>
> It is useful for bootstrap and allows you check out what
> X32 code looks like even if your OS doesn't support it.

well, OK, but I'd have thought that to be something for developers to  
experiment with, rather than an 'end user' flag.
However, it is, as stated 'trivial' I'm not going to lose sleep over  
it ;-).

>> (b) [serious] the m64 ObjC multi-lib is broken on i?86-darwin* (and  
>> likely
>> there are other more subtle effects).
>>
>> This is because the code in config/darwin.c that Joseph pointed out  
>> (earlier
>> in this thread) is called for SUBSUBTARGET_OVERRIDE_OPTIONS.
>>
>> That code sets defaults for, and checks errors for, flags that  
>> apply to
>> *-*-darwin* (and are needed for LTO as well as c-family).
>>
>> In the case of the ObjC ABI (fobjc-abi-version=) we need to default  
>> it to
>> "2" @ m64 (and default it to 0 or 1 depending on the darwin version  
>> @ m32).
>>
>> I accept that some of this could possibly be done in driver-self- 
>> specs;
>> however, we allow m32/m64 to be unspecified on the c/l and to  
>> default for
>> the target.  I'm also not yet sure whether %:version-compare()  
>> would be
>> applicable to fobjc-abi-version.
>>
>> Thus, the current trunk implementation is broken by your patch and  
>> we need
>> to address that pending other solutions (I'm also very short of  
>> free time
>> for Darwin right now - to experiment with the specs solution).
>
> Please try this

<snip>

This is not enough to solve the problem, because there are decisions  
made earlier (e.g. in c-family/c-opts.c re. exceptions) that depend on  
flag states.

It would be possible to move/repeat some of those (which I did in the  
proposed fix attached to my last post).

=--=
>>
>> It is possible that there is an options handling issue, (although I  
>> might
>> also have misunderstood) viz:
>>
>> =========
>> (current) i386.opt:
>>
>> ;; ISA support
>>
>>
>> m32
>> Target RejectNegative Negative(m64) Report InverseMask(ISA_64BIT)
>> Var(ix86_isa_flags) Save
>> Generate 32bit i386 code
>>
>> m64
>> Target RejectNegative Negative(mx32) Report Mask(ABI_64)  
>> Var(ix86_isa_flags)
>> Save
>>
>> Generate 64bit x86-64 code
>>
>> mx32
>> Target RejectNegative Negative(m32) Report Mask(ABI_X32)  
>> Var(ix86_isa_flags)
>> Save
>> Generate 32bit x86-64 code
>>
>> =======
>> from gccint.pdf (section 8.2):
>>
>> Negative(othername )
>> The option will turn off another option othername, which is the  
>> option name
>> with the leading “-” removed. This chain action will propagate  
>> through the
>> Negative property of the option to be turned off.
>> As a consequence, if you have a group of mutually-exclusive  
>> options, their
>> Negative properties should form a circular chain. For example, if  
>> options
>> ‘-a ’, ‘-b ’ and ‘-c ’ are mutually exclusive, their  
>> respective Negative
>> properties
>> should be ‘Negative(b )’, ‘Negative(c )’ and  
>> ‘Negative(a )’.
>>
>> ======
>>
>> I read this as "if the User specifies -a on the command line the  
>> inverse of
>> -b *and* the inverse of -c will be applied".
>>
>> so that when -m64 is issued, the *inverse* of Mask(ABI_X32) should be
>> applied and then the *inverse* of InverseMask(ISA_64BIT) - which  
>> would
>> (correctly, for the case we're considering) set MASK_ISA_64BIT.
>>
>> However, in the example above this does NOT happen - if I set a  
>> breakpoint
>> at the entry of x86_internal_override_options - ISA_64BIT ends up  
>> as 0 when
>> -m64 is specified on the c/l for i?86-darwin*.
>>
>> The x86_isa_explicit stuff doesn't appear to get set either, although
>> global_options_set.x_x86_isa... does, which I've used in the  
>> attached patch.
>>
>> so is this an options bug or misunderstanding on my part?
>>
>
> There is no problem here.  ISA_64BIT is turned on by ABI_64:

Well, I accept that your code in i386.c enforces the assumption above  
- my point is that the documentation implies (at least to me) that the  
enforcement should be done in options processing.

The i386.c enforcement happens *after* some points in the code that  
(currently) assume the behavior as I describe above (or as per the  
code before your patch).

I am sure that a hybrid of the two patches can be made to work - my  
concern is to be clear about what is *supposed* to happen at options  
parsing time.

thanks
Iain
diff mbox

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index c959113..69893ab 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -3109,14 +3109,6 @@  ix86_option_override_internal (bool main_args_p)
       sw = "attribute";
     }

-#ifdef SUBTARGET_OVERRIDE_OPTIONS
-  SUBTARGET_OVERRIDE_OPTIONS;
-#endif
-
-#ifdef SUBSUBTARGET_OVERRIDE_OPTIONS
-  SUBSUBTARGET_OVERRIDE_OPTIONS;
-#endif
-
   /* Turn off both OPTION_MASK_ABI_64 and OPTION_MASK_ABI_X32 if
      TARGET_64BIT_DEFAULT is true and TARGET_64BIT is false.  */
   if (TARGET_64BIT_DEFAULT && !TARGET_64BIT)
@@ -3157,6 +3149,14 @@  ix86_option_override_internal (bool main_args_p)
       ix86_isa_flags &= ~OPTION_MASK_ABI_X32;
     }

+#ifdef SUBTARGET_OVERRIDE_OPTIONS
+  SUBTARGET_OVERRIDE_OPTIONS;
+#endif
+
+#ifdef SUBSUBTARGET_OVERRIDE_OPTIONS
+  SUBSUBTARGET_OVERRIDE_OPTIONS;
+#endif
+
   /* -fPIC is the default for x86_64.  */
   if (TARGET_MACHO && TARGET_64BIT)
     flag_pic = 2;