From fa3fc14a37fbda9cdbb45bbad780b9f2e6494009 Mon Sep 17 00:00:00 2001 From: jjwilke Date: Thu, 24 Sep 2020 22:52:29 -0700 Subject: [PATCH 1/3] fix pragma deletion logic: Issue #577 --- bin/clang/astVisitor.cc | 6 +- bin/clang/pragmas.cc | 1 - bin/clang/pragmas.h | 1 - tests/Makefile.clang_tests | 1 + .../clang_src2src/pragma_sst_multi_pragma.cc | 68 +++++++++++++++++++ .../test_clang_pragma_sst_memory_cpp.ref-out | 1 + ..._clang_pragma_sst_multi_pragma_cpp.ref-out | 26 +++++++ .../test_clang_pragma_sst_replace_cpp.ref-out | 1 + 8 files changed, 99 insertions(+), 6 deletions(-) create mode 100644 tests/clang_src2src/pragma_sst_multi_pragma.cc create mode 100644 tests/reference/test_clang_pragma_sst_multi_pragma_cpp.ref-out diff --git a/bin/clang/astVisitor.cc b/bin/clang/astVisitor.cc index 66f1490c3..92ddd0127 100644 --- a/bin/clang/astVisitor.cc +++ b/bin/clang/astVisitor.cc @@ -2461,10 +2461,8 @@ void PragmaActivateGuard::deletePragmaText(SSTPragma *prg) { //eliminate the pragma text - if (prg->depth == 0){ - SourceRange rng(prg->pragmaDirectiveLoc, prg->endPragmaLoc); - ::replace(rng, ""); - } + SourceRange rng(prg->pragmaDirectiveLoc, prg->endPragmaLoc); + ::replace(rng, ""); } void diff --git a/bin/clang/pragmas.cc b/bin/clang/pragmas.cc index cc88a70b3..3e60749e2 100644 --- a/bin/clang/pragmas.cc +++ b/bin/clang/pragmas.cc @@ -265,7 +265,6 @@ SSTPragmaHandler::configure(bool delOnUse, Token& /*PragmaTok*/, Preprocessor& maxPragmaDepth = 0; } --pragmaDepth; - fsp->depth = pragmaDepth; } void diff --git a/bin/clang/pragmas.h b/bin/clang/pragmas.h index 2b2a71a3b..0265ed10c 100644 --- a/bin/clang/pragmas.h +++ b/bin/clang/pragmas.h @@ -75,7 +75,6 @@ struct SSTPragma { clang::SourceLocation startPragmaLoc; clang::SourceLocation endPragmaLoc; clang::SourceLocation targetLoc; - int depth; bool deleteOnUse; std::uintptr_t classId; diff --git a/tests/Makefile.clang_tests b/tests/Makefile.clang_tests index 5b4342899..17606ad5c 100644 --- a/tests/Makefile.clang_tests +++ b/tests/Makefile.clang_tests @@ -30,6 +30,7 @@ CLANGTESTS = \ pragma_sst_scoped_replace_cpp \ pragma_sst_replace_cpp \ pragma_sst_memory_cpp \ + pragma_sst_multi_pragma_cpp \ pragma_sst_compute_cpp \ pragma_sst_compute_global_var_cpp \ pragma_sst_loop_count_cpp \ diff --git a/tests/clang_src2src/pragma_sst_multi_pragma.cc b/tests/clang_src2src/pragma_sst_multi_pragma.cc new file mode 100644 index 000000000..b13460f03 --- /dev/null +++ b/tests/clang_src2src/pragma_sst_multi_pragma.cc @@ -0,0 +1,68 @@ +/** +Copyright 2009-2020 National Technology and Engineering Solutions of Sandia, +LLC (NTESS). Under the terms of Contract DE-NA-0003525, the U.S. Government +retains certain rights in this software. + +Sandia National Laboratories is a multimission laboratory managed and operated +by National Technology and Engineering Solutions of Sandia, LLC., a wholly +owned subsidiary of Honeywell International, Inc., for the U.S. Department of +Energy's National Nuclear Security Administration under contract DE-NA0003525. + +Copyright (c) 2009-2020, NTESS + +All rights reserved. + +Redistribution and use in source and binary forms, with or without modification, +are permitted provided that the following conditions are met: + + * Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + + * Redistributions in binary form must reproduce the above + copyright notice, this list of conditions and the following + disclaimer in the documentation and/or other materials provided + with the distribution. + + * Neither the name of the copyright holder nor the names of its + contributors may be used to endorse or promote products derived + from this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +Questions? Contact sst-macro-help@sandia.gov +*/ + +int fxn() +{ + int i=0; + int mul = 0; + double* x = new double[10]; + int* idx = new int[5]; +#pragma sst memory 10 +#pragma sst compute + for (i=0; i < 5; ++i){ + mul *= i; + for (int j=0; j < 10; ++j){ + mul += (j-1); + x[j] += i; + mul -= x[j]; + j=7; + mul += x[j]; + mul *= x[idx[i]]; + idx[i] -= 3; + mul *= x[idx[i]]; + } + } + return 0; +} + diff --git a/tests/reference/test_clang_pragma_sst_memory_cpp.ref-out b/tests/reference/test_clang_pragma_sst_memory_cpp.ref-out index 3eb532058..90cd3324e 100644 --- a/tests/reference/test_clang_pragma_sst_memory_cpp.ref-out +++ b/tests/reference/test_clang_pragma_sst_memory_cpp.ref-out @@ -5,6 +5,7 @@ int fxn() double* x = new double[10]; int* idx = new int[5]; + { uint64_t flops=0; uint64_t readBytes=0; uint64_t writeBytes=0; uint64_t intops=0; { uint64_t tripCount0=(((5)-(0))); intops += tripCount0*1;{ uint64_t tripCount1=tripCount0*(((10)-(0))); flops += tripCount1*1; readBytes += tripCount1*36; writeBytes += tripCount1*12; intops += tripCount1*16;}}readBytes=1000;sstmac_compute_detailed(flops,intops,readBytes); } diff --git a/tests/reference/test_clang_pragma_sst_multi_pragma_cpp.ref-out b/tests/reference/test_clang_pragma_sst_multi_pragma_cpp.ref-out new file mode 100644 index 000000000..06f1e2257 --- /dev/null +++ b/tests/reference/test_clang_pragma_sst_multi_pragma_cpp.ref-out @@ -0,0 +1,26 @@ +int fxn() +{ + int i=0; + int mul = 0; + double* x = new double[10]; + int* idx = new int[5]; + + + { uint64_t flops=0; uint64_t readBytes=0; uint64_t writeBytes=0; uint64_t intops=0; { uint64_t tripCount0=(((5)-(0))); intops += tripCount0*1;{ uint64_t tripCount1=tripCount0*(((10)-(0))); flops += tripCount1*1; readBytes += tripCount1*36; writeBytes += tripCount1*12; intops += tripCount1*16;}}readBytes=10;sstmac_compute_detailed(flops,intops,readBytes); } + + + + + + + + + + + + + return 0; +} +#include +#include + diff --git a/tests/reference/test_clang_pragma_sst_replace_cpp.ref-out b/tests/reference/test_clang_pragma_sst_replace_cpp.ref-out index f5e2e6b96..a7f0e73b7 100644 --- a/tests/reference/test_clang_pragma_sst_replace_cpp.ref-out +++ b/tests/reference/test_clang_pragma_sst_replace_cpp.ref-out @@ -28,6 +28,7 @@ void fxn3() int x = 42; + for (int i=0; i < 3; ++i){ int x = 10; int z = 42; From 7758fbf691fd1c1113b870d8c2acd2b2a5464fde Mon Sep 17 00:00:00 2001 From: jjwilke Date: Fri, 4 Sep 2020 16:18:07 -0700 Subject: [PATCH 2/3] fix pisces empty epoch bug --- sstmac/hardware/pisces/pisces_arbitrator.cc | 50 ++++++++++++++++++--- sstmac/hardware/pisces/pisces_arbitrator.h | 6 +++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/sstmac/hardware/pisces/pisces_arbitrator.cc b/sstmac/hardware/pisces/pisces_arbitrator.cc index e3697eb18..bb00e4e40 100644 --- a/sstmac/hardware/pisces/pisces_arbitrator.cc +++ b/sstmac/hardware/pisces/pisces_arbitrator.cc @@ -50,24 +50,31 @@ Questions? Contact sst-macro-help@sandia.gov #define one_indent " " #define two_indent " " -#if 0 +#if PISCES_DEBUG_INDIVIDUAL_HISTORY +#define dprintf(flag, ...) \ + history_.push_back(sprockit::sprintf(__VA_ARGS__)) +#else +#define dprintf(flag, ...) debug_printf(flag, __VA_ARGS__) +#endif + +#if PISCES_DETAILED_DEBUG #define pflow_arb_debug_printf_l0(format_str, ...) \ - debug_printf(sprockit::dbg::pisces, \ + dprintf(sprockit::dbg::pisces, \ " [arbitrator] " format_str , \ __VA_ARGS__) #define pflow_arb_debug_printf_l1(format_str, ...) \ - debug_printf(sprockit::dbg::pisces, \ + dprintf(sprockit::dbg::pisces, \ one_indent " [arbitrator] " format_str , \ __VA_ARGS__) #define pflow_arb_debug_printf_l2(format_str, ...) \ - debug_printf(sprockit::dbg::pisces, \ + dprintf(sprockit::dbg::pisces, \ two_indent " [arbitrator] " format_str , \ __VA_ARGS__) #define pflow_arb_debug_print_l2(format_str) \ - debug_printf(sprockit::dbg::pisces, \ + dprintf(sprockit::dbg::pisces, \ two_indent " [arbitrator] " format_str "%s", "") #else #define pflow_arb_debug_printf_l0(format_str, ...) @@ -188,6 +195,10 @@ PiscesCutThroughArbitrator::clearOut(Timestamp now) cut_through_epoch_debug("clearing at %9.5e", now.sec()); Timestamp end = epoch->start + epoch->numCycles * cycleLength_; if (now <= epoch->start){ + if (!epoch->next){ + //this is the last epoch, restore it to full size + epoch->numCycles = std::numeric_limits::max(); + } return; } else if (now < end){ if (epoch->next){ @@ -205,6 +216,16 @@ PiscesCutThroughArbitrator::clearOut(Timestamp now) Epoch* next = epoch->next; delete epoch; head_ = next; +#if SSTMAC_SANITY_CHECK + if (head_ == nullptr){ +#if PISCES_DEBUG_INDIVIDUAL_HISTORY + for (auto& str : history_){ + std::cerr << str << std::endl; + } +#endif + spkt_abort_printf("internal error: head epoch is null in PiscesCutThroughArbitrator"); + } +#endif epoch = next; } else { //this is the last epoch - restore it to "full size" @@ -223,6 +244,16 @@ PiscesCutThroughArbitrator::advance(Epoch* epoch, Epoch* prev) if (prev) prev->next = epoch->next; else head_ = next; delete epoch; +#if SSTMAC_SANITY_CHECK + if (head_ == nullptr){ + #if PISCES_DEBUG_INDIVIDUAL_HISTORY + for (auto& str : history_){ + std::cerr << str << std::endl; + } + #endif + spkt_abort_printf("internal error: head epoch is null in PiscesCutThroughArbitrator"); + } +#endif return next; } @@ -232,7 +263,7 @@ PiscesCutThroughArbitrator::arbitrate(IncomingPacket &st) pflow_arb_debug_printf_l0("Cut-through: arbitrator %p starting packet %p:%llu of size %u with byte_delay=%9.5e epoch_delay=%9.5e start=%9.5e: %s", this, st.pkt, st.pkt->flowId(), st.pkt->numBytes(), st.pkt->byteDelay().sec(), byteDelay_.sec(), st.now.sec(), - (st.pkt->orig() ? sprockit::toString(st.pkt->orig()).c_str() : "null payload")); + (st.pkt->flow() ? sprockit::toString(st.pkt->flow()).c_str() : "null payload")); #define PRINT_EPOCHS 0 #if PRINT_EPOCHS @@ -257,6 +288,11 @@ PiscesCutThroughArbitrator::arbitrate(IncomingPacket &st) while (bytesLeft > 2){ //we often end up with 1,2 byte stragglers - ignore them for efficiency #if SSTMAC_SANITY_CHECK if (!epoch){ +#if PISCES_DEBUG_INDIVIDUAL_HISTORY + for (auto& str : history_){ + std::cerr << str << std::endl; + } +#endif spkt_abort_printf("ran out of epochs on arbitrator %p: this should not be possible", this); } #endif @@ -265,7 +301,7 @@ PiscesCutThroughArbitrator::arbitrate(IncomingPacket &st) if (st.pkt->byteDelay() <= cycleLength_){ //every cycle gets used, arriving faster than leaving if (epoch->numCycles <= bytesLeft){ - cut_through_arb_debug_noargs("used all cycles") + cut_through_arb_debug_noargs("used all cycles"); //epoch is completely busy bytesSent += epoch->numCycles; bytesLeft -= epoch->numCycles; diff --git a/sstmac/hardware/pisces/pisces_arbitrator.h b/sstmac/hardware/pisces/pisces_arbitrator.h index 78114510f..3258e0c75 100644 --- a/sstmac/hardware/pisces/pisces_arbitrator.h +++ b/sstmac/hardware/pisces/pisces_arbitrator.h @@ -51,6 +51,9 @@ Questions? Contact sst-macro-help@sandia.gov #include #include +#define PISCES_DEBUG_INDIVIDUAL_HISTORY 0 +#define PISCES_DETAILED_DEBUG 0 + namespace sstmac { namespace hw { @@ -99,6 +102,9 @@ class PiscesBandwidthArbitrator protected: TimeDelta byteDelay_; +#if PISCES_DEBUG_INDIVIDUAL_HISTORY + std::vector history_; +#endif }; From adee708cddf50cb5141d5a9b638b23a24fbba024 Mon Sep 17 00:00:00 2001 From: jjwilke Date: Fri, 4 Sep 2020 16:25:04 -0700 Subject: [PATCH 3/3] fix function skip bug for pragmas on FunctionDecls --- bin/clang/astConsumers.cc | 21 +++++---------------- bin/clang/astVisitor.cc | 20 ++++++++++++++------ bin/clang/pragmas.cc | 3 +++ sstmac/replacements/string.h | 4 ---- 4 files changed, 22 insertions(+), 26 deletions(-) diff --git a/bin/clang/astConsumers.cc b/bin/clang/astConsumers.cc index 33ab1fb9e..4ed5ce20b 100644 --- a/bin/clang/astConsumers.cc +++ b/bin/clang/astConsumers.cc @@ -65,24 +65,13 @@ SkeletonASTConsumer::HandleTopLevelDecl(DeclGroupRef DR) { for (DeclGroupRef::iterator b = DR.begin(), e = DR.end(); b != e; ++b){ Decl* d = *b; - switch(d->getKind()){ - case Decl::Function: { - FunctionDecl* fd = cast(d); - //the function decl will have its body filled in later - //possibly - we need to make sure to only add the function once - if (fd->isThisDeclarationADefinition()){ - //also, we only really care about the definition anyway - allTopLevelDecls_.push_back(d); - } - if (isNullWhitelisted(fd->getNameAsString())){ - CompilerGlobals::astNodeMetadata.nullSafeFunctions[fd] = nullptr; - } + if (d->getKind() == Decl::Function){ + FunctionDecl* fd = cast(d); + if (isNullWhitelisted(fd->getNameAsString())){ + CompilerGlobals::astNodeMetadata.nullSafeFunctions[fd] = nullptr; } - break; - default: - allTopLevelDecls_.push_back(d); - break; } + allTopLevelDecls_.push_back(d); } return true; } diff --git a/bin/clang/astVisitor.cc b/bin/clang/astVisitor.cc index 92ddd0127..e0380b65c 100644 --- a/bin/clang/astVisitor.cc +++ b/bin/clang/astVisitor.cc @@ -1040,6 +1040,9 @@ SkeletonASTVisitor::TraverseCallExpr(CallExpr* expr, DataRecursionQueue* /*queu PragmaActivateGuard pag(expr, this); if (pag.skipVisit()) return true; + //first go into the call expression to see if we should even keep it + //we might get a delete exception, in which case we should skip traversal + //if we got here, we are safe to modify the function name if (CompilerGlobals::modeActive(SKELETONIZE | SHADOWIZE)) { Expr* fxn = getUnderlyingExpr(const_cast(expr->getCallee())); if (fxn->getStmtClass() == Stmt::DeclRefExprClass){ @@ -1939,6 +1942,9 @@ SkeletonASTVisitor::traverseFunctionBody(clang::Stmt* s) bool SkeletonASTVisitor::TraverseFunctionDecl(clang::FunctionDecl* D) { + if (!D->isThisDeclarationADefinition()){ + return true; + } if (D->isMain() && CompilerGlobals::refactorMain){ replaceMain(D); } else if (D->isTemplateInstantiation() @@ -2399,7 +2405,6 @@ SkeletonASTVisitor::TraverseArraySubscriptExpr(ArraySubscriptExpr* expr, DataRec PushGuard pg(activeDerefs_, expr); TraverseStmt(expr->getBase()); TraverseStmt(expr->getIdx()); - return true; } @@ -2443,7 +2448,7 @@ SkeletonASTVisitor::TraverseDecl(Decl *D) PragmaActivateGuard pag(D, this); if (pag.skipVisit()) return true; - RecursiveASTVisitor::TraverseDecl(D); + return RecursiveASTVisitor::TraverseDecl(D); } catch (DeclDeleteException& e) { if (e.deleted != D) throw e; } @@ -2697,16 +2702,19 @@ bool FirstPassASTVisitor::TraverseFunctionDecl(FunctionDecl *fd, DataRecursionQueue* /*queue*/) { PushGuard pg(CompilerGlobals::astContextLists.enclosingFunctionDecls, fd); - Parent::TraverseFunctionDecl(fd); - return true; + PragmaActivateGuard pag(fd, this, true/*always do first pass pragmas*/); + if (fd->isThisDeclarationADefinition()){ + return Parent::TraverseFunctionDecl(fd); + } else { + return true; + } } bool FirstPassASTVisitor::TraverseCompoundStmt(CompoundStmt* cs, DataRecursionQueue* /*queue*/) { PushGuard pg(CompilerGlobals::astContextLists.compoundStmtBlocks, cs); - Parent::TraverseCompoundStmt(cs); - return true; + return Parent::TraverseCompoundStmt(cs); } FirstPassASTVisitor::FirstPassASTVisitor(SSTPragmaList& prgs) : diff --git a/bin/clang/pragmas.cc b/bin/clang/pragmas.cc index 3e60749e2..f5d45661e 100644 --- a/bin/clang/pragmas.cc +++ b/bin/clang/pragmas.cc @@ -152,6 +152,9 @@ static void tokenToString(const Token& tok, std::ostream& os) case tok::kw_true: os << "true"; break; + case tok::kw_void: + os << "void"; + break; case tok::kw_false: os << "false"; break; diff --git a/sstmac/replacements/string.h b/sstmac/replacements/string.h index 03a6d23ca..0768615db 100644 --- a/sstmac/replacements/string.h +++ b/sstmac/replacements/string.h @@ -75,14 +75,10 @@ extern "C" { #pragma clang diagnostic ignored "-Wunknown-pragmas" #endif -#if defined(__clang__) // Only include S2S things if clang #pragma sst null_ptr safe -#endif void* sstmac_memset(void* ptr, int value, unsigned long sz); -#if defined(__clang__) // Only include S2S things if clang #pragma sst null_ptr safe -#endif void* sstmac_memcpy(void* dst, const void* src, unsigned long sz); #if defined(__clang__) // Add back the warnings