clangupc / upc2c

Clang based UPC to C Translator
https://clangupc.github.io/clang-upc2c
Other
4 stars 3 forks source link

upc2c fails to emit calls UPCR_EXIT_FUNCTION() #63

Closed PHHargrove closed 10 years ago

PHHargrove commented 10 years ago

The UPCR spec requires a call to UPCR_EXIT_FUNCTION() be emitted before each function return, including any implicit one at the end of the function.

The use of a distinct ReturnStmt node in the AST can be used for the explicit returns. The implicit one could be dealt with much as is done now for the user_main() rewrite.

Note that this is a requirement for full GASP support with backend compilers which do not support __attribute__((__cleanup__)). However, I am pretty certain this function-exit hook is otherwise unused currently.

PHHargrove commented 10 years ago

Simple "proof" that there is enough analysis done already to detect (for the non-void case) the case of control-flow reaching the end of a function w/o a 'return':

$ cat noreturn.upc 
#include <stdio.h>
int foo(void) { puts("Hello"); }

$ bin/clangupc -c noreturn.upc 
noreturn.upc:2:32: warning: control reaches end of non-void function [-Wreturn-type]
int foo(void) { puts("Hello"); }
                               ^
1 warning generated.

$ bin/upc2c noreturn.upc 
noreturn.upc:2:32: warning: control reaches end of non-void function [-Wreturn-type]
int foo(void) { puts("Hello"); }
                               ^
1 warning generated.

I am hoping that one can leverage this to detect all functions (including those with void return type) which contain an implicit return.

Note that the clang doxygen already makes clear that there is no ReturnStmt node created for implicit returns.

PHHargrove commented 10 years ago

Steven,

Sorry to say, but 4601cd6 broke builds on my Mac (see below). Rolling back just that one commit fixes it. However, so does the following 1-line patch:

diff --git a/Transform.cpp b/Transform.cpp
index e759a48..b0fddc1 100644
--- a/Transform.cpp
+++ b/Transform.cpp
@@ -1238,7 +1238,7 @@ namespace {
                                              IsStmtExpr);
     }
     StmtResult TransformReturnStmt(ReturnStmt * S) {
-      StmtResult result = TreeTransform::TransformReturnStmt(S);
+      StmtResult result = TreeTransformUPC::TransformReturnStmt(S);
       SmallVector<Stmt*, 2> Statements;
       std::vector<Expr*> args;
       Statements.push_back(BuildUPCRCall(Decls->UPCR_EXIT_FUNCTION, args).get());

This is a corner of C++ that is beyond my experience. So, could you please confirm that the patch is correct?

-Paul

The failure and compiler ID:

$ make VERBOSE=1
[....] 
[ 88%] Building CXX object tools/clang/tools/upc2c/CMakeFiles/clang-upc2c.dir/Transform.cpp.o
cd /Users/paul/upc2c/bld/tools/clang/tools/upc2c && /usr/bin/c++   -DCLANG_ENABLE_ARCMT -DCLANG_ENABLE_REWRITER -DCLANG_ENABLE_STATIC_ANALYZER -DNDEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-uninitialized -Wnon-virtual-dtor -fno-common -Woverloaded-virtual -Wcast-qual -fno-strict-aliasing -fno-rtti -I/Users/paul/upc2c/bld/tools/clang/tools/upc2c -I/Users/paul/upc2c/src/llvm/tools/clang/tools/upc2c -I/Users/paul/upc2c/src/llvm/tools/clang/include -I/Users/paul/upc2c/bld/tools/clang/include -I/Users/paul/upc2c/bld/include -I/Users/paul/upc2c/src/llvm/include    -fno-exceptions -o CMakeFiles/clang-upc2c.dir/Transform.cpp.o -c /Users/paul/upc2c/src/llvm/tools/clang/tools/upc2c/Transform.cpp
/Users/paul/upc2c/src/llvm/tools/clang/tools/upc2c/Transform.cpp: In member function ‘clang::StmtResult<unnamed>::RemoveUPCTransform::TransformReturnStmt(clang::ReturnStmt*)’:
/Users/paul/upc2c/src/llvm/tools/clang/tools/upc2c/Transform.cpp:1241: error: ‘template<class Derived> class clang::TreeTransform’ used without template parameters
make[2]: *** [tools/clang/tools/upc2c/CMakeFiles/clang-upc2c.dir/Transform.cpp.o] Error 1
make[1]: *** [tools/clang/tools/upc2c/CMakeFiles/clang-upc2c.dir/all] Error 2
make: *** [all] Error 2

$ c++ --version
i686-apple-darwin11-llvm-g++-4.2 (GCC) 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2335.15.00)
Copyright (C) 2007 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
swatanabe commented 10 years ago

AMDG

On 03/11/2014 11:27 PM, Paul H. Hargrove wrote:

Steven,

Sorry to say, but 4601cd6 broke builds on my Mac (see below). Rolling back just that one commit fixes it. However, so does the following 1-line patch:

diff --git a/Transform.cpp b/Transform.cpp
index e759a48..b0fddc1 100644
--- a/Transform.cpp
+++ b/Transform.cpp
@@ -1238,7 +1238,7 @@ namespace {
                                              IsStmtExpr);
     }
     StmtResult TransformReturnStmt(ReturnStmt * S) {
-      StmtResult result = TreeTransform::TransformReturnStmt(S);
+      StmtResult result = TreeTransformUPC::TransformReturnStmt(S);
       SmallVector<Stmt*, 2> Statements;
       std::vector<Expr*> args;
       Statements.push_back(BuildUPCRCall(Decls->UPCR_EXIT_FUNCTION, args).get());

This is a corner of C++ that is beyond my experience. So, could you please confirm that the patch is correct?

Yes, this patch is correct. Please commit.

In Christ, Steven Watanabe

PHHargrove commented 10 years ago

Fixed in https://github.com/Intrepid/upc2c/commit/ade34f3f40b832b9cbe4482547f718b5c0a51381