[clang] Fixes compile error that double colon operator cannot resolve macro with parentheses.

[lygstate]

Summary

Error message:

In file included from ../src/amd/addrlib/src/core/addrobject.h:21:
../src/amd/addrlib/src/core/addrcommon.h:343:13: error: expected unqualified-id
    out = ::_tzcnt_u32(mask);
            ^
/usr/lib/llvm-15/lib/clang/15.0.6/include/bmiintrin.h:74:27: note: expanded from macro '_tzcnt_u32'
#define _tzcnt_u32(a)     (__tzcnt_u32((a)))

This is because both GCC/Clang doesn't support compiling the following code:

#ifdef _MSC_VER
#include <intrin.h>
#else
#include <x86intrin.h>
#endif

int f(int a) {
    return ::(_tzcnt_u32)(a);
}

This is because the return statement expects an expression or braced init list: http://eel.is/c++draft/stmt.jump#general-1 but we really only need to care about the expression form (there's no braces in sight).
Grammatically, the leading :: will parse as a qualified-id because it matches the production for nested-name-specifier: http://eel.is/c++draft/expr.prim.id.qual#nt:qualified-id
That needs to be followed by an unqualified-id (http://eel.is/c++draft/expr.prim.id.unqual#nt:unqualified-id), but the open paren does not match any of the grammar productions, so this is a syntax error.

Closes: https://github.com/llvm/llvm-project/issues/64467

Signed-off-by: Yonggang Luo <[e-mail]>

[pengfei]

The description is not clear to me. You should describe the reason rather than phenomenon.

My understanding is double colon operator cannot resolve functions with parentheses. https://godbolt.org/z/6P47se9WW

But I didn't find enough proof in Google. It'd be more persuasive if you can find it and add to the description.

[RKSimon]

Would we be better off creating proper function definitions - we already have similar duplicates for _bextr_u64 (Intel) vs __bextr_u64 (AMD) names that do this

[lygstate]

In D157297#4571572, @pengfei wrote:

The description is not clear to me. You should describe the reason rather than phenomenon.

My understanding is double colon operator cannot resolve functions with parentheses. https://godbolt.org/z/6P47se9WW

error: expected unqualified-id

return ::_tzcnt_u32(a);
         ^

/opt/compiler-explorer/clang-16.0.0/lib/clang/16/include/bmiintrin.h:74:27: note: expanded from macro '_tzcnt_u32'
#define _tzcnt_u32(a) (__tzcnt_u32((a)))

Looks like double colon operator cannot resolve macros with parentheses, is that a compiler bug or we should handled it in code?

But I didn't find enough proof in Google. It'd be more persuasive if you can find it and add to the description.

[pengfei]

In D157297#4583802, @lygstate wrote:

In D157297#4571572, @pengfei wrote:

The description is not clear to me. You should describe the reason rather than phenomenon.

My understanding is double colon operator cannot resolve functions with parentheses. https://godbolt.org/z/6P47se9WW

error: expected unqualified-id

return ::_tzcnt_u32(a);
         ^

/opt/compiler-explorer/clang-16.0.0/lib/clang/16/include/bmiintrin.h:74:27: note: expanded from macro '_tzcnt_u32'
#define _tzcnt_u32(a) (__tzcnt_u32((a)))

Looks like double colon operator cannot resolve macros with parentheses, is that a compiler bug or we should handled it in code?

But I didn't find enough proof in Google. It'd be more persuasive if you can find it and add to the description.

GCC doesn't resolve it either. https://godbolt.org/z/Prb4s5d6o. So it should not be a bug.

[lygstate]

creating proper function definitions instead of macro define

[pengfei]

I'd prefer macro to duplicated definitions. We have such precedents, e.g., https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/xmmintrin.h#L2994
Besides, you should update the summary as well.

[lygstate]

In D157297#4590580, @pengfei wrote:

I'd prefer macro to duplicated definitions. We have such precedents, e.g., https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/xmmintrin.h#L2994

Do you means revert to the first version?

Besides, you should update the summary as well.

I don't know what kind of summary you want, You can give me that so I can update it

[pengfei]

In D157297#4590692, @lygstate wrote:

In D157297#4590580, @pengfei wrote:

I'd prefer macro to duplicated definitions. We have such precedents, e.g., https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/xmmintrin.h#L2994

Do you means revert to the first version?

Yes.

Besides, you should update the summary as well.

I don't know what kind of summary you want, I can give me that so I can update it

A reliable source about double colon operator cannot resolve function with parentheses in C++ is the best I expect, however I haven't find one so far.
Maybe a short description about the phenomenon on GCC/Clang is better than just giving error message. I don't have strong request for this.

[lygstate]

Revert to version 1

[pengfei]

LGTM.

[pengfei]

BTW, maybe @aaron.ballman knows why we don't support such syntax in C++.

[aaron.ballman]

In D157297#4590886, @pengfei wrote:

BTW, maybe @aaron.ballman knows why we don't support such syntax in C++.

The return statement expects an expression or braced init list: http://eel.is/c++draft/stmt.jump#general-1 but we really only need to care about the expression form (there's no braces in sight).
Grammatically, the leading :: will parse as a qualified-id because it matches the production for nested-name-specifier: http://eel.is/c++draft/expr.prim.id.qual#nt:qualified-id
That needs to be followed by an unqualified-id (http://eel.is/c++draft/expr.prim.id.unqual#nt:unqualified-id), but the open paren does not match any of the grammar productions, so this is a syntax error.

[pengfei]

In D157297#4592851, @aaron.ballman wrote:

In D157297#4590886, @pengfei wrote:

BTW, maybe @aaron.ballman knows why we don't support such syntax in C++.

The return statement expects an expression or braced init list: http://eel.is/c++draft/stmt.jump#general-1 but we really only need to care about the expression form (there's no braces in sight).
Grammatically, the leading :: will parse as a qualified-id because it matches the production for nested-name-specifier: http://eel.is/c++draft/expr.prim.id.qual#nt:qualified-id
That needs to be followed by an unqualified-id (http://eel.is/c++draft/expr.prim.id.unqual#nt:unqualified-id), but the open paren does not match any of the grammar productions, so this is a syntax error.

Thanks @aaron.ballman for the detailed explanation! I have learnt it. @lygstate maybe include Aaron's explanation in commit message?

[lygstate]

Thanks @aaron.ballman for the detailed explanation! I have learnt it. @lygstate maybe include Aaron's explanation in commit message?

The commit message is updated, is that ready for merge?

clang/lib/Headers/bmiintrin.h

    to use it as a potentially faster version of BSF. */
 #define __RELAXED_FN_ATTRS __attribute__((__always_inline__, __nodebug__))
 
-#define _tzcnt_u16(a)     (__tzcnt_u16((a)))
-
+/* AMD-specified, double-leading-underscore version of TZCNT */
 /// Counts the number of trailing zero bits in the operand.
 ///
 /// \headerfile <x86intrin.h>

 ///    An unsigned 16-bit integer whose trailing zeros are to be counted.
 /// \returns An unsigned 16-bit integer containing the number of trailing zero
 ///    bits in the operand.
+/// \see _tzcnt_u16
 static __inline__ unsigned short __RELAXED_FN_ATTRS
 __tzcnt_u16(unsigned short __X)
 {
   return __builtin_ia32_tzcnt_u16(__X);
 }
 
+/* Intel-specified, single-leading-underscore version of TZCNT */
+/// Counts the number of trailing zero bits in the operand.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the <c> TZCNT </c> instruction.
+///
+/// \param __X
+///    An unsigned 16-bit integer whose trailing zeros are to be counted.
+/// \returns An unsigned 16-bit integer containing the number of trailing zero
+///    bits in the operand.
+/// \see __tzcnt_u16
+static __inline__ unsigned short __RELAXED_FN_ATTRS
+_tzcnt_u16(unsigned short __X)
+{
+  return __builtin_ia32_tzcnt_u16(__X);
+}
+
+/* AMD-specified, double-leading-underscore version of TZCNT */
 /// Counts the number of trailing zero bits in the operand.
 ///
 /// \headerfile <x86intrin.h>

 ///    An unsigned 32-bit integer whose trailing zeros are to be counted.
 /// \returns An unsigned 32-bit integer containing the number of trailing zero
 ///    bits in the operand.
-/// \see _mm_tzcnt_32
+/// \see _tzcnt_u32
 static __inline__ unsigned int __RELAXED_FN_ATTRS
 __tzcnt_u32(unsigned int __X)
 {
   return __builtin_ia32_tzcnt_u32(__X);
 }
 
+/* Intel-specified, single-leading-underscore version of TZCNT */
+/// Counts the number of trailing zero bits in the operand.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the <c> TZCNT </c> instruction.
+///
+/// \param __X
+///    An unsigned 32-bit integer whose trailing zeros are to be counted.
+/// \returns An unsigned 32-bit integer containing the number of trailing zero
+///    bits in the operand.
+/// \see __tzcnt_u32
+static __inline__ unsigned int __RELAXED_FN_ATTRS
+_tzcnt_u32(unsigned int __X)
+{
+  return __builtin_ia32_tzcnt_u32(__X);
+}
+
 /// Counts the number of trailing zero bits in the operand.
 ///
 /// \headerfile <x86intrin.h>

   return (int)__builtin_ia32_tzcnt_u32(__X);
 }
 
-#define _tzcnt_u32(a)     (__tzcnt_u32((a)))
-
 #ifdef __x86_64__
 
+/* AMD-specified, double-leading-underscore version of TZCNT */
 /// Counts the number of trailing zero bits in the operand.
 ///
 /// \headerfile <x86intrin.h>

 ///    An unsigned 64-bit integer whose trailing zeros are to be counted.
 /// \returns An unsigned 64-bit integer containing the number of trailing zero
 ///    bits in the operand.
-/// \see _mm_tzcnt_64
+/// \see _tzcnt_u64
 static __inline__ unsigned long long __RELAXED_FN_ATTRS
 __tzcnt_u64(unsigned long long __X)
 {
   return __builtin_ia32_tzcnt_u64(__X);
 }
 
+/* Intel-specified, single-leading-underscore version of TZCNT */
+/// Counts the number of trailing zero bits in the operand.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the <c> TZCNT </c> instruction.
+///
+/// \param __X
+///    An unsigned 64-bit integer whose trailing zeros are to be counted.
+/// \returns An unsigned 64-bit integer containing the number of trailing zero
+///    bits in the operand.
+/// \see __tzcnt_u64
+static __inline__ unsigned long long __RELAXED_FN_ATTRS
+_tzcnt_u64(unsigned long long __X)
+{
+  return __builtin_ia32_tzcnt_u64(__X);
+}
+
 /// Counts the number of trailing zero bits in the operand.
 ///
 /// \headerfile <x86intrin.h>

   return (long long)__builtin_ia32_tzcnt_u64(__X);
 }
 
-#define _tzcnt_u64(a)     (__tzcnt_u64((a)))
-
 #endif /* __x86_64__ */
 
 #undef __RELAXED_FN_ATTRS

 /* Define the default attributes for the functions in this file. */
 #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("bmi")))
 
-#define _andn_u32(a, b)   (__andn_u32((a), (b)))
-
-/* _bextr_u32 != __bextr_u32 */
-#define _blsi_u32(a)      (__blsi_u32((a)))
-
-#define _blsmsk_u32(a)    (__blsmsk_u32((a)))
-
-#define _blsr_u32(a)      (__blsr_u32((a)))
-
+/* AMD-specified, double-leading-underscore version of ANDN */
 /// Performs a bitwise AND of the second operand with the one's
 ///    complement of the first operand.
 ///

 ///    An unsigned integer containing one of the operands.
 /// \returns An unsigned integer containing the bitwise AND of the second
 ///    operand with the one's complement of the first operand.
+/// \see _andn_u32
 static __inline__ unsigned int __DEFAULT_FN_ATTRS
 __andn_u32(unsigned int __X, unsigned int __Y)
 {
   return ~__X & __Y;
 }
 
+/* Intel-specified, single-leading-underscore version of ANDN */
+/// Performs a bitwise AND of the second operand with the one's
+///    complement of the first operand.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the <c> ANDN </c> instruction.
+///
+/// \param __X
+///    An unsigned integer containing one of the operands.
+/// \param __Y
+///    An unsigned integer containing one of the operands.
+/// \returns An unsigned integer containing the bitwise AND of the second
+///    operand with the one's complement of the first operand.
+/// \see __andn_u32
+static __inline__ unsigned int __DEFAULT_FN_ATTRS
+_andn_u32(unsigned int __X, unsigned int __Y)
+{
+  return ~__X & __Y;
+}
+
 /* AMD-specified, double-leading-underscore version of BEXTR */
 /// Extracts the specified bits from the first operand and returns them
 ///    in the least significant bits of the result.

   return __builtin_ia32_bextr_u32(__X, __Y);
 }
 
+/* _bextr_u32 != __bextr_u32 */
 /* Intel-specified, single-leading-underscore version of BEXTR */
 /// Extracts the specified bits from the first operand and returns them
 ///    in the least significant bits of the result.

   return __builtin_ia32_bextr_u32(__X, __Y);
 }
 
+/* AMD-specified, double-leading-underscore version of BLSI */
 /// Clears all bits in the source except for the least significant bit
 ///    containing a value of 1 and returns the result.
 ///

 ///    An unsigned integer whose bits are to be cleared.
 /// \returns An unsigned integer containing the result of clearing the bits from
 ///    the source operand.
+/// \see _blsi_u32
 static __inline__ unsigned int __DEFAULT_FN_ATTRS
 __blsi_u32(unsigned int __X)
 {
   return __X & -__X;
 }
 
+/* Intel-specified, single-leading-underscore version of BLSI */
+/// Clears all bits in the source except for the least significant bit
+///    containing a value of 1 and returns the result.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the <c> BLSI </c> instruction.
+///
+/// \param __X
+///    An unsigned integer whose bits are to be cleared.
+/// \returns An unsigned integer containing the result of clearing the bits from
+///    the source operand.
+/// \see __blsi_u32
+static __inline__ unsigned int __DEFAULT_FN_ATTRS
+_blsi_u32(unsigned int __X)
+{
+  return __X & -__X;
+}
+
+/* AMD-specified, double-leading-underscore version of BLSMSK */
 /// Creates a mask whose bits are set to 1, using bit 0 up to and
 ///    including the least significant bit that is set to 1 in the source
 ///    operand and returns the result.

 /// \param __X
 ///    An unsigned integer used to create the mask.
 /// \returns An unsigned integer containing the newly created mask.
+/// \see _blsmsk_u32
 static __inline__ unsigned int __DEFAULT_FN_ATTRS
 __blsmsk_u32(unsigned int __X)
 {
   return __X ^ (__X - 1);
 }
 
+/* Intel-specified, single-leading-underscore version of BLSMSK */
+/// Creates a mask whose bits are set to 1, using bit 0 up to and
+///    including the least significant bit that is set to 1 in the source
+///    operand and returns the result.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the <c> BLSMSK </c> instruction.
+///
+/// \param __X
+///    An unsigned integer used to create the mask.
+/// \returns An unsigned integer containing the newly created mask.
+/// \see __blsmsk_u32
+static __inline__ unsigned int __DEFAULT_FN_ATTRS
+_blsmsk_u32(unsigned int __X)
+{
+  return __X ^ (__X - 1);
+}
+
+/* AMD-specified, double-leading-underscore version of BLSR */
 /// Clears the least significant bit that is set to 1 in the source
 ///    operand and returns the result.
 ///

 ///    An unsigned integer containing the operand to be cleared.
 /// \returns An unsigned integer containing the result of clearing the source
 ///    operand.
+/// \see _blsr_u32
 static __inline__ unsigned int __DEFAULT_FN_ATTRS
 __blsr_u32(unsigned int __X)
 {
   return __X & (__X - 1);
 }
 
-#ifdef __x86_64__
-
-#define _andn_u64(a, b)   (__andn_u64((a), (b)))
-
-/* _bextr_u64 != __bextr_u64 */
-#define _blsi_u64(a)      (__blsi_u64((a)))
-
-#define _blsmsk_u64(a)    (__blsmsk_u64((a)))
+/* Intel-specified, single-leading-underscore version of BLSR */
+/// Clears the least significant bit that is set to 1 in the source
+///    operand and returns the result.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the <c> BLSR </c> instruction.
+///
+/// \param __X
+///    An unsigned integer containing the operand to be cleared.
+/// \returns An unsigned integer containing the result of clearing the source
+///    operand.
+/// \see __blsr_u32
+static __inline__ unsigned int __DEFAULT_FN_ATTRS
+_blsr_u32(unsigned int __X)
+{
+  return __X & (__X - 1);
+}
 
-#define _blsr_u64(a)      (__blsr_u64((a)))
+#ifdef __x86_64__
 
+/* AMD-specified, double-leading-underscore version of ANDN */
 /// Performs a bitwise AND of the second operand with the one's
 ///    complement of the first operand.
 ///

 ///    An unsigned 64-bit integer containing one of the operands.
 /// \returns An unsigned 64-bit integer containing the bitwise AND of the second
 ///    operand with the one's complement of the first operand.
+/// \see _andn_u64
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS
 __andn_u64 (unsigned long long __X, unsigned long long __Y)
 {
   return ~__X & __Y;
 }
 
+/* Intel-specified, single-leading-underscore version of ANDN */
+/// Performs a bitwise AND of the second operand with the one's
+///    complement of the first operand.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the <c> ANDN </c> instruction.
+///
+/// \param __X
+///    An unsigned 64-bit integer containing one of the operands.
+/// \param __Y
+///    An unsigned 64-bit integer containing one of the operands.
+/// \returns An unsigned 64-bit integer containing the bitwise AND of the second
+///    operand with the one's complement of the first operand.
+/// \see __andn_u64
+static __inline__ unsigned long long __DEFAULT_FN_ATTRS
+_andn_u64 (unsigned long long __X, unsigned long long __Y)
+{
+  return ~__X & __Y;
+}
+
 /* AMD-specified, double-leading-underscore version of BEXTR */
 /// Extracts the specified bits from the first operand and returns them
 ///    in the least significant bits of the result.

   return __builtin_ia32_bextr_u64(__X, __Y);
 }
 
+/* _bextr_u64 != __bextr_u64 */
 /* Intel-specified, single-leading-underscore version of BEXTR */
 /// Extracts the specified bits from the first operand and returns them
 ///     in the least significant bits of the result.

   return __builtin_ia32_bextr_u64(__X, __Y);
 }
 
+/* AMD-specified, double-leading-underscore version of BLSI */
 /// Clears all bits in the source except for the least significant bit
 ///    containing a value of 1 and returns the result.
 ///

 ///    An unsigned 64-bit integer whose bits are to be cleared.
 /// \returns An unsigned 64-bit integer containing the result of clearing the
 ///    bits from the source operand.
+/// \see _blsi_u64
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS
 __blsi_u64(unsigned long long __X)
 {
   return __X & -__X;
 }
 
+/* Intel-specified, single-leading-underscore version of BLSI */
+/// Clears all bits in the source except for the least significant bit
+///    containing a value of 1 and returns the result.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the <c> BLSI </c> instruction.
+///
+/// \param __X
+///    An unsigned 64-bit integer whose bits are to be cleared.
+/// \returns An unsigned 64-bit integer containing the result of clearing the
+///    bits from the source operand.
+/// \see __blsi_u64
+static __inline__ unsigned long long __DEFAULT_FN_ATTRS
+_blsi_u64(unsigned long long __X)
+{
+  return __X & -__X;
+}
+
+/* AMD-specified, double-leading-underscore version of BLSMSK */
 /// Creates a mask whose bits are set to 1, using bit 0 up to and
 ///    including the least significant bit that is set to 1 in the source
 ///    operand and returns the result.

 /// \param __X
 ///    An unsigned 64-bit integer used to create the mask.
 /// \returns An unsigned 64-bit integer containing the newly created mask.
+/// \see _blsmsk_u64
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS
 __blsmsk_u64(unsigned long long __X)
 {
   return __X ^ (__X - 1);
 }
 
+/* Intel-specified, single-leading-underscore version of BLSMSK */
+/// Creates a mask whose bits are set to 1, using bit 0 up to and
+///    including the least significant bit that is set to 1 in the source
+///    operand and returns the result.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the <c> BLSMSK </c> instruction.
+///
+/// \param __X
+///    An unsigned 64-bit integer used to create the mask.
+/// \returns An unsigned 64-bit integer containing the newly created mask.
+/// \see __blsmsk_u64
+static __inline__ unsigned long long __DEFAULT_FN_ATTRS
+_blsmsk_u64(unsigned long long __X)
+{
+  return __X ^ (__X - 1);
+}
+
+/* AMD-specified, double-leading-underscore version of BLSR */
 /// Clears the least significant bit that is set to 1 in the source
 ///    operand and returns the result.
 ///

 ///    An unsigned 64-bit integer containing the operand to be cleared.
 /// \returns An unsigned 64-bit integer containing the result of clearing the
 ///    source operand.
+/// \see _blsr_u64
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS
 __blsr_u64(unsigned long long __X)
 {
   return __X & (__X - 1);
 }
 
+/* Intel-specified, single-leading-underscore version of BLSR */
+/// Clears the least significant bit that is set to 1 in the source
+///    operand and returns the result.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the <c> BLSR </c> instruction.
+///
+/// \param __X
+///    An unsigned 64-bit integer containing the operand to be cleared.
+/// \returns An unsigned 64-bit integer containing the result of clearing the
+///    source operand.
+/// \see __blsr_u64
+static __inline__ unsigned long long __DEFAULT_FN_ATTRS
+_blsr_u64(unsigned long long __X)
+{
+  return __X & (__X - 1);
+}
+
 #endif /* __x86_64__ */
 
 #undef __DEFAULT_FN_ATTRS