Re: PATCH: Add OPTION_MASK_ISA_X86_64 and support TARGET_BI_ARCH == 2

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Wed, Mar 28, 2012 at 2:10 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Mar 28, 2012 at 10:13 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Mar 28, 2012 at 12:51 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> On Wed, Mar 28, 2012 at 9:33 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>
>>>> What do we do with TARGET_64BIT and TARGET_64BIT_DEFAULT? ?They
>>>> are used to indicate 64bit ISA like:
>>>>
>>>> collect2.c:/* TARGET_64BIT may be defined to use driver specific
>>>> functionality. */
>>>> collect2.c:#undef TARGET_64BIT
>>>> collect2.c:#define TARGET_64BIT TARGET_64BIT_DEFAULT
>>>> reg-stack.c: ?if ((flag_pic && !TARGET_64BIT)
>>>> tlink.c:/* TARGET_64BIT may be defined to use driver specific functionality. */
>>>> tlink.c:#undef TARGET_64BIT
>>>> tlink.c:#define TARGET_64BIT TARGET_64BIT_DEFAULT
>>>> xcoffout.c: ? ? ? if (lno > 0 && (TARGET_64BIT || lno <= (int)USHRT_MAX)) ? ? ? ? ?\
>>>> config/darwin.c: ? ? ? ? ? ? ? && TARGET_64BIT
>>>> config/darwin.c: ? ? ? ? ? && TARGET_64BIT
>>>> config/darwin.c: ? ? ? ? ? ? ? ?: (TARGET_64BIT ? 2
>>>> config/darwin.c: ? ? ?if (TARGET_64BIT && global_options.x_flag_objc_abi < 2)
>>>> config/darwin.c: ? ? ?if (!TARGET_64BIT && global_options.x_flag_objc_abi >= 2)
>>>> config/darwin.c: ? ? ?&& !TARGET_64BIT)
>>>> config/darwin.c: ?if (!TARGET_64BIT) ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
>>>> config/darwin.c: ?if (!TARGET_64BIT ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
>>>> config/darwin.h: ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?flag_next_runtime && !TARGET_64BIT; ? ? \
>>>> config/sol2-bi.h:#define WCHAR_TYPE (TARGET_64BIT ? "int" : "long int")
>>>> config/sol2-bi.h:#define WINT_TYPE (TARGET_64BIT ? "int" : "long int")
>>>>
>>>> Should we keep them? Right now I have
>>>>
>>>> #define TARGET_64BIT ? ?OPTION_ARCH_X86_64
>>>>
>>>> and
>>>>
>>>> #define TARGET_64BIT_DEFAULT (OPTION_MASK_ARCH_X86_64 | OPTION_MASK_ISA_64)
>>>>
>>>> Should we go with
>>>>
>>>> OPTION_MASK_ISA_64BIT -> stays the same
>>>> OPTION_MASK_ISA_X86_64 -> OPTION_MASK_ABI_64
>>>> OPTION_MASK_ISA_X32 -> ?OPTION_MASK_ABI_X32
>>>
>>> Yes, the above is IMO much better. We have to separate ABI and ISA
>>> defines in some meaningful way.
>>>
>>> Uros.
>>
>> Here is the new patch. ?OK for trunk if there are no regressions on
>> Linux/ia32 and Linux/x86-64?
>
> @@ -2657,7 +2657,7 @@ ix86_target_string (HOST_WIDE_INT isa, int
> flags, const char *arch,
> ? ? ?preceding options while match those first. ?*/
> ? static struct ix86_target_opts isa_opts[] =
> ? {
> - ? ?{ "-m64", ? ? ? ? ?OPTION_MASK_ISA_64BIT },
> + ? ?{ "-m64", ? ? ? ? ?OPTION_MASK_ABI_64 },
> ? ? { "-mfma4", ? ? ? ? ? ? ? ?OPTION_MASK_ISA_FMA4 },
>
> Please add -mx32 here. Probably also -m32.

I will add -mx32.  Adding -m32 is hard since clearing the
OPTION_MASK_ISA_64BIT bit isn't supported here.  Can I
try

{ "!-m32",          OPTION_MASK_ISA_64BIT },

> ?#define TARGET_64BIT ? OPTION_ISA_64BIT
> -#define TARGET_X32 ? ? OPTION_ISA_X32
> +#define TARGET_X32 ? ? OPTION_ABI_X32
> ?#define TARGET_MMX ? ? OPTION_ISA_MMX
> ?#define TARGET_3DNOW ? OPTION_ISA_3DNOW
> ?#define TARGET_3DNOW_A OPTION_ISA_3DNOW_A
> @@ -77,7 +77,7 @@ see the files COPYING3 and COPYING.RUNTIME
> respectively. ?If not, see
> ?#define TARGET_F16C ? ?OPTION_ISA_F16C
> ?#define TARGET_RTM ? ? ?OPTION_ISA_RTM
>
> -#define TARGET_LP64 ? ?(TARGET_64BIT && !TARGET_X32)
> +#define TARGET_LP64 ? ?OPTION_ABI_64
>
> Please group new OPTION_ABI_xxx defines together, after ISA defines.

Will do.

> The patch is OK, but please also get approval from Joseph (options maintainer).
>

Thanks.

-- 
H.J.