From 2a65457e19f1abb2aa8bf746c2ed64c108afe9b5 Mon Sep 17 00:00:00 2001
From: Andy McFadden
Date: Fri, 24 Jul 2020 21:34:36 -0700
Subject: [PATCH] Default "smart PLP handling" to off
The feature is mildly broken and, frankly, unreliable on its best
day. Default it to off for new projects.
---
SourceGen/CodeAnalysis.cs | 32 ++++++++++++++----------
SourceGen/ProjectProperties.cs | 2 +-
SourceGen/RuntimeData/Help/settings.html | 4 ++-
3 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/SourceGen/CodeAnalysis.cs b/SourceGen/CodeAnalysis.cs
index a7bc721..13ee67a 100644
--- a/SourceGen/CodeAnalysis.cs
+++ b/SourceGen/CodeAnalysis.cs
@@ -799,24 +799,30 @@ namespace SourceGen {
///
/// Attempts to guess what the flags will be after a PLP instruction.
///
+ ///
+ /// We're not tracking stack contents or register contents, so this just
+ /// generally won't work. However, there's a lot of code that uses PHP to
+ /// save the current state and PLP to restore it, so if we can find a nearby
+ /// PHP we can just grab from that.
+ ///
+ /// Failing that, we mark all flags as "indeterminate" and let the user sort
+ /// out what it should be. It's unlikely to matter except for M/X flags on
+ /// the 65816.
+ ///
+ /// The emulation flag is not part of the status register, even if we do carry
+ /// it around like one. The E-flag is always carried over from the previous
+ /// instruction.
+ ///
/// Offset of PLP instruction.
/// Best guess at status flags.
private StatusFlags GuessFlagsForPLP(int plpOffset) {
- // We're not tracking stack contents or register contents, so this just
- // generally won't work. However, there's a lot of code that uses PHP to
- // save the current state and PLP to restore it, so if we can find a nearby
- // PHP we can just grab from that.
- //
- // Failing that, we mark all flags as "indeterminate" and let the user sort
- // out what it should be. It's unlikely to matter except for M/X flags on
- // the 65816.
- //
- // The emulation flag is not part of the status register, even if we do carry
- // it around like one. The E-flag is always carried over from the previous
- // instruction.
-
StatusFlags flags = StatusFlags.AllIndeterminate;
if (mAnalysisParameters.SmartPlpHandling) {
+ // TODO: this is broken. In some cases we end up latching the result from the
+ // first visit only. When the PHP instruction gets updated, the subsequent
+ // instructions are only re-evaluated if the flags have changed. If we reach
+ // an instruction where the flags match, we stop looking forward, and might
+ // not re-visit the PLP.
int backOffsetLimit = plpOffset - 128; // arbitrary 128-byte reach
if (backOffsetLimit < 0) {
backOffsetLimit = 0;
diff --git a/SourceGen/ProjectProperties.cs b/SourceGen/ProjectProperties.cs
index 4d77412..d590072 100644
--- a/SourceGen/ProjectProperties.cs
+++ b/SourceGen/ProjectProperties.cs
@@ -63,7 +63,7 @@ namespace SourceGen {
MinCharsForString = DataAnalysis.DEFAULT_MIN_STRING_LENGTH;
SeekNearbyTargets = true;
UseRelocData = false;
- SmartPlpHandling = true;
+ SmartPlpHandling = false;
SmartPlbHandling = true;
}
public AnalysisParameters(AnalysisParameters src) {
diff --git a/SourceGen/RuntimeData/Help/settings.html b/SourceGen/RuntimeData/Help/settings.html
index 072244a..c5aa5c2 100644
--- a/SourceGen/RuntimeData/Help/settings.html
+++ b/SourceGen/RuntimeData/Help/settings.html
@@ -274,7 +274,9 @@ improve automatic operand formatting.
the processor status flags from a nearby PHP
when a
PLP
is encountered. If not enabled, all flags are set to
"indeterminate" following a PLP
, except for the M/X
-flags on the 65816, which are left unmodified.
+flags on the 65816, which are left unmodified. (In practice this
+approach doesn't seem to work all that well, so the setting is
+un-checked by default.)
If "smart PLB handling" is checked, the analyzer will watch for
code patterns like PLB
preceded by PHK
,
and generate appropriate Data Bank Register changes. If not enabled,