llvm dev - Mar 2009 - [LLVMdev] [Bug 3756] __attribute__((always_inline)) and __builtin_constant_p

[ please CC: me as I'm not subscribed ]

On Wed, Mar 11, 2009 at 04:13:34AM +0000, bugzilla-daemon at cs.uiuc.edu
wrote:
> http://llvm.org/bugs/show_bug.cgi?id=3756
> 
> Chris Lattner <clattner at apple.com> changed:
>
>            What    |Removed                     |Added
>
----------------------------------------------------------------------------
>              Status|NEW                         |RESOLVED
>          Resolution|                            |WONTFIX
>
> --- Comment #6 from Chris Lattner <clattner at apple.com>  2009-03-10
23:13:33 ---
> Unfortunately, this will never be fixed in either llvm-gcc or clang. 
> __builtin_constant_p is a "best effort" constant folding tester,
which is
> allowed to fail.  You should never write code that assumes that
> __builtin_constant_p can't fail (if you need that, just don't use
> __builtin_constant_p).
> It would be interesting and useful to bring this up on the glibc list
> to see if they are willing to fix their headers.
Hahahaha, you never talked to Uli before did you ?
> Barring that, if this is really important to you, please raise the issue on
the
> llvmdev list.  We will need IR extensions to support this among other
things.
Well, I don't expect __builtin_constant_p to works always. That's not
precisely the issue. I expect though __attribute__((always_inline))
functions to be expanded prior to any optimization pass, because it's
what it means. Those functions are basically type-safe macros.

If you do that, then my problem go away at once. For what it's worth, my
always_inline functions are always functions that allow constant-folding
of its result when parameters are known at compile time, or fallbacks to
a real, non inlined, implementation else. Typical (silly but useful for
my point) use is for example, a function that is:

extern int __utf8_charlen(wchar_t c);
__attribute__((always_inline))
static inline int utf8_charlen(wchar_t c) {
    if (__builtin_constant_p(c)) {
        if (c < 0 || c >= 0x200000)
	    return 0;
        return 1 + (c >= 0x80) + (c >= 0x800) + (c >= 0x10000);
    }
    return __utf8_charlen(c);
}

I really expect that here, if I have a straight utf8_charlen('\0') call,
llvm-gcc would first expand utf8_charlen inline, and then, optimizes the
__builtin_constant_p away. I mean, the cases where llvm-gcc is unable to
do the optimizations I somehow expect it to do, the arguments are always
direct constants passed to it, not through any kind of local variable,
hence a best-effort algorithm should just work.

-- 
·O·  Pierre Habouzit
··O                                                madcoder at debian.org
OOO                                                http://www.madism.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20090315/e128078c/attachment.sig>

Pierre Habouzit wrote:
> [ please CC: me as I'm not subscribed ]
> 
> On Wed, Mar 11, 2009 at 04:13:34AM +0000, bugzilla-daemon at cs.uiuc.edu
wrote:
>> http://llvm.org/bugs/show_bug.cgi?id=3756
>>
>> Chris Lattner <clattner at apple.com> changed:
>>
>>            What    |Removed                     |Added
>>
----------------------------------------------------------------------------
>>              Status|NEW                         |RESOLVED
>>          Resolution|                            |WONTFIX
>>
>> --- Comment #6 from Chris Lattner <clattner at apple.com> 
2009-03-10 23:13:33 ---
>> Unfortunately, this will never be fixed in either llvm-gcc or clang. 
>> __builtin_constant_p is a "best effort" constant folding
tester, which is
>> allowed to fail.  You should never write code that assumes that
>> __builtin_constant_p can't fail (if you need that, just don't
use
>> __builtin_constant_p).
> 
> 
>> It would be interesting and useful to bring this up on the glibc list
>> to see if they are willing to fix their headers.
> 
> Hahahaha, you never talked to Uli before did you ?
> 
>> Barring that, if this is really important to you, please raise the
issue on the
>> llvmdev list.  We will need IR extensions to support this among other
things.
> 
> Well, I don't expect __builtin_constant_p to works always. That's
not
> precisely the issue. I expect though __attribute__((always_inline))
> functions to be expanded prior to any optimization pass, because it's
> what it means. Those functions are basically type-safe macros.
> 
> If you do that, then my problem go away at once.
No, that's not the problem.

Before any optimizations can be applied the function must be converted 
into LLVM IR. In LLVM IR we have no equivalent for
"builtin_constant_p".
Thus the __builtin_constant_p is evaluated and found false as part of 
converting the GCC trees into LLVM IR, then the always_inliner is 
applied on the IR.

If this particular optimization is of great interest to you, I suggest 
adding this folding to the simplify-libcalls pass. It can check whether 
the parameter to the call is constant and apply the range tests.

For __builtin_constant_p support in general, I think the trick is to 
emit a call to "i1 @__builtin_constant_p" then later on run a pass
that
eliminates them based on isa<Constant>. This is probably what GCC does?

Nick

  For what it's worth, my
> always_inline functions are always functions that allow constant-folding
> of its result when parameters are known at compile time, or fallbacks to
> a real, non inlined, implementation else. Typical (silly but useful for
> my point) use is for example, a function that is:
> 
> extern int __utf8_charlen(wchar_t c);
> __attribute__((always_inline))
> static inline int utf8_charlen(wchar_t c) {
>     if (__builtin_constant_p(c)) {
>         if (c < 0 || c >= 0x200000)
> 	    return 0;
>         return 1 + (c >= 0x80) + (c >= 0x800) + (c >= 0x10000);
>     }
>     return __utf8_charlen(c);
> }
> 
> I really expect that here, if I have a straight utf8_charlen('\0')
call,
> llvm-gcc would first expand utf8_charlen inline, and then, optimizes the
> __builtin_constant_p away. I mean, the cases where llvm-gcc is unable to
> do the optimizations I somehow expect it to do, the arguments are always
> direct constants passed to it, not through any kind of local variable,
> hence a best-effort algorithm should just work.
> 
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

On Mar 15, 2009, at 6:16 PM, Nick Lewycky wrote:
> Pierre Habouzit wrote:
>> [ please CC: me as I'm not subscribed ]
>>
>> On Wed, Mar 11, 2009 at 04:13:34AM +0000, bugzilla- 
>> daemon at cs.uiuc.edu wrote:
>>> http://llvm.org/bugs/show_bug.cgi?id=3756
>>>
>>> Chris Lattner <clattner at apple.com> changed:
>>>
>>>           What    |Removed                     |Added
>>>
----------------------------------------------------------------------------
>>>             Status|NEW                         |RESOLVED
>>>         Resolution|                            |WONTFIX
>>>
>>> --- Comment #6 from Chris Lattner <clattner at apple.com> 
2009-03-10
>>> 23:13:33 ---
>>> Unfortunately, this will never be fixed in either llvm-gcc or
clang.
>>> __builtin_constant_p is a "best effort" constant folding
tester,
>>> which is
>>> allowed to fail.  You should never write code that assumes that
>>> __builtin_constant_p can't fail (if you need that, just
don't use
>>> __builtin_constant_p).
>>
>>
>>> It would be interesting and useful to bring this up on the glibc  
>>> list
>>> to see if they are willing to fix their headers.
>>
>> Hahahaha, you never talked to Uli before did you ?
>>
>>> Barring that, if this is really important to you, please raise the
>>> issue on the
>>> llvmdev list.  We will need IR extensions to support this among  
>>> other things.
>>
>> Well, I don't expect __builtin_constant_p to works always.
That's not
>> precisely the issue. I expect though __attribute__((always_inline))
>> functions to be expanded prior to any optimization pass, because
it's
>> what it means. Those functions are basically type-safe macros.
>>
>> If you do that, then my problem go away at once.
>
> No, that's not the problem.
I disagree; I think incomplete support for always_inline is the  
primary issue
here.
>
> Before any optimizations can be applied the function must be converted
> into LLVM IR. In LLVM IR we have no equivalent for  
> "builtin_constant_p".
> Thus the __builtin_constant_p is evaluated and found false as part of
> converting the GCC trees into LLVM IR, then the always_inliner is
> applied on the IR.
Supporting always_inline as a type-safe macro which is always expanded
early is a feature of GCC that people have found useful.  It is a  
limitation of
LLVM that this happens to be complicated to implement there.

Dan