Skip to content

Conversation

@ZenithalHourlyRate
Copy link
Collaborator

@ZenithalHourlyRate ZenithalHourlyRate commented May 16, 2025

NoiseAnalysis (and also other analysis) may want to get unique name of a Value, e.g. for symbolic way of analysis (see #1817)

I do not know if this way of providing name for analysis is favorable (so requesting opinions from @j2kun before more technical details). I used to have another NameAnalysis so that NoiseAnalysis could access NameLattice, turned out that is a duplicate of SelectVariableNames so I modified this analysis to have a dataflow framework wrapper.

Notice that other ways of solving it include

  • Pass SelectVariableName struct to every analysis, which is inconvenient if we get more analysis. Passing it to the DataFlowSolver might be more convenient.
  • Pass it in LocalParam, which is essentially constructed from the IR, which requires annotating the IR with HEIR generated name (quite strange)
solved now
It was me that used reference for destroyed struct.

Other problem involves loop-carried variable which has the same name but does not have the same analysis state, which we (and also MLIR) are quite unable to handle. One way is to force the invariant for these variables via bootstrapping, but then we need to think about what "reset the Symbolic State" means for bootstrapping inside a loop.

@ZenithalHourlyRate ZenithalHourlyRate marked this pull request as ready for review May 16, 2025 15:27
@ZenithalHourlyRate ZenithalHourlyRate force-pushed the select-variable-name-analysis branch from 75dba05 to 91cbd96 Compare May 16, 2025 15:44
Copy link
Collaborator

@j2kun j2kun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I think that because the variable name selection is purely syntactic (evidenced by a no-op transfer function), it doesn't need a dataflow analysis and should just be passed as an extra argument to the analysis constructor.


if (lhs.getName() == rhs.getName()) return lhs;

// If the names are different, we can just return the first one
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this line never actually executes, if the transfer function below is a no-op... Maybe it would be worth adding an assertion here? Otherwise, I would question the use of this analysis that trivially ignores half the information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants