From f1acb7e8e0305d2ce719d341de29d087fa143b83 Mon Sep 17 00:00:00 2001
From: Clark Coleman <clc@zephyr-software.com>
Date: Thu, 19 Nov 2020 00:23:22 -0500
Subject: [PATCH] SPARK: Implement new algorithm for
 FindConditionalFollowNode(); compare to old.

---
 include/base/SMPFunction.h |   9 +-
 src/base/SMPBasicBlock.cpp |  23 +-
 src/base/SMPFunction.cpp   | 566 ++++++++++++++++++++++++++++---------
 src/base/SMPInstr.cpp      |  93 ++++--
 src/base/SMPProgram.cpp    |   4 +-
 5 files changed, 525 insertions(+), 170 deletions(-)

diff --git a/include/base/SMPFunction.h b/include/base/SMPFunction.h
index 3c841d51..997b69b1 100644
--- a/include/base/SMPFunction.h
+++ b/include/base/SMPFunction.h
@@ -554,7 +554,7 @@ public:
 
 	// Printing methods
 	void Dump(void); // debug dump
-	void DumpDotCFG(void) const; // Dump CFG representation for processing by "dot" program.
+	void DumpDotCFG(void); // Dump CFG representation for processing by "dot" program.
 	inline void DumpFuncNameAndAddr(void) const { SMP_msg("in %s at %llx\n", this->GetFuncName(), (uint64_t) this->GetFirstFuncAddr()); };
 
 	// Analysis methods
@@ -706,6 +706,7 @@ private:
 	bool HasMallocCall; // calls malloc()
 	bool TranslatingSPARKLoop; // Currently translating a loop into a separate SPARK Ada procedure
 	bool PrintedSPARKUnstructuredMsg; // Printed a message about current function being unstructured
+	bool PrintedCFG; // DumpDotCFG() has been called
 	
 	long TypedDefs;  // How many DEFs in instructions were not UNINIT type after InferTypes()
 	long UntypedDefs; // How many DEFs in instructions are UNINIT type after InferTypes()
@@ -983,7 +984,7 @@ private:
 	bool DoesBlockExitLoop(std::size_t LoopNumber, SMPBasicBlock *LoopBlock, int &FollowBlockNum); // return true if block can exit the loop.
 	bool DetectMultiLevelLoopBreak(const int LoopBlockNum, const int ExitTargetBlockNum) const; // Is ExitTargetBlockNum more than 1 loop nesting level away from LoopBlockNum?
 	void ComputeLoopFollowNodesReachability(const std::size_t LoopNum); // populate entry in LoopExitTargetsReachabilitySets
-	void MarkReachableBlocks(const int BlockNum, const std::size_t LoopNum, int LoopHeadBlockNum, STARSBitSet &ReachableSet); // Set bits of current BlockNum and its descendants in CFG that are dominated by LoopHeadBlockNum
+	void MarkReachableBlocks(const int BlockNum, const int LoopNum, int LoopHeadBlockNum, STARSBitSet &ReachableSet); // Set bits of current BlockNum and its descendants in CFG that are dominated by LoopHeadBlockNum
 	uint32_t GetReachableSourcesCount(const std::size_t BlockNum, const std::size_t LoopNum) const; // How many of the multiple exit targets from LoopNum reach BlockNum?
 	void DetectLoopInvariantDEFs(void); // Collect a set of loop-invariant DEFs with the inst IDs of the DEFs.
 	void DetectLoopInvariantDEFs2(void); // Collect a set of loop-invariant DEFs with the inst IDs of the DEFs.
@@ -1030,8 +1031,10 @@ private:
 	bool AnalyzeCompoundConditionalStatements(void); // Analyze if-statements with short-circuit operators
 	bool AnalyzeConditionalStatements(void); // Analyze if-then-elses starting at COND_BRANCH at end of all blocks; return false if not well-structured
 	int FindConditionalFollowNode(int HeadBlockNum); // Find candidate block # for if-else follow node for HeadBlockNum; return -1 otherwise
+	int FindConditionalFollowNode2(int HeadBlockNum); // Find candidate block # for if-else follow node for HeadBlockNum; return -1 otherwise
+	int FindConditionalFollowNode3(int HeadBlockNum); // Find candidate block # for if-else follow node for HeadBlockNum; return -1 otherwise
 	int TrackConditionalBranchTerminus(int BranchHeadBlockNum, int CurrBlockNum, STARSBitSet &BlocksSeen, int &BlockAlreadySeenCounter); // Track CurrBlockNum until we reach block that BranchHeadBlockNum doesn't dominate, or dead end. Return block num, possibly SMP_BLOCKNUM_COMMON_RETURN.
-	bool FindUnstructuredIfThenElse(const int HeadBlockNum, const int FollowBlockNum) const; // Do IF and ELSE branches merge before FollowBlockNum?
+	bool FindUnstructuredIfThenElse(const int HeadBlockNum, const int ThenBlockNum, const int ElseBlockNum, const int FollowBlockNum) const; // Do IF and ELSE branches merge before FollowBlockNum?
 	bool FindMultiLoopContinue(const int BranchBlockNum); // Does COND_BRANCH behave as an unstructured multi-level loop continue statement?
 	void FindGuardedLoops(void); // Find guarded loops and fill the GuardToLoopMap and LoopToGuardMap
 
diff --git a/src/base/SMPBasicBlock.cpp b/src/base/SMPBasicBlock.cpp
index 1f4e750d..2912fe70 100644
--- a/src/base/SMPBasicBlock.cpp
+++ b/src/base/SMPBasicBlock.cpp
@@ -389,24 +389,39 @@ bool SMPBasicBlock::IsBlockReachableWithoutBackEdges(const int DestBlockNum) con
 	bool Found = false;
 	list<SMPBasicBlock *>::const_iterator CurrSucc;
 	int CurrBlockNum = this->GetNumber();
-	for (CurrSucc = this->GetFirstConstSucc(); CurrSucc != this->GetLastConstSucc(); ++CurrSucc) {
+
+	// See if we can get lucky by finding the block in the DomFrontier.
+	//  This search is more breadth-first; combining breadth-first checks
+	//  into the depth-first search can speed up the search in a deep CFG.
+	for (int DomFrontierBlockNum : this->DomFrontier) {
+		if (DomFrontierBlockNum == DestBlockNum) {
+			Found = true;
+			break;
+		}
+	} // end for all DomFrontier blocks
+
+	for (CurrSucc = this->GetFirstConstSucc(); !Found && (CurrSucc != this->GetLastConstSucc()); ++CurrSucc) {
 		SMPBasicBlock *SuccBlock = (*CurrSucc);
 		int SuccBlockNum = SuccBlock->GetNumber();
 		if (SuccBlockNum > CurrBlockNum) { // not back edge
 			if (SuccBlockNum == DestBlockNum) {
 				Found = true;
+				SuccBlock->SetSCCPVisited(true);
+			}
+			else if (SuccBlock->IsSCCPVisited()) { // already hit this block on our search
+				break; // return false
 			}
 			else if (SuccBlockNum > DestBlockNum) {
 				// RPO ordering means we can no longer reach DestBlockNum without a back edge.
+				SuccBlock->SetSCCPVisited(true);
 				break; // return false
 			}
 			else {
+				SuccBlock->SetSCCPVisited(true);
 				Found = SuccBlock->IsBlockReachableWithoutBackEdges(DestBlockNum); // recurse, depth-first
 			}
-			if (Found)
-				break;
 		}
-	}
+	} // end for all successor blocks
 
 	return Found;
 } // end of SMPBasicBlock::IsBlockReachableWithoutBackEdges()
diff --git a/src/base/SMPFunction.cpp b/src/base/SMPFunction.cpp
index 85abc724..fdcd719a 100644
--- a/src/base/SMPFunction.cpp
+++ b/src/base/SMPFunction.cpp
@@ -305,6 +305,7 @@ SMPFunction::SMPFunction(STARS_Function_t *Info, SMPProgram* pgm) {
 	this->HasMallocCall = false;
 	this->TranslatingSPARKLoop = false;
 	this->PrintedSPARKUnstructuredMsg = false;
+	this->PrintedCFG = false;
 	this->TypedDefs = 0;
 	this->UntypedDefs = 0;
 	this->TypedPhiDefs = 0;
@@ -1557,8 +1558,8 @@ void SMPFunction::SetControlFlowType(STARS_ea_t InstAddr, ControlFlowType JumpTy
 	}
 	else { // old entry found; update
 		// We permit a LOOP_EXIT to become a SHORT_CIRCUIT_LOOP_EXIT
-		bool ExitCase = (SHORT_CIRCUIT_LOOP_EXIT == JumpTypeCode) && (LOOP_EXIT == MapIter->second);
-		bool InvertedExitCase = (SHORT_CIRCUIT_INVERTED_LOOP_EXIT == JumpTypeCode) && (INVERTED_LOOP_EXIT == MapIter->second);
+		bool ExitCase = (SHORT_CIRCUIT_LOOP_EXIT == JumpTypeCode) && ((LOOP_EXIT == MapIter->second) || (SHORT_CIRCUIT_BRANCH == MapIter->second));
+		bool InvertedExitCase = (SHORT_CIRCUIT_INVERTED_LOOP_EXIT == JumpTypeCode) && ((INVERTED_LOOP_EXIT == MapIter->second) || (SHORT_CIRCUIT_BRANCH == MapIter->second));
 #if 1	// probably need to make SHORT_CIRCUIT_INVERTED_LOOP_EXIT in our caller
 		bool SuspectExitCase = (SHORT_CIRCUIT_LOOP_EXIT == JumpTypeCode) && (INVERTED_LOOP_EXIT == MapIter->second);
 #endif
@@ -7377,7 +7378,7 @@ void SMPFunction::ComputeLoopFollowNodesReachability(const std::size_t LoopNum)
 	for (int FollowNodeNum : this->LoopExitTargets[LoopNum]) {
 		STARSBitSet Reachable;
 		Reachable.AllocateBits(this->GetNumBlocks());
-		this->MarkReachableBlocks(FollowNodeNum, LoopNum, LoopHeadBlockNum, Reachable);
+		this->MarkReachableBlocks(FollowNodeNum, (int)LoopNum, LoopHeadBlockNum, Reachable);
 		pair<int, STARSBitSet> MapItem(FollowNodeNum, Reachable);
 		this->LoopExitTargetsReachabilitySets[LoopNum].insert(MapItem);
 	}
@@ -7403,8 +7404,9 @@ void SMPFunction::ComputeLoopFollowNodesReachability(const std::size_t LoopNum)
 } // end of SMPFunction::ComputeLoopFollowNodesReachability()
 
 // Set bits of current BlockNum and its descendants in CFG that are dominated by LoopHeadBlockNum
-void SMPFunction::MarkReachableBlocks(const int BlockNum, const size_t LoopNum, const int LoopHeadBlockNum, STARSBitSet &ReachableSet) {
-	if (!ReachableSet.GetBit((size_t)BlockNum)) { // avoid looping
+// NOTE: LoopHeadBlockNum can be any dominating block num, with LoopNum < 0 indicating we are not in a loop.
+void SMPFunction::MarkReachableBlocks(const int BlockNum, const int LoopNum, const int LoopHeadBlockNum, STARSBitSet &ReachableSet) {
+	if (!ReachableSet.GetBit((size_t)BlockNum)) { // avoid looping and duplication of work
 		ReachableSet.SetBit((size_t)BlockNum); // Initialize to itself being reachable
 		SMPBasicBlock *CurrBlock = this->GetBlockByNum((size_t)BlockNum);
 		assert(nullptr != CurrBlock);
@@ -7470,7 +7472,8 @@ void SMPFunction::MarkReachableBlocks(const int BlockNum, const size_t LoopNum,
 				//  our translation. Therefore we end our reachability marking when we are about to exit another
 				//  loop level. Such a multi-level trace can be detected by the existing method that detects
 				//  multi-level loop breaks, which are unstructured and need to be structured separately.
-				if (!this->DetectMultiLevelLoopBreak(LoopHeadBlockNum, SuccBlockNum)) {
+				bool BadLoopBreak = (0 > LoopNum) ? false : this->DetectMultiLevelLoopBreak(LoopHeadBlockNum, SuccBlockNum);
+				if (!BadLoopBreak) {
 					if (this->DoesBlockDominateBlock(LoopHeadBlockNum, SuccBlockNum)) {
 						this->MarkReachableBlocks(SuccBlockNum, LoopNum, LoopHeadBlockNum, ReachableSet);
 					}
@@ -7481,9 +7484,10 @@ void SMPFunction::MarkReachableBlocks(const int BlockNum, const size_t LoopNum,
 						//  reachable blocks.
 						// NOTE: If the ExitTargetBlock (BlockNum) is not dominated by LoopHeadBlockNum,
 						//  then we should not be looking at its successors in the first place.
-						if (this->DoesBlockDominateBlock(LoopHeadBlockNum, BlockNum)) {
-							size_t LoopNum = this->GetLoopNumFromHeaderBlockNum(LoopHeadBlockNum);
-							this->MultiLoopExitTargetsConvergenceMap[LoopNum].insert(SuccBlockNum);
+						if (0 <= LoopNum) {
+							if (this->DoesBlockDominateBlock(LoopHeadBlockNum, BlockNum)) {
+								this->MultiLoopExitTargetsConvergenceMap[(size_t)LoopNum].insert(SuccBlockNum);
+							}
 						}
 					}
 				}
@@ -11783,8 +11787,8 @@ bool SMPFunction::AnalyzeCompoundConditionalStatements(void) {
 							int OldFollowBlockNum = ExistingValueIter->second;
 							if (OldFollowBlockNum != FollowBlockNum) {
 								if (!this->PrintedSPARKUnstructuredMsg) {
-									SMP_msg("ERROR: SPARK: Unstructured due to short-circuit CFG JumpFollowNodesMap conflict at %llx Old: %d New:%d ",
-										(uint64_t)LastAddr, OldFollowBlockNum, FollowBlockNum);
+									SMP_msg("ERROR: SPARK: Unstructured due to short-circuit CFG JumpFollowNodesMap conflict at %llx in block %d Old: %d New:%d ",
+										(uint64_t)LastAddr, (*PredIter)->GetNumber(), OldFollowBlockNum, FollowBlockNum);
 									this->DumpFuncNameAndAddr();
 									this->PrintedSPARKUnstructuredMsg = true;
 								}
@@ -11809,8 +11813,8 @@ bool SMPFunction::AnalyzeCompoundConditionalStatements(void) {
 						int OldFollowBlockNum = ExistingValueIter->second;
 						if (OldFollowBlockNum != FollowBlockNum) {
 							if (!this->PrintedSPARKUnstructuredMsg) {
-								SMP_msg("ERROR: SPARK: Unstructured due to short-circuit CFG JumpFollowNodesMap conflict at %llx Old: %d New:%d ",
-									(uint64_t)LastAddr, OldFollowBlockNum, FollowBlockNum);
+								SMP_msg("ERROR: SPARK: Unstructured due to short-circuit CFG JumpFollowNodesMap conflict at %llx in block %zu Old: %d New:%d ",
+									(uint64_t)LastAddr, BlockIndex, OldFollowBlockNum, FollowBlockNum);
 								this->DumpFuncNameAndAddr();
 								this->PrintedSPARKUnstructuredMsg = true;
 							}
@@ -11937,8 +11941,12 @@ bool SMPFunction::AnalyzeConditionalStatements(void) {
 				if (IfThenCase) {
 					this->SetControlFlowType(LastAddr, BRANCH_IF_THEN);
 					if (!UnresolvedBranchBlocks.empty()) { // error; cannot have elsif branches for IfThenCase
-						SMP_msg("ERROR: if-then-endif at %llx has unresolved branches inside.\n",
-							(unsigned long long) LastAddr);
+						if (!this->PrintedSPARKUnstructuredMsg) {
+							SMP_msg("ERROR: SPARK: Unstructured due to if-then-endif at %llx has unresolved branches inside ",
+								(uint64_t)LastAddr);
+							this->DumpFuncNameAndAddr();
+							this->PrintedSPARKUnstructuredMsg = true;
+						}
 						StructuredConditional = false;
 						break;
 					}
@@ -12038,12 +12046,63 @@ bool SMPFunction::AnalyzeConditionalStatements(void) {
 	} // end for all blocks in reverse iteration
 
 	// We should have no unresolved elsif branches
-	return (StructuredConditional && UnresolvedBranchBlocks.empty());
+	if (!UnresolvedBranchBlocks.empty()) {
+		if (!this->PrintedSPARKUnstructuredMsg) {
+			SMP_msg("ERROR: SPARK: Unstructured due to unresolved branch blocks ");
+			this->DumpFuncNameAndAddr();
+			for (int BlockNum : UnresolvedBranchBlocks) {
+				SMP_msg("%d ", BlockNum);
+			}
+			SMP_msg("\n");
+			this->PrintedSPARKUnstructuredMsg = true;
+		}
+		StructuredConditional = false;
+	}
+	return StructuredConditional;
 } // end of SMPFunction::AnalyzeConditionalStatements()
 
-// Find candidate block # for if-else follow node for HeadBlockNum; return -1 otherwise
 int SMPFunction::FindConditionalFollowNode(int HeadBlockNum) {
+	int ReturnFollowBlockNum = SMP_BLOCKNUM_UNINIT;
+	int OldAlgorithmFollowNode = this->FindConditionalFollowNode2(HeadBlockNum);
+#if 0
+	ReturnFollowBlockNum = OldAlgorithmFollowNode;
+#else
+	int NewAlgorithmFollowNode = this->FindConditionalFollowNode3(HeadBlockNum);
+	bool BadOld = (OldAlgorithmFollowNode == SMP_BLOCKNUM_UNINIT);
+	bool BadNew = (NewAlgorithmFollowNode == SMP_BLOCKNUM_UNINIT);
+	if (BadOld) {
+		if (BadNew) {
+			SMP_msg("INFO: SPARK: Failure on both versions of FindConditionalFollowNode at block %d ", HeadBlockNum);
+		}
+		else {
+			SMP_msg("INFO: SPARK: Failure on old version, success on new version of FindConditionalFollowNode at block %d Follow: %d ", HeadBlockNum, NewAlgorithmFollowNode);
+			ReturnFollowBlockNum = NewAlgorithmFollowNode;
+		}
+	}
+	else {
+		if (BadNew) {
+			SMP_msg("INFO: SPARK: Success on old version, failure on new version of FindConditionalFollowNode at block %d Follow: %d ", HeadBlockNum, OldAlgorithmFollowNode);
+			ReturnFollowBlockNum = OldAlgorithmFollowNode;
+		}
+		else {
+			SMP_msg("INFO: SPARK: Success on both versions of FindConditionalFollowNode at block %d ", HeadBlockNum);
+			if (OldAlgorithmFollowNode != NewAlgorithmFollowNode) {
+				SMP_msg("Inconsistent answers: Old %d New %d ", OldAlgorithmFollowNode, NewAlgorithmFollowNode);
+			}
+			ReturnFollowBlockNum = NewAlgorithmFollowNode;
+		}
+	}
+	this->DumpFuncNameAndAddr();
+#endif
+
+	return ReturnFollowBlockNum;
+} // end of SMPFunction::FindConditionalFollowNode()
+
+// Find candidate block # for if-else follow node for HeadBlockNum; return -1 otherwise
+int SMPFunction::FindConditionalFollowNode2(int HeadBlockNum) {
 	int FollowBlockNum = SMP_BLOCKNUM_UNINIT;
+	int ElseBlockNum = SMP_BLOCKNUM_UNINIT;
+	int ThenBlockNum = SMP_BLOCKNUM_UNINIT;
 	bool VerboseOutput = global_stars_interface->VerboseSPARKMode();
 
 	// Guard against pathological code like jz $+2 which jumps and falls through
@@ -12078,6 +12137,9 @@ int SMPFunction::FindConditionalFollowNode(int HeadBlockNum) {
 	bool ShortCircuit = (IsShortCircuitFlow(LastCFType));
 
 	// Go through DomTree[HeadBlockNum] in reverse order until we find a well-structured candidate or fail.
+#if 1
+	if (!ShortCircuit)
+#endif
 	for (int CurrBlockNum : this->DomTree[HeadBlockNum].second) {
 		if (2 <= this->RPOBlocks[(size_t) CurrBlockNum]->GetNumPredsMinusBackEdges()) { // candidate
 			if (!(ShortCircuit && this->ShadowCFGBlocks[(size_t)CurrBlockNum]->IsCoalesced())) {
@@ -12126,9 +12188,9 @@ int SMPFunction::FindConditionalFollowNode(int HeadBlockNum) {
 				list<SMPBasicBlock *>::const_iterator SuccIter;
 				for (SuccIter = CurrBlock->GetFirstConstSucc(); SuccIter != CurrBlock->GetLastConstSucc(); ++SuccIter) {
 					int SuccBlockNum = (*SuccIter)->GetNumber();
-					if (SuccBlockNum == InnerMostFollowBlockNum) {
+					if (this->IsBlockLoopFollowBlock((size_t)InnerMostLoopNum, SuccBlockNum)) {
 						HitLoopFollowBlock = true;
-						FollowBlockNum = InnerMostFollowBlockNum;
+						FollowBlockNum = MultiExitLoop ? SMP_BLOCKNUM_COMMON_RETURN : SuccBlockNum;
 					}
 					else if (!this->IsBlockInLoop(SuccBlockNum, (size_t)InnerMostLoopNum)) {
 						// We are exiting the innermost loop without finding InnerMostFollowBlockNum.
@@ -12153,7 +12215,7 @@ int SMPFunction::FindConditionalFollowNode(int HeadBlockNum) {
 	}
 
 	if (0 <= FollowBlockNum) {
-		if (this->FindUnstructuredIfThenElse(HeadBlockNum, FollowBlockNum)) {
+		if (this->FindUnstructuredIfThenElse(HeadBlockNum, ThenBlockNum, ElseBlockNum, FollowBlockNum)) {
 			FollowBlockNum = SMP_BLOCKNUM_UNINIT; // error signal
 		}
 		return FollowBlockNum;
@@ -12289,38 +12351,54 @@ int SMPFunction::FindConditionalFollowNode(int HeadBlockNum) {
 		}
 		else {
 			assert(2 == CurrBlock->GetNumSuccessors());
-			int ElseBlockNum = (*(CurrBlock->GetCondNonFallThroughSucc()))->GetNumber();
-			assert(SMP_BLOCKNUM_UNINIT != ElseBlockNum);
-			int ThenBlockNum = (*(CurrBlock->GetCondOtherSucc(ElseBlockNum)))->GetNumber();
-			assert(SMP_BLOCKNUM_UNINIT != ThenBlockNum);
-
-			// For an if-then-else, both successors should be dominated by the branch header block.
-			//  If one is dominated but the other is not, then the non-dominated block is the follow block
-			//  and we just have an if-then structure, e.g.
-			//    branch_block
-			//        |
-			//        +-----+
-			//        |     |
-			//   |    |    then_block
-			//   |    |     |
-			//   +--block2--+
-			//   
-			//   block2 is reachable from somewhere above branch_block. We don't want to start
-			//   at block2 and search downwards, because block2 is already the follow block for
-			//   branch_block.
-			int ThenIDom = this->IDom[(size_t)ThenBlockNum];
-			int ElseIDom = this->IDom[(size_t)ElseBlockNum];
-			bool IfThenElseCase = ((ThenIDom == ElseIDom) && (ThenIDom == HeadBlockNum));
+			int ElseIDom = SMP_BLOCKNUM_UNINIT;
+			int ThenIDom = SMP_BLOCKNUM_UNINIT;
+			bool IfThenElseCase = true;
+
+			if (!ShortCircuit) {
+				ElseBlockNum = (*(CurrBlock->GetCondNonFallThroughSucc()))->GetNumber();
+				assert(SMP_BLOCKNUM_UNINIT != ElseBlockNum);
+				ThenBlockNum = (*(CurrBlock->GetCondOtherSucc(ElseBlockNum)))->GetNumber();
+				assert(SMP_BLOCKNUM_UNINIT != ThenBlockNum);
+
+				// For an if-then-else, both successors should be immediately dominated by the branch header block.
+				//  If one is immediately dominated but the other is not, then the non-dominated block is the follow block
+				//  and we just have an if-then structure, e.g.
+				//    branch_block
+				//        |
+				//        +-----+
+				//        |     |
+				//   |    |    then_block
+				//   |    |     |
+				//   +--block2--+
+				//   
+				//   block2 is reachable from somewhere above branch_block. We don't want to start
+				//   at block2 and search downwards, because block2 is already the follow block for
+				//   branch_block.
+				ThenIDom = this->IDom[(size_t)ThenBlockNum];
+				ElseIDom = this->IDom[(size_t)ElseBlockNum];
+				IfThenElseCase = ((ThenIDom == ElseIDom) && (ThenIDom == HeadBlockNum));
+			}
+			else { // ShortCircuit
+				STARSCFGBlock *HeadCFGBlock = this->GetCFGBlockByNum((size_t)HeadBlockNum);
+				ThenBlockNum = HeadCFGBlock->GetExpr()->GetFallThroughBlockNum();
+				ElseBlockNum = HeadCFGBlock->GetExpr()->GetNonFallThroughBlockNum();
+				// Fudge IDoms so short circuit branches follow regular branch pattern.
+				ThenIDom = ElseIDom = HeadBlockNum;
+			}
+
 			if (IfThenElseCase) {
 				// Screen out remaining if-then cases by ensuring that neither branch
 				//  can reach the other without following a back edge. It is sufficient
 				//  to check whether the lower RPO-numbered block can reach the other.
 				if (ThenBlockNum < ElseBlockNum) {
 					SMPBasicBlock *ThenBlock = this->GetBlockByNum(ThenBlockNum);
+					this->ResetSCCPVisitedBlocks(); // SCCP is long since done; reuse its visited flag in each block
 					IfThenElseCase = (!ThenBlock->IsBlockReachableWithoutBackEdges(ElseBlockNum));
 				}
 				else {
 					SMPBasicBlock *ElseBlock = this->GetBlockByNum(ElseBlockNum);
+					this->ResetSCCPVisitedBlocks(); // SCCP is long since done; reuse its visited flag in each block
 					IfThenElseCase = (!ElseBlock->IsBlockReachableWithoutBackEdges(ThenBlockNum));
 				}
 			}
@@ -12424,13 +12502,223 @@ int SMPFunction::FindConditionalFollowNode(int HeadBlockNum) {
 	}
 
 	if (0 <= FollowBlockNum) {
-		if (this->FindUnstructuredIfThenElse(HeadBlockNum, FollowBlockNum)) {
+		if (this->FindUnstructuredIfThenElse(HeadBlockNum, ThenBlockNum, ElseBlockNum, FollowBlockNum)) {
 			FollowBlockNum = SMP_BLOCKNUM_UNINIT; // error signal
 		}
 	}
 
 	return FollowBlockNum;
-} // end of SMPFunction::FindConditionalFollowNode()
+} // end of SMPFunction::FindConditionalFollowNode2()
+
+// Find candidate block # for if-else follow node for HeadBlockNum; return -1 otherwise
+int SMPFunction::FindConditionalFollowNode3(int HeadBlockNum) {
+	int FollowBlockNum = SMP_BLOCKNUM_UNINIT;
+	int NFTBlockNum = SMP_BLOCKNUM_UNINIT;
+	int FTBlockNum = SMP_BLOCKNUM_UNINIT;
+	bool VerboseOutput = global_stars_interface->VerboseSPARKMode();
+
+	// Guard against pathological code like jz $+2 which jumps and falls through
+	//  to the same ensuing instruction.
+	SMPBasicBlock *HeadBlock = this->RPOBlocks[HeadBlockNum];
+	assert(nullptr != HeadBlock);
+	if (1 == HeadBlock->GetNumSuccessors()) {
+		if (!this->PrintedSPARKUnstructuredMsg) {
+			SMP_msg("ERROR: SPARK: Unstructured due to failure in FindConditionalFollowNode due to single successor in HeadBlockNum %d ", HeadBlockNum);
+			this->DumpFuncNameAndAddr();
+			this->PrintedSPARKUnstructuredMsg = true;
+		}
+		return FollowBlockNum;
+	}
+	else if (0 == HeadBlock->GetNumSuccessors()) { // must branch and fall-through outside function
+		if (!this->PrintedSPARKUnstructuredMsg) {
+			SMP_msg("ERROR: SPARK: Unstructured due to failure in FindConditionalFollowNode for FarBranch in HeadBlockNum %d ", HeadBlockNum);
+			this->DumpFuncNameAndAddr();
+			this->PrintedSPARKUnstructuredMsg = true;
+		}
+		return FollowBlockNum;
+	}
+
+	// Screen out branches that go outside their current loop in an unstructured way.
+	if (this->FindMultiLoopContinue(HeadBlockNum)) {
+		return FollowBlockNum;
+	}
+
+	STARS_ea_t HeadLastAddr = HeadBlock->GetLastAddr();
+	bool HeadBlockIsInALoop = this->IsBlockInAnyLoop(HeadBlockNum);
+	ControlFlowType LastCFType = this->GetControlFlowType(HeadLastAddr);
+	int InnerMostLoopNum = HeadBlockIsInALoop ? this->GetInnermostLoopNum(HeadBlockNum) : -1;
+
+	bool ShortCircuit = (IsShortCircuitFlow(LastCFType));
+
+	assert(2 == HeadBlock->GetNumSuccessors());
+
+	// The simple new algorithm depends entirely on the reachable blocks from each of the
+	//  two successors of the COND_BRANCH. Without following back edges, and staying within
+	//  the region dominated by HeadBlockNum, we mark reachable blocks in bitsets for each
+	//  of the two COND_BRANCH successor blocks. Then we have the following cases:
+	//  0. If one of the COND_BRANCH successor blocks dominates HeadBlock, we have a back edge that
+	//     cannot be followed, so we cannot have an intersection of traces from the COND_BRANCH successor
+	//     blocks. So, set the code for structured lack of intersection (SMP_BLOCKNUM_COMMON_RETURN) as
+	//     the follow block.
+	//  1. No intersection of the two bitsets. FollowBlockNum is -2 (SMP_BLOCKNUM_COMMON_RETURN)
+	//     indicating the lack of intersection, with each trace ending in a block with no successors
+	//     (e.g. return instructions or calls to non-returning functions or halt instructions or LOOP_BACKs).
+	//     EXCEPTION: Intersection point was about to happen, but it is just out of the region dominated
+	//     by HeadBlockNum, e.g. conditional join point is also a loop follow block for an enclosing loop or
+	//     is a join point for an enclosing conditional. This can be detected by seeing if the end of each
+	//     of the two traces has only one successor, and the successor is the same for each trace.
+	//  2. Intersection point is the non-fall-through successor of the HeadBlock: simple if-then case,
+	//     with the intersection point being the FollowBlockNum.
+	//  3. Intersection point is the fall-through successor of the HeadBlock: OddIfThenCase,
+	//     with the intersection point being the FollowBlockNum.
+	//  4. Intersection point is neither successor of the HeadBlock, but is dominated by HeadBlock. Subcases:
+	//   4A. Blocks reachable from both traces continue after the intersection point, but no block with
+	//       RPO number greater than the intersection block is reachable from only one trace. Structured,
+	//       with the intersection point being the FollowBlockNum.
+	//   4B. Like 4A, but at least one block with RPO number greater than the intersection point is reachable
+	//       from only one of the two traces. UNSTRUCTURED.
+	//   4C: Conditional is inside a loop, and 2 or more loop tail blocks for that loop are reachable from the
+	//       two traces. The decompilation of the conditional will need to continue until the loop backs and loop
+	//       boundaries are reached, so the follow block needs to be -2 (SMP_BLOCKNUM_COMMON_RETURN).
+	//  5. Intersection point is not dominated by HeadBlock. Some path is entering the conditional in the 
+	//     middle. UNSTRUCTURED.
+
+	if (!ShortCircuit) {
+		NFTBlockNum = (*(HeadBlock->GetCondNonFallThroughSucc()))->GetNumber();
+		assert(SMP_BLOCKNUM_UNINIT != NFTBlockNum);
+		FTBlockNum = (*(HeadBlock->GetCondOtherSucc(NFTBlockNum)))->GetNumber();
+		assert(SMP_BLOCKNUM_UNINIT != FTBlockNum);
+	}
+	else { // ShortCircuit
+		STARSCFGBlock *HeadCFGBlock = this->GetCFGBlockByNum((size_t)HeadBlockNum);
+		FTBlockNum = HeadCFGBlock->GetExpr()->GetFallThroughBlockNum();
+		NFTBlockNum = HeadCFGBlock->GetExpr()->GetNonFallThroughBlockNum();
+	}
+
+	STARSBitSet FTBlocksSeen;
+	STARSBitSet NFTBlocksSeen;
+	FTBlocksSeen.AllocateBits(this->GetNumBlocks());
+	NFTBlocksSeen.AllocateBits(this->GetNumBlocks());
+
+	this->MarkReachableBlocks(FTBlockNum, -1, HeadBlockNum, FTBlocksSeen);
+	this->MarkReachableBlocks(NFTBlockNum, -1, HeadBlockNum, NFTBlocksSeen);
+	STARSBitSet CommonBlocksSeen;
+	CommonBlocksSeen.AllocateBits(this->GetNumBlocks());
+	CommonBlocksSeen = STARSBitSetIntersection(FTBlocksSeen, NFTBlocksSeen);
+	size_t CommonBlocksCount = CommonBlocksSeen.CountSetBits();
+	int FirstCommonBlockNum = CommonBlocksSeen.FindLowestBitSet();
+	int LastFTReachableBlockNum = FTBlocksSeen.FindHighestBitSet();
+	int LastNFTReachableBlockNum = NFTBlocksSeen.FindHighestBitSet();
+	bool IfThenElseCase = true;
+	bool FTNotDominated = (!this->DoesBlockDominateBlock(HeadBlockNum, FTBlockNum));
+	bool NFTNotDominated = (!this->DoesBlockDominateBlock(HeadBlockNum, NFTBlockNum));
+	bool DominanceProblem = (FTNotDominated || NFTNotDominated);
+	bool Case0 = false;
+
+	// Based on CommonBlocksCount, FirstCommonBlockNum, FTBlockNum, and NFTBlockNum, we
+	//  can find which case in the comment above applies.
+	if (0 == CommonBlocksCount) { // Case 1 or Case 0.
+		FollowBlockNum = SMP_BLOCKNUM_COMMON_RETURN;
+		// Detect the exception subcase.
+		SMPBasicBlock *LastFTBlockReached = this->GetBlockByNum((size_t)LastFTReachableBlockNum);
+		SMPBasicBlock *LastNFTBlockReached = this->GetBlockByNum((size_t)LastNFTReachableBlockNum);
+		if ((1 == LastFTBlockReached->GetNumSuccessors()) && (1 == LastNFTBlockReached->GetNumSuccessors())) {
+			int NextFTBlockNum = (*(LastFTBlockReached->GetFirstConstSucc()))->GetNumber();
+			int NextNFTBlockNum = (*(LastNFTBlockReached->GetFirstConstSucc()))->GetNumber();
+			bool NotLoopBack = (NextFTBlockNum > HeadBlockNum);
+			if ((NextFTBlockNum == NextNFTBlockNum) && NotLoopBack) { // Exception to Case 1
+				FollowBlockNum = NextFTBlockNum;
+			}
+		}
+		if (FollowBlockNum == SMP_BLOCKNUM_COMMON_RETURN) { // Not the exception to Case 1; search for Case 0.
+			if (HeadBlockIsInALoop && DominanceProblem) { // Maybe one COND_BRANCH successor is our own loop head.
+				SMPBasicBlock *NFTBlock = this->GetBlockByNum((size_t)NFTBlockNum);
+				SMPBasicBlock *FTBlock = this->GetBlockByNum((size_t)FTBlockNum);
+				if (NFTBlock->IsLoopHeaderBlock()) {
+					int NFTLoopNum = this->GetLoopNumFromHeaderBlockNum(NFTBlockNum);
+					Case0 = (NFTLoopNum == InnerMostLoopNum);
+				}
+				if (!Case0 && FTBlock->IsLoopHeaderBlock()) {
+					int FTLoopNum = this->GetLoopNumFromHeaderBlockNum(FTBlockNum);
+					Case0 = (FTLoopNum == InnerMostLoopNum);
+				}
+				// Case0 ==> SMP_BLOCKNUM_COMMON_RETURN for FollowBlock; use boolean flag below to avoid
+				//  falsely declaring a CFG to be unstructured due to the DominanceProblem.
+			}
+		}
+	}
+	else if (FirstCommonBlockNum == NFTBlockNum) { // Case 2
+		FollowBlockNum = FirstCommonBlockNum;
+		IfThenElseCase = false;
+	}
+	else if (FirstCommonBlockNum == FTBlockNum) { // Case 3; OddIfThenCase
+		FollowBlockNum = FirstCommonBlockNum;
+		IfThenElseCase = false;
+	}
+	else { // if-then-else with at least one intersection point
+		if (this->DoesBlockDominateBlock(HeadBlockNum, FirstCommonBlockNum)) { // Case 4
+			uint32_t TailBlocksSeen = 0;
+			if (HeadBlockIsInALoop) {
+				int FirstFTReachableBlockNum = FTBlocksSeen.FindLowestBitSet();
+				int FirstNFTReachableBlockNum = NFTBlocksSeen.FindLowestBitSet();
+				for (int BlockIndex = FirstFTReachableBlockNum; BlockIndex <= LastFTReachableBlockNum; ++BlockIndex) {
+					if (FTBlocksSeen.GetBit((size_t)BlockIndex)) {
+						if (this->GetBlockByNum((size_t)BlockIndex)->IsLoopTailBlock()) {
+							++TailBlocksSeen;
+						}
+					}
+				}
+				for (int BlockIndex = FirstNFTReachableBlockNum; BlockIndex <= LastNFTReachableBlockNum; ++BlockIndex) {
+					if (NFTBlocksSeen.GetBit((size_t)BlockIndex)) {
+						if (this->GetBlockByNum((size_t)BlockIndex)->IsLoopTailBlock()) {
+							++TailBlocksSeen;
+						}
+					}
+				}
+			}
+			if (2 <= TailBlocksSeen) { // Case 4C
+				FollowBlockNum = SMP_BLOCKNUM_COMMON_RETURN;
+			}
+			else if (LastFTReachableBlockNum != LastNFTReachableBlockNum) { // Case 4B
+				FollowBlockNum = SMP_BLOCKNUM_UNINIT;
+			}
+			else {
+				bool NotCommonBlockFound = false;
+				for (int BlockIndex = FirstCommonBlockNum; BlockIndex < LastFTReachableBlockNum; ++BlockIndex) {
+					if (FTBlocksSeen.GetBit((size_t)BlockIndex) != (NFTBlocksSeen.GetBit((size_t)BlockIndex))) {
+						NotCommonBlockFound = true;
+						break;
+					}
+				}
+				if (NotCommonBlockFound) { // Case 4B
+					FollowBlockNum = SMP_BLOCKNUM_UNINIT;
+				}
+				else { // Case 4A
+					FollowBlockNum = FirstCommonBlockNum;
+				}
+			}
+		}
+		else { // Case 5
+			FollowBlockNum = SMP_BLOCKNUM_UNINIT;
+		}
+	}
+	// Detect unstructured entry points.
+	if (IfThenElseCase) {
+		// Both sides of the conditional should be dominated by HeadBlock.
+		if (DominanceProblem && (!Case0)) {
+			// Some path is entering one side of our conditional at the FTBlock or at the NFTBlock.
+			FollowBlockNum = SMP_BLOCKNUM_UNINIT;
+		}
+		if (0 <= FollowBlockNum) {
+			if (this->FindUnstructuredIfThenElse(HeadBlockNum, FTBlockNum, NFTBlockNum, FollowBlockNum)) {
+				FollowBlockNum = SMP_BLOCKNUM_UNINIT; // error signal
+			}
+		}
+	}
+
+	return FollowBlockNum;
+} // end of SMPFunction::FindConditionalFollowNode3()
+
 
 // Track CurrBlockNum until we reach block that BranchHeadBlockNum doesn't dominate, or dead end. Return block num, possibly SMP_BLOCKNUM_COMMON_RETURN.
 //  A dead end is a block that returns or that calls a non-returning function.
@@ -12567,78 +12855,78 @@ int SMPFunction::TrackConditionalBranchTerminus(int BranchHeadBlockNum, int Curr
 		} // end while work list is not empty
 	}
 
+	// Corner case: Escaped domination by head block or escaped loop, and hit return block.
+	if (0 <= TerminusBlockNum) {
+		SMPBasicBlock *TerminusBlock = this->GetBlockByNum((size_t)TerminusBlockNum);
+		if (TerminusBlock->HasReturn() || TerminusBlock->HasNonReturningCall()) {
+			TerminusBlockNum = SMP_BLOCKNUM_COMMON_RETURN;
+		}
+	}
+
 	return TerminusBlockNum;
 } // end of SMPFunction::TrackConditionalBranchTerminus()
 
 // Do IF and ELSE branches merge before FollowBlockNum?
-bool SMPFunction::FindUnstructuredIfThenElse(const int HeadBlockNum, const int FollowBlockNum) const {
+bool SMPFunction::FindUnstructuredIfThenElse(const int HeadBlockNum, const int ThenBlockNum, const int ElseBlockNum, const int FollowBlockNum) const {
 	bool UnstructuredClauses = false;
 
-	STARSBitSet ThenBlocksSeen, ElseBlocksSeen;
-	int BlockAlreadySeenCounter = 0;
-	SMPBasicBlock *HeadBlock = this->GetBlockByNum((size_t) HeadBlockNum);
-	ThenBlocksSeen.AllocateBits(this->GetNumBlocks());
-	ElseBlocksSeen.AllocateBits(this->GetNumBlocks());
-
-	// Arbitrarily call the first non-back-edge successor the THEN clause, second one the ELSE clause.
-	int ElseBlockNum = HeadBlock->GetCondNonFallThroughSuccBlockNum();
-	assert(SMP_BLOCKNUM_UNINIT != ElseBlockNum);
-	list<SMPBasicBlock *>::const_iterator ThenIter = HeadBlock->GetCondOtherSucc(ElseBlockNum);
-	assert(ThenIter != HeadBlock->GetLastConstSucc());
-	SMPBasicBlock *ThenBlock = (*ThenIter);
-
-	// Follow no back edges, start at ThenBlock, stop at FollowBlockNum, record blocks seen.
-	list<int> WorkList;
-	int ThenBlockNum = ThenBlock->GetNumber();
-	ThenBlocksSeen.SetBit((size_t)ThenBlockNum);
-	WorkList.push_back(ThenBlockNum);
-	while (!WorkList.empty()) { // RPO ordering, no back edges, ensures termination
-		int CurrBlockNum = WorkList.front();
-		WorkList.pop_front();
-		SMPBasicBlock *CurrBlock = this->GetBlockByNum(CurrBlockNum);
-		for (list<SMPBasicBlock *>::const_iterator SuccIter = CurrBlock->GetFirstConstSucc(); SuccIter != CurrBlock->GetLastConstSucc(); ++SuccIter) {
-			int SuccBlockNum = (*SuccIter)->GetNumber();
-			if (!ThenBlocksSeen.GetBit((size_t)SuccBlockNum)) { // don't follow an unstructured loop
-				if (!this->DoesBlockDominateBlock(SuccBlockNum, CurrBlockNum)) { // no back edge
-					if (SuccBlockNum != FollowBlockNum) {
-						ThenBlocksSeen.SetBit((size_t)SuccBlockNum);
-						WorkList.push_back(SuccBlockNum);
+	if ((SMP_BLOCKNUM_UNINIT != ElseBlockNum) && (SMP_BLOCKNUM_UNINIT != ThenBlockNum)) {
+		STARSBitSet ThenBlocksSeen, ElseBlocksSeen;
+		int BlockAlreadySeenCounter = 0;
+		ThenBlocksSeen.AllocateBits(this->GetNumBlocks());
+		ElseBlocksSeen.AllocateBits(this->GetNumBlocks());
+
+		// Follow no back edges, start at ThenBlock, stop at FollowBlockNum, record blocks seen.
+		list<int> WorkList;
+		ThenBlocksSeen.SetBit((size_t)ThenBlockNum);
+		WorkList.push_back(ThenBlockNum);
+		while (!WorkList.empty()) { // RPO ordering, no back edges, ensures termination
+			int CurrBlockNum = WorkList.front();
+			WorkList.pop_front();
+			SMPBasicBlock *CurrBlock = this->GetBlockByNum(CurrBlockNum);
+			for (list<SMPBasicBlock *>::const_iterator SuccIter = CurrBlock->GetFirstConstSucc(); SuccIter != CurrBlock->GetLastConstSucc(); ++SuccIter) {
+				int SuccBlockNum = (*SuccIter)->GetNumber();
+				if (!ThenBlocksSeen.GetBit((size_t)SuccBlockNum)) { // don't follow an unstructured loop
+					if (!this->DoesBlockDominateBlock(SuccBlockNum, CurrBlockNum)) { // no back edge
+						if (SuccBlockNum != FollowBlockNum) {
+							ThenBlocksSeen.SetBit((size_t)SuccBlockNum);
+							WorkList.push_back(SuccBlockNum);
+						}
 					}
 				}
 			}
-		}
-	} // end while WorkList is not empty
+		} // end while WorkList is not empty
 
-	WorkList.push_back(ElseBlockNum);
-	ElseBlocksSeen.SetBit((size_t)ElseBlockNum);
-	while (!WorkList.empty()) { // RPO ordering, no back edges, ensures termination
-		int CurrBlockNum = WorkList.front();
-		WorkList.pop_front();
-		SMPBasicBlock *CurrBlock = this->GetBlockByNum(CurrBlockNum);
-		for (list<SMPBasicBlock *>::const_iterator SuccIter = CurrBlock->GetFirstConstSucc(); SuccIter != CurrBlock->GetLastConstSucc(); ++SuccIter) {
-			int SuccBlockNum = (*SuccIter)->GetNumber();
-			if (!ElseBlocksSeen.GetBit((size_t)SuccBlockNum)) { // don't follow an unstructured loop
-				if (!this->DoesBlockDominateBlock(SuccBlockNum, CurrBlockNum)) { // no back edge
-					if (SuccBlockNum != FollowBlockNum) {
-						ElseBlocksSeen.SetBit((size_t)SuccBlockNum);
-						WorkList.push_back(SuccBlockNum);
+		WorkList.push_back(ElseBlockNum);
+		ElseBlocksSeen.SetBit((size_t)ElseBlockNum);
+		while (!WorkList.empty()) { // RPO ordering, no back edges, ensures termination
+			int CurrBlockNum = WorkList.front();
+			WorkList.pop_front();
+			SMPBasicBlock *CurrBlock = this->GetBlockByNum(CurrBlockNum);
+			for (list<SMPBasicBlock *>::const_iterator SuccIter = CurrBlock->GetFirstConstSucc(); SuccIter != CurrBlock->GetLastConstSucc(); ++SuccIter) {
+				int SuccBlockNum = (*SuccIter)->GetNumber();
+				if (!ElseBlocksSeen.GetBit((size_t)SuccBlockNum)) { // don't follow an unstructured loop
+					if (!this->DoesBlockDominateBlock(SuccBlockNum, CurrBlockNum)) { // no back edge
+						if (SuccBlockNum != FollowBlockNum) {
+							ElseBlocksSeen.SetBit((size_t)SuccBlockNum);
+							WorkList.push_back(SuccBlockNum);
+						}
 					}
 				}
 			}
-		}
-	} // end while WorkList is not empty
+		} // end while WorkList is not empty
 
-	// A structured if-then-else should have no blocks reached from both THEN and ELSE
-	//  clauses, except the FollowBlock, which we avoided marking.
-	for (size_t BitIndex = 0; BitIndex < ThenBlocksSeen.GetNumBits(); ++BitIndex) {
-		if (ThenBlocksSeen.GetBit(BitIndex) && ElseBlocksSeen.GetBit(BitIndex)) {
-			UnstructuredClauses = true;
-			// ERROR message will be output later, so don't cause redundant statistics to be collected. Make it INFO.
-			SMP_msg("INFO: SPARK: Unstructured due to THEN and ELSE clauses merge prematurely at block %zu for HeadBlock %d FollowBlock %d ",
-				BitIndex, HeadBlockNum, FollowBlockNum);
-			this->DumpFuncNameAndAddr();
-			this->DumpDotCFG();
-			break;
+		// A structured if-then-else should have no blocks reached from both THEN and ELSE
+		//  clauses, except the FollowBlock, which we avoided marking.
+		for (size_t BitIndex = 0; BitIndex < ThenBlocksSeen.GetNumBits(); ++BitIndex) {
+			if (ThenBlocksSeen.GetBit(BitIndex) && ElseBlocksSeen.GetBit(BitIndex)) {
+				UnstructuredClauses = true;
+				// ERROR message will be output later, so don't cause redundant statistics to be collected. Make it INFO.
+				SMP_msg("INFO: SPARK: Unstructured due to THEN and ELSE clauses merge prematurely at block %zu for HeadBlock %d FollowBlock %d ",
+					BitIndex, HeadBlockNum, FollowBlockNum);
+				this->DumpFuncNameAndAddr();
+				break;
+			}
 		}
 	}
 
@@ -14556,7 +14844,7 @@ bool SMPFunction::AreBlocksInSameLoops(const int BlockNum1, const int BlockNum2)
 // Does going from BlockNum1 to BlockNum2 exit a loop?
 bool SMPFunction::DoesEdgeExitLoop(const int BlockNum1, const int BlockNum2) const {
 	bool ExitsLoop = false;
-	if (0 == this->GetNumLoops())
+	if ((0 == this->GetNumLoops()) || (0 > BlockNum2)) // BlockNum2 might be code like SMP_BLOCKNUM_COMMON_RETURN
 		return ExitsLoop;
 	assert(((size_t)BlockNum1) < this->GetNumBlocks());
 	assert(((size_t)BlockNum2) < this->GetNumBlocks());
@@ -19316,35 +19604,38 @@ void SMPFunction::Dump(void) {
 } // end of SMPFunction::Dump()
 
 // Dump CFG representation for processing by "dot" program.
-void SMPFunction::DumpDotCFG(void) const {
-	string DotFileName(global_STARS_program->GetRootFileName());
-	DotFileName += ".STARS.";
-	char FuncBuf[20] = { '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0' };
-	// itoa(this->GetFirstFuncAddr(), FuncBuf, 16);
-	SMP_snprintf(FuncBuf, 18, "%x", this->GetFirstFuncAddr());
-	string FuncName("func");
-	FuncName += string(FuncBuf);
-	string FileExtension(".gv");
-	DotFileName += FuncName;
-	DotFileName += FileExtension;
-	FILE *DotFile = SMP_fopen(DotFileName.c_str(), "w");
-	if (nullptr == DotFile) {
-		SMP_msg("ERROR: Cannot open dot file %s for CFG dump.\n", DotFileName.c_str());
-		return;
-	}
+void SMPFunction::DumpDotCFG(void) {
+	if (!this->PrintedCFG) { // avoid redundant work
+		this->PrintedCFG = true;
+		string DotFileName(global_STARS_program->GetRootFileName());
+		DotFileName += ".STARS.";
+		char FuncBuf[20] = { '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0' };
+		// itoa(this->GetFirstFuncAddr(), FuncBuf, 16);
+		SMP_snprintf(FuncBuf, 18, "%x", this->GetFirstFuncAddr());
+		string FuncName("func");
+		FuncName += string(FuncBuf);
+		string FileExtension(".gv");
+		DotFileName += FuncName;
+		DotFileName += FileExtension;
+		FILE *DotFile = SMP_fopen(DotFileName.c_str(), "w");
+		if (nullptr == DotFile) {
+			SMP_msg("ERROR: Cannot open dot file %s for CFG dump.\n", DotFileName.c_str());
+			return;
+		}
 
-	SMP_fprintf(DotFile, "digraph %s {\n", FuncName.c_str());
+		SMP_fprintf(DotFile, "digraph %s {\n", FuncName.c_str());
 
-	for (SMPBasicBlock *CurrBlock : this->RPOBlocks) {
-		if (CurrBlock->GetNumSuccessors() > 0) {
-			int CurrBlockNum = CurrBlock->GetNumber();
-			for (std::list<SMPBasicBlock *>::const_iterator CurrLink = CurrBlock->GetFirstConstSucc(); CurrLink != CurrBlock->GetLastConstSucc(); ++CurrLink) {
-				SMP_fprintf(DotFile, "  %d -> %d\n", CurrBlockNum, (*CurrLink)->GetNumber());
+		for (SMPBasicBlock *CurrBlock : this->RPOBlocks) {
+			if (CurrBlock->GetNumSuccessors() > 0) {
+				int CurrBlockNum = CurrBlock->GetNumber();
+				for (std::list<SMPBasicBlock *>::const_iterator CurrLink = CurrBlock->GetFirstConstSucc(); CurrLink != CurrBlock->GetLastConstSucc(); ++CurrLink) {
+					SMP_fprintf(DotFile, "  %d -> %d\n", CurrBlockNum, (*CurrLink)->GetNumber());
+				}
 			}
 		}
+		SMP_fprintf(DotFile, "}\n");
+		(void)SMP_fclose(DotFile);
 	}
-	SMP_fprintf(DotFile, "}\n");
-	(void) SMP_fclose(DotFile);
 
 	return;
 } // end of SMPFunction::DumpDotCFG()
@@ -22820,7 +23111,9 @@ void SMPFunction::EmitSPARKLoopProcGlobals(FILE *BodyFile, FILE *HeaderFile, boo
 // recursive descent translation to SPARK Ada starting with CurrBlock, stop before Follow Block or limit to blocks with the same LoopExitTargets reachability.
 void SMPFunction::EmitSPARKAdaForBlock(int CurrBlockNum, int FollowBlockNum, FILE *SPARKBodyFile, bool ReadytoEmitSwitchDefault, bool LoopToProc, const uint32_t ReachableSourcesCount) {
 	PrintSPARKIndentTabs(SPARKBodyFile);
+	int InnerLoopNum = this->GetInnermostLoopNum(CurrBlockNum);
 	SMP_fprintf(SPARKBodyFile, "-- Basic block %d;\n", CurrBlockNum);
+	bool InsideLoop = (0 <= InnerLoopNum);
 	if (LoopToProc && (!this->IsSPARKLoopInTranslationStack())) {
 		// See if we are beginning to translate a guarded loop.
 		SMPBasicBlock *CurrBlock = this->GetBlockByNum(CurrBlockNum);
@@ -22843,9 +23136,9 @@ void SMPFunction::EmitSPARKAdaForBlock(int CurrBlockNum, int FollowBlockNum, FIL
 	// Recursive descent based on the kind of block CurrBlock is.
 	while ((CurrBlockNum != FollowBlockNum) && (CurrBlockNum >= 0)) {
 		if (0 < ReachableSourcesCount) {
-			int InnerLoopNum = this->GetInnermostDominatingLoopNum(CurrBlockNum);
-			if (0 <= InnerLoopNum) {
-				uint32_t CurrReachableSourcesCount = this->GetReachableSourcesCount((size_t)CurrBlockNum, InnerLoopNum);
+			int InnerDominatingLoopNum = this->GetInnermostDominatingLoopNum(CurrBlockNum);
+			if (0 <= InnerDominatingLoopNum) {
+				uint32_t CurrReachableSourcesCount = this->GetReachableSourcesCount((size_t)CurrBlockNum, InnerDominatingLoopNum);
 				if (CurrReachableSourcesCount != ReachableSourcesCount) // cannot follow beyond scope of current guard Boolean
 					break;
 			}
@@ -23040,7 +23333,10 @@ void SMPFunction::EmitSPARKAdaForBlock(int CurrBlockNum, int FollowBlockNum, FIL
 						// Translate if-else structure previously identified
 						int NextFollowBlockNum = this->FindFollowBlockNum(CurrBlock, true);
 						this->EmitSPARKAdaForConditional(CurrBlockNum, NextFollowBlockNum, SPARKBodyFile);
-						ResumeBlockNum = NextFollowBlockNum;
+						bool CrossingLoopBoundary = (InsideLoop && (0 < NextFollowBlockNum) && this->DoesEdgeExitLoop(CurrBlockNum, NextFollowBlockNum));
+						if (!CrossingLoopBoundary) {
+							ResumeBlockNum = NextFollowBlockNum;
+						}
 					}
 				}
 				else if (FlowType == JUMP) {
@@ -23408,7 +23704,7 @@ void SMPFunction::EmitSPARKAdaForMultipleLoopExitTargets(FILE *SPARKBodyFile, si
 	STARSBitSet ReachableUnion;
 	ReachableUnion.AllocateBits(this->GetNumBlocks());
 	bool Initialized = false;
-	assert(!LoopExitTargetsReachabilitySets[LoopNum].empty());
+	assert(!this->LoopExitTargetsReachabilitySets[LoopNum].empty());
 	for (int ExitTargetBlockNum : this->LoopExitTargets[LoopNum]) {
 		if (!Initialized) {
 			ReachableUnion = this->LoopExitTargetsReachabilitySets[LoopNum].at(ExitTargetBlockNum);
@@ -23696,8 +23992,8 @@ void SMPFunction::EmitSPARKAdaForConditional(int HeaderBlockNum, int FollowBlock
 	//  For the if-then-else case, we emit the code for the then-block and then "else" etc.
 	ControlFlowType LastCFType = this->GetControlFlowType(LastAddr);
 	bool IsLoopBack = ((LastCFType == LOOP_BACK) || (LastCFType == INVERTED_LOOP_BACK));
-	bool StaysInLoop = this->IsNonExitingLoopBackBranch(LastAddr);
-	bool IfThenCase = ((LastCFType == BRANCH_IF_THEN) || (StaysInLoop && (IsLoopBack || (LastCFType == LOOP_EXIT))));
+	bool LoopBackStaysInLoop = this->IsNonExitingLoopBackBranch(LastAddr);
+	bool IfThenCase = ((LastCFType == BRANCH_IF_THEN) || (LoopBackStaysInLoop && (IsLoopBack || (LastCFType == LOOP_EXIT))));
 	bool ShortCircuitCase = (LastCFType == SHORT_CIRCUIT_BRANCH);
 	assert(IfThenCase || ShortCircuitCase || (LastCFType == BRANCH_IF_THEN_ELSE));
 	int FallThroughBlockNum = SMP_BLOCKNUM_UNINIT;
@@ -23749,7 +24045,7 @@ void SMPFunction::EmitSPARKAdaForConditional(int HeaderBlockNum, int FollowBlock
 		this->SPARKControlStack.push_back(SPARK_THEN_CLAUSE);
 		bool OddIfThenCase = LastInst->IsOddIfThenCase();
 		if (!OddIfThenCase) { // normal case
-			assert(StaysInLoop || (DistantBlockNum == FollowBlockNum)); // non-fall-through block is normal follow block for if-then
+			assert(LoopBackStaysInLoop || (DistantBlockNum == FollowBlockNum)); // non-fall-through block is normal follow block for if-then
 			// Avoid generating code outside the loop from THEN clause if unknown follow block.
 			if (InsideLoop && UnknownFollowBlock && this->DoesEdgeExitLoop(HeaderBlockNum, FallThroughBlockNum)) {
 				PrintSPARKIndentTabs(SPARKBodyFile);
diff --git a/src/base/SMPInstr.cpp b/src/base/SMPInstr.cpp
index 8ed5abf7..fa3f2602 100644
--- a/src/base/SMPInstr.cpp
+++ b/src/base/SMPInstr.cpp
@@ -2991,14 +2991,21 @@ void STARSExpression::EmitSPARKAdaString(std::string &OutString, bool Processing
 		STARSOpndTypePtr LeftOp = CloneIfNecessary(this->GetConstLeftOperand(), UseFP);
 		if (MDIsDirectStackAccessOpnd(LeftOp, UseFP)) {
 			// Crash observed using ParentInst to unnormalize the stack op.
-			//  Problem was [rdx+rdx] expression, where RCX traced back to before
+			//  Problem was [rdx+rcx] expression, where RCX traced back to before
 			//  stack allocation instruction, while RDX traced back to after 
 			//  the stack allocation instruction. Unnormalizing RDX's [RSP+40]
 			//  expression at the RCX init point produced stack location above the frame.
 #if 0
 			this->GetParentInst()->MDGetUnnormalizedOp(LeftOp);
+#else
+#if 1
+			STARS_ea_t LeftUseAddr = this->GetLeftUseAddr();
+			assert(STARS_BADADDR != LeftUseAddr);
+			SMPInstr *UseInst = this->GetParentFunc()->GetInstFromAddr(LeftUseAddr);
+			UseInst->MDGetUnnormalizedOp(LeftOp);
 #else
 			this->GetOriginalParentInst()->MDGetUnnormalizedOp(LeftOp);
+#endif
 #endif
 		}
 
@@ -3045,7 +3052,18 @@ void STARSExpression::EmitSPARKAdaString(std::string &OutString, bool Processing
 			bool OmitTrailingSpace = IsRegOp && (ProcessingLoop || OldSuffix);
 			STARSOpndTypePtr RightOp = CloneIfNecessary(this->GetConstRightOperand(), UseFP);
 			if (MDIsDirectStackAccessOpnd(RightOp, UseFP)) {
+#if 1
+				STARS_ea_t RightUseAddr = this->GetRightUseAddr();
+				assert(STARS_BADADDR != RightUseAddr);
+				SMPInstr *UseInst = this->GetParentFunc()->GetInstFromAddr(RightUseAddr);
+				UseInst->MDGetUnnormalizedOp(RightOp);
+#else
+#if 0
 				this->GetParentInst()->MDGetUnnormalizedOp(RightOp);
+#else
+				this->GetOriginalParentInst()->MDGetUnnormalizedOp(RightOp);
+#endif
+#endif
 			}
 			bool InArgOp = false;
 			if (IsRegOp) {
@@ -9201,38 +9219,61 @@ bool SMPInstr::IsOddIfThenCase(void) const {
 	//  FallThroughBlock has only one predecessor (the COND_BRANCH block falls through to it). We can use
 	//  the number of predecessor blocks to distinguish the two cases, with the additional limitation
 	//  that one predecessor reaches the fall-through block via unconditional direct jump.
+	// It is also true that in the OddIfThenCase, the NonFallThroughBlock (ThenBlock) has only one predecessor. We
+	//  can test for this fact first to prevent any complications.
+	//  NOTE: The unconditional direct jump to the fall-through block is not necessary if L1 and L2 mark
+	//  blocks with return statements or calls to non-returning functions, because then there will be no
+	//  merging back to an "end if;" point.
 	bool OddIfThenCase = false;
 	SMPBasicBlock *CurrBlock = this->GetBlock();
 	STARS_ea_t InstAddr = this->GetAddr();
 	ControlFlowType FuncControlFlowType = CurrBlock->GetFunc()->GetControlFlowType(InstAddr);
 	if (BRANCH_IF_THEN == FuncControlFlowType) {
-		list<SMPBasicBlock *>::const_iterator SuccIter = CurrBlock->GetFallThroughSucc();
+		list<SMPBasicBlock *>::const_iterator SuccIter = CurrBlock->GetCondNonFallThroughSucc();
 		assert(SuccIter != CurrBlock->GetLastConstSucc());
-		size_t PredCount = (*SuccIter)->GetNumPredsMinusBackEdges();
-		// Don't let loop-back edges increase the PredCount, e.g.:
-		//   if cond then
-		//      loop
-		//         [loop body]
-		//      end loop;
-		//      ...
-		//   end if;
-		// The normal if-then case will fall through to a loop header block, and branch to the follow block
-		//  for the if-then. That is not the odd case.
-
-		OddIfThenCase = (1 < PredCount); // fall-through block has more than 1 predecessor
-#if 1
-		// See if we have an unconditional jump to the fall-through block.
-		if (OddIfThenCase) {
-			OddIfThenCase = false;
-			for (list<SMPBasicBlock *>::const_iterator PredIter = (*SuccIter)->GetFirstConstPred(); PredIter != (*SuccIter)->GetLastConstPred(); ++PredIter) {
-				if ((*PredIter)->HasDirectJump()) {
-					OddIfThenCase = true;
-					break;
+		SMPBasicBlock *NFTBlock = (*SuccIter);
+		size_t NFTPredCount = NFTBlock->GetNumPredsMinusBackEdges();
+		if (1 == NFTPredCount) { // might be OddIfThenCase
+			SuccIter = CurrBlock->GetFallThroughSucc();
+			assert(SuccIter != CurrBlock->GetLastConstSucc());
+			SMPBasicBlock *FTBlock = (*SuccIter);
+			size_t FTPredCount = FTBlock->GetNumPredsMinusBackEdges();
+			// Don't let loop-back edges increase the PredCount, e.g.:
+			//   if cond then
+			//      loop
+			//         [loop body]
+			//      end loop;
+			//      ...
+			//   end if;
+			// The normal if-then case will fall through to a loop header block, and branch to the follow block
+			//  for the if-then. That is not the odd case.
+
+			OddIfThenCase = (1 < FTPredCount); // fall-through block has more than 1 predecessor
+	#if 1
+			// See if we have an unconditional jump to the fall-through block.
+			if (OddIfThenCase) {
+				bool TerminalFTBlock = (FTBlock->GetNumSuccessors() == 0);
+				bool TerminalNFTBlock = (NFTBlock->GetNumSuccessors() == 0);
+
+				if (TerminalFTBlock && TerminalNFTBlock) {
+					// Should we only check TerminalNFTBlock? Or should we trace NFTBlock to a return-block terminus
+					//  and then we know it will not rejoin the FTBlock? !!!!****!!!!
+					SMP_msg("INFO: OddIfThenCase holds because of common_return termini at %llx ", (uint64_t)this->GetAddr());
+					this->GetBlock()->GetFunc()->DumpFuncNameAndAddr();
+				}
+				else {
+					OddIfThenCase = false;
+					for (list<SMPBasicBlock *>::const_iterator PredIter = (*SuccIter)->GetFirstConstPred(); PredIter != (*SuccIter)->GetLastConstPred(); ++PredIter) {
+						if ((*PredIter)->HasDirectJump()) {
+							OddIfThenCase = true;
+							break;
+						}
+					}
+					if (!OddIfThenCase) {
+						SMP_msg("INFO: SMPInstr::IsOddIfThenCase() false due to second criterion at %llx ", (uint64_t)InstAddr);
+						this->GetBlock()->GetFunc()->DumpFuncNameAndAddr();
+					}
 				}
-			}
-			if (!OddIfThenCase) {
-				SMP_msg("INFO: SMPInstr::IsOddIfThenCase() false due to second criterion at %llx in func %s\n",
-					(uint64_t) InstAddr, this->GetBlock()->GetFunc()->GetFuncName());
 			}
 		}
 #endif
diff --git a/src/base/SMPProgram.cpp b/src/base/SMPProgram.cpp
index 43c9d2b6..b280b464 100644
--- a/src/base/SMPProgram.cpp
+++ b/src/base/SMPProgram.cpp
@@ -1147,7 +1147,7 @@ bool SMPProgram::EmitProgramSPARKAda(void) {
 	for (FuncIter = this->FuncMap.begin(); FuncIter != this->FuncMap.end(); ++FuncIter) {
 		SMPFunction *TempFunc = FuncIter->second;
 		if (TempFunc == nullptr) continue;
-		bool FuncFound = (0x4018c0 == TempFunc->GetFirstFuncAddr());
+		bool FuncFound = (0x444492 == TempFunc->GetFirstFuncAddr());
 		if (FuncFound) {
 			TempFunc->Dump();
 			TempFunc->DumpDotCFG();
@@ -1191,8 +1191,8 @@ bool SMPProgram::EmitProgramSPARKAda(void) {
 					bool EdgeSuccess = TempFunc->ComputeEdgeExpressions();
 					if (global_stars_interface->VerboseSPARKMode()) {
 						TempFunc->Dump();
-						TempFunc->DumpDotCFG();
 					}
+					TempFunc->DumpDotCFG();
 				}
 			}
 		}
-- 
GitLab