This is an archive of the discontinued LLVM Phabricator instance.

Improved __builtin_constant_p
Needs ReviewPublic

Authored by janusz on Jul 14 2014, 5:32 AM.

Details

Reviewers
nlewycky
Summary

Improved __builtin_constant_p() implementation to match GCC closer.
If the compiler cannot confirm that the argument is constant, the function
is lowered to a (new) llvm.is.constant intrinsic and the constantness is
evaluated again after optimisations such as inlining.

Depends on D4276

Diff Detail

Event Timeline

janusz updated this revision to Diff 11370.Jul 14 2014, 5:32 AM
janusz retitled this revision from to Improved __builtin_constant_p.
janusz updated this object.
janusz edited the test plan for this revision. (Show Details)
janusz added a reviewer: nlewycky.
janusz added a subscriber: Unknown Object (MLST).
rsmith added a subscriber: rsmith.Jul 14 2014, 7:50 PM

This makes Clang's constant expression evaluator and its IR generation disagree about the value of __builtin_constant_p. That seems dangerous to me, so I'd like to understand the justification for this a bit better. Random example of why this is dangerous:

int f(int n) {
  if (!__builtin_constant_p(n))
    return 0;
}
int k = f(0);

Here, we'll suppress the -Wreturn-type warning because the frontend believes that it's impossible to reach the } of f. But when we generate IR, we'll fall off the function and experience nasal demons.

What's the motivation behind this change?

In D4492#5, @rsmith wrote:

This makes Clang's constant expression evaluator and its IR generation disagree about the value of __builtin_constant_p.

How would you address this issue? My concern is that there are cases, where we need to fold to a constant early (i.e. global variable initialisation) and other cases where we can emit IR code and make the decision about 'constantness' at a later stage.

What's the motivation behind this change?

Matching GCC behaviour as closely as we can.