clangupc / upc2c

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

TLD: missing TLD macros when accessing upcrt_forall_control #107

Closed nenadv closed 10 years ago

nenadv commented 10 years ago

After Steven's commits for TLD fixes of #100, ... I get only one failure on intrepid test suite:

[intrepid/test10]   0sec  20140725_110545  FAILED (CRASH=SIGABRT/NEW)           
commandline: [ /eng/upc/dev/nenad/clang-upc-pthreads/bld/upcr-packed-dbg/dbg_cupc2c/upcrun -q -n 4  -p 4 ./test10 ]
PassExpr: passed                                                                
FailExpr: rror                                                                  
--- App stdout ---                                                              
--- App stderr ---                                                              
test10: error at element [0,0]. Does not have affinity with thread 3, inside nested upc_forall.
*** Caught a fatal signal: SIGABRT(6) on node 0/1                               
test10: error at element [0,0]. Does not have affinity with thread 2, inside nested upc_forall.
test10: error at element [0,1]. Does not have affinity with thread 0, inside nested upc_forall.
NOTICE: Before reporting bugs, run with GASNET_BACKTRACE=1 in the environment to generate a backtrace.
*** Caught a fatal signal: SIGABRT(6) on node 0/1 

I am running the full test suite now.

PHHargrove commented 10 years ago

The failure within a nested forall could be related to making the state which records the nesting into TLD (or failing to do so). Without looking at code I am guessing the accesses to the control variable in the transformed forall are not using the TLD access macros.

FYI: I am also retesting this pthreads support today.

PHHargrove commented 10 years ago

Without looking at code I am guessing the accesses to the control variable in the transformed forall are not using the TLD access macros.

That guess has been confirmed:

$ grep upcrt_forall_control test10.trans.c
int32_t UPCR_TLD_DEFINE_TENTATIVE(upcrt_forall_control, 4, 4);
            if (upcrt_forall_control) {
                upcrt_forall_control = 1;
                upcrt_forall_control = 0;
PHHargrove commented 10 years ago

There appear to be many tests in the full suite that fail due to the lack of TLD macros when accessing upcrt_forall_control. That omission leads both to nested forall's which don't know they are nested and non-nested foralls which erroneously execute as if they are nested.

Use of upc_forall is sufficiently pervasive in the test suite that I cannot (yet) definitively determine if any of the numerous runtime failures seen in a full harness run are not due to this particular bug.

PHHargrove commented 10 years ago

I have a possible fix in testing now.

PHHargrove commented 10 years ago

With the fix mentioned in my previous comment, the number of "NEW" runtime failures drops from 78 to just 20.

Fixed in e5008aa