[Bug 3756] __attribute__((always_inline)) and __builtin_constant_p

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

http://llvm.org/bugs/show_bug.cgi?id=3756

Chris Lattner <clattner@apple.com> changed:

           What |Removed |Added
----------------------------------------------------------------------------
             Status>NEW |RESOLVED
         Resolution> >WONTFIX

--- Comment #6 from Chris Lattner <clattner@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.

Pierre Habouzit wrote:

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

http://llvm.org/bugs/show_bug.cgi?id=3756

Chris Lattner <clattner@apple.com> changed:

           What |Removed |Added
----------------------------------------------------------------------------
             Status>NEW |RESOLVED
         Resolution> >WONTFIX

--- Comment #6 from Chris Lattner <clattner@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

Pierre Habouzit wrote:

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

http://llvm.org/bugs/show_bug.cgi?id=3756

Chris Lattner <clattner@apple.com> changed:

          What |Removed |Added
----------------------------------------------------------------------------
            Status>NEW |RESOLVED
        Resolution> >WONTFIX

--- Comment #6 from Chris Lattner <clattner@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

Dan Gohman wrote:

Pierre Habouzit wrote:

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

http://llvm.org/bugs/show_bug.cgi?id=3756

Chris Lattner <clattner@apple.com> changed:

          What |Removed |Added
----------------------------------------------------------------------------
            Status>NEW |RESOLVED
        Resolution> >WONTFIX

--- Comment #6 from Chris Lattner <clattner@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.

Okay. I find it hard to call this a bug in LLVM's always-inliner, as there's nothing you can change in the always-inliner to fix it.

Are you suggesting that we support this by running GCC's always-inliner? I don't think that fixes the example unless you also run some GCC optimizations to perform the forward substitution. That's a problem when we're trying to run the LLVM optimizers instead.

The most straight-forward way to implement it is to leave this function in the code, and write a pass that detects a call to this magic function and does isa<Constant> on the parameter and replaces the call with the result. That pass sounds specific enough to this GCC feature that it should probably just live in the llvm-gcc tree.

Thoughts?

Nick

clang is also going to want any solution here... although, since such
a pass would only be about 10 lines of code, it might not be a big
deal to duplicate it.

-Eli

It's a missing feature in llvm-gcc, from an end-user perspective.
How it might be implemented is an implementation detail.

Dan