[m-rev.] for review: separate gcc and clang specific code in the runtime

[Julien Fischer]

On Thu, 11 Aug 2011, Paul Bone wrote:

> On Wed, Jul 27, 2011 at 05:51:57PM +1000, Julien Fischer wrote:
>>
>> For review by anyone.
>>
>> runtime/mercury_atomic_ops.c:
>> runtime/mercury_atomic_ops.h
>
> I'll review these files by modifing them myself.  What you've done is correct,
> but there are some cases where clang appears to be compatible enough to support
> the inline assembly and compiler intrinsics here.

Yes, however I think as a policy we ought to maintain a strict
separation between GCC and clang here, i.e. if both of them support a
particular extension then the condtion should be:

   #if defined(MR_GNUC) || defined(MR_CLANG)

In particular, we shouldn't be using __GNUC__ to also mean clang.

>> Index: runtime/mercury_conf_param.h
>> ===================================================================
>> RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_conf_param.h,v
>> retrieving revision 1.118
>> diff -u -r1.118 mercury_conf_param.h
>> --- runtime/mercury_conf_param.h	20 May 2011 04:16:54 -0000	1.118
>> +++ runtime/mercury_conf_param.h	27 Jul 2011 07:46:13 -0000
>> @@ -1054,4 +1054,29 @@
>>
>>  /*---------------------------------------------------------------------------*/
>>
>> +/*
>> +** C compilers.
>> +*/
>> +
>> +/*
>> +** MR_CLANG -- The C compiler is clang.
>> +**
>> +** MR_GNUC -- The C compiler is GCC.  We use this macro instead of __GNUC__
>> +**            since clang also defines __GNUC__.
>> +	      The value of this macro gives the major version number.
>> +**
>> +** MR_MSVC -- The C compiler is Visual C.
>> +**	      The value of this macro gives the version number.
>> +*/
>> +
>
> Is it just me or is the formatting in this comment a little strange?

There were some unexpanded tabs there; I've expanded them.

> The value of which macro? MR_GNUC or __GNUC__?

MR_GNUC, that's what the description is for.  I've swapped the order of
the last two sentences so this should now be more clear.

>> Index: runtime/mercury_std.h
>> ===================================================================
>> RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_std.h,v
>> retrieving revision 1.32
>> diff -u -r1.32 mercury_std.h
>> --- runtime/mercury_std.h	12 Jul 2011 03:21:58 -0000	1.32
>> +++ runtime/mercury_std.h	27 Jul 2011 05:22:10 -0000
>> @@ -161,8 +161,8 @@
>>    #define MR_INLINE			inline
>>    #define MR_EXTERN_INLINE		inline
>>    #define MR_OUTLINE_DEFN(DECL,BODY)
>> -#elif defined(__clang__)
>> -  /* clang: note that since clang also defines the macro __GNUC__
>> +#elif defined(MR_CLANG)
>> +  /* clang: note that since clang also defines the macro MR_GNUC
>>    ** we must handle it before we handle the GCC case.
>>    ** XXX why don't the C99 definitions work for clang?
>>    */
>
> Clang does not also define MR_GNUC.

Fixed.

Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------