[cfe-commits] r147655 - in /cfe/trunk: lib/CodeGen/CGBuiltin.cpp test/CodeGen/no-builtin.c

[David Chisnall]

Then I am confused - the test case in the bug report failed for me before making that change and passed after that change...

Apparently (according to Ed, who reported PR11711) the part of the regression test suite of which this is a reduced form also now passes after this change.

David

On 6 Jan 2012, at 15:18, Richard Smith wrote:

> On Fri, January 6, 2012 12:20, David Chisnall wrote:
>> Author: theraven
>> Date: Fri Jan  6 06:20:19 2012
>> New Revision: 147655
>> 
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=147655&view=rev
>> Log:
>> If we are compiling with -fno-builtin then don't do constant folding of
>> builtins.
>> 
>> This fixes PR11711.
>> 
>> 
>> 
>> Added:
>> cfe/trunk/test/CodeGen/no-builtin.c Modified:
>> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>> 
>> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=1
>> 47655&r1=147654&r2=147655&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Jan  6 06:20:19 2012
>> @@ -175,7 +175,8 @@
>> unsigned BuiltinID, const CallExpr *E) { // See if we can constant fold this
>> builtin.  If so, don't emit it at all. Expr::EvalResult Result;
>> -  if (E->EvaluateAsRValue(Result, CGM.getContext()) &&
>> +  if (!getContext().getLangOptions().NoBuiltin &&
>> +      E->EvaluateAsRValue(Result, CGM.getContext()) &&
>> !Result.hasSideEffects()) {
>> if (Result.Val.isInt()) return
>> RValue::get(llvm::ConstantInt::get(getLLVMContext(),
> 
> I don't understand this change. The constant folder doesn't fold any of the
> builtins which the bug report complained about other than fabs. And the
> constant folder only folds them when they're in the __builtin_ form (which
> isn't what the PR was complaining about). And this change doesn't prevent the
> same builtins from being folded in other situations where the constant folder
> is run.
> 
>> Added: cfe/trunk/test/CodeGen/no-builtin.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/no-builtin.c?rev=1
>> 47655&view=auto
>> ==============================================================================
>> --- cfe/trunk/test/CodeGen/no-builtin.c (added)
>> +++ cfe/trunk/test/CodeGen/no-builtin.c Fri Jan  6 06:20:19 2012
>> @@ -0,0 +1,20 @@
>> +// RUN: %clang_cc1 -fno-builtin -emit-llvm %s -o - | FileCheck %s
>> +//
>> +// Check that -fno-builtin prevents us from constant-folding through builtins
>> +// (PR11711)
> 
> I cannot reproduce the problem reported in this bug, before or after your change.
> 
>> +
>> +double
>> +cos(double x)
>> +{
>> +  printf("ok\n");
>> +  exit(0);
>> +}
>> +
>> +int
>> +main(int argc, char *argv[])
>> +{
>> +  cos(1); // CHECK: cos
>> +  printf("not ok\n");
>> +  abort();
>> +}
>> +
> 
> This test passes with or without your change, with or without -fno-builtin.
>