[llvm-commits] [llvm] r162873 - in /llvm/trunk: include/llvm/Support/Compiler.h lib/Support/raw_ostream.cpp

[Jakub Staszak]

I meant, once we remove BUILTIN_EXPECT, it is not possible to use in an easy interface in LLVM. However as i said, nobody probably ever used that. 

- Kuba
On Sep 3, 2012, at 4:51 PM, Benjamin Kramer <benny.kra at gmail.com> wrote:

> 
> On 03.09.2012, at 16:43, Jakub Staszak <kubastaszak at gmail.com> wrote:
> 
>> BUILTIN_EXPECT also supported SwitchInst, which is impossible for LLVM_LIKELY. Nobody probably ever used that though. Also, you may want to upgrade docs/BranchWeightMetadata.html.
> 
> This isn't about the representation of __builtin_expect in llvm IR, it's just a portable macro to allow using __builtin_expect to optimize llvm itself. __builtin_expect is a rather low-level thing and for the specific uses in LLVM the LIKELY/UNLIKELY macros provide an easier interface.
> 
> - Ben
> 
>> - Kuba
>> 
>> On Aug 30, 2012, at 1:30 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:
>> 
>>> 
>>> On 30.08.2012, at 01:18, Chandler Carruth <chandlerc at google.com> wrote:
>>> 
>>>> On Wed, Aug 29, 2012 at 3:57 PM, Benjamin Kramer <benny.kra at googlemail.com> wrote:
>>>> Author: d0k
>>>> Date: Wed Aug 29 17:57:00 2012
>>>> New Revision: 162873
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=162873&view=rev
>>>> Log:
>>>> Replace the BUILTIN_EXPECT macro with a less horrible LLVM_LIKELY/LLVM_UNLIKELY interface.
>>>> 
>>>> Modified:
>>>>  llvm/trunk/include/llvm/Support/Compiler.h
>>>>  llvm/trunk/lib/Support/raw_ostream.cpp
>>>> 
>>>> Modified: llvm/trunk/include/llvm/Support/Compiler.h
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Compiler.h?rev=162873&r1=162872&r2=162873&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/include/llvm/Support/Compiler.h (original)
>>>> +++ llvm/trunk/include/llvm/Support/Compiler.h Wed Aug 29 17:57:00 2012
>>>> @@ -106,9 +106,11 @@
>>>> #endif
>>>> 
>>>> #if (__GNUC__ >= 4)
>>>> -#define BUILTIN_EXPECT(EXPR, VALUE) __builtin_expect((EXPR), (VALUE))
>>>> +#define LLVM_LIKELY(EXPR) __builtin_expect((EXPR), true)
>>>> +#define LLVM_UNLIKELY(EXPR) __builtin_expect((EXPR), false)
>>>> 
>>>> Do you want to cast (EXPR) to bool before handing it to __builtin_expect? The true variant will do the wrong thing for integers in some weird cases without that...
>>> 
>>> Yup, that's safer and better fits the if(LLVM_LIKELY(…)) idiom. Added bool casts in r162877.
>>> 
>>> - Ben
>>> 
>>>> 
>>>> #else
>>>> -#define BUILTIN_EXPECT(EXPR, VALUE) (EXPR)
>>>> +#define LLVM_LIKELY(EXPR) (EXPR)
>>>> +#define LLVM_UNLIKELY(EXPR) (EXPR)
>>>> #endif
>>>> 
>>>> 
>>>> 
>>>> Modified: llvm/trunk/lib/Support/raw_ostream.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/raw_ostream.cpp?rev=162873&r1=162872&r2=162873&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Support/raw_ostream.cpp (original)
>>>> +++ llvm/trunk/lib/Support/raw_ostream.cpp Wed Aug 29 17:57:00 2012
>>>> @@ -266,8 +266,8 @@
>>>> 
>>>> raw_ostream &raw_ostream::write(unsigned char C) {
>>>> // Group exceptional cases into a single branch.
>>>> -  if (BUILTIN_EXPECT(OutBufCur >= OutBufEnd, false)) {
>>>> -    if (BUILTIN_EXPECT(!OutBufStart, false)) {
>>>> +  if (LLVM_UNLIKELY(OutBufCur >= OutBufEnd)) {
>>>> +    if (LLVM_UNLIKELY(!OutBufStart)) {
>>>>     if (BufferMode == Unbuffered) {
>>>>       write_impl(reinterpret_cast<char*>(&C), 1);
>>>>       return *this;
>>>> @@ -286,8 +286,8 @@
>>>> 
>>>> raw_ostream &raw_ostream::write(const char *Ptr, size_t Size) {
>>>> // Group exceptional cases into a single branch.
>>>> -  if (BUILTIN_EXPECT(size_t(OutBufEnd - OutBufCur) < Size, false)) {
>>>> -    if (BUILTIN_EXPECT(!OutBufStart, false)) {
>>>> +  if (LLVM_UNLIKELY(size_t(OutBufEnd - OutBufCur) < Size)) {
>>>> +    if (LLVM_UNLIKELY(!OutBufStart)) {
>>>>     if (BufferMode == Unbuffered) {
>>>>       write_impl(Ptr, Size);
>>>>       return *this;
>>>> @@ -302,7 +302,7 @@
>>>>   // If the buffer is empty at this point we have a string that is larger
>>>>   // than the buffer. Directly write the chunk that is a multiple of the
>>>>   // preferred buffer size and put the remainder in the buffer.
>>>> -    if (BUILTIN_EXPECT(OutBufCur == OutBufStart, false)) {
>>>> +    if (LLVM_UNLIKELY(OutBufCur == OutBufStart)) {
>>>>     size_t BytesToWrite = Size - (Size % NumBytes);
>>>>     write_impl(Ptr, BytesToWrite);
>>>>     copy_to_buffer(Ptr + BytesToWrite, Size - BytesToWrite);
>>>> @@ -523,7 +523,7 @@
>>>>   ssize_t ret;
>>>> 
>>>>   // Check whether we should attempt to use atomic writes.
>>>> -    if (BUILTIN_EXPECT(!UseAtomicWrites, true)) {
>>>> +    if (LLVM_LIKELY(!UseAtomicWrites)) {
>>>>     ret = ::write(FD, Ptr, Size);
>>>>   } else {
>>>>     // Use ::writev() where available.
>>>> 
>>>> 
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>> 
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>