Skip to content

[AMDGPU] IGLP: Fix static variables (alternative to #137549)#185436

Draft
ro-i wants to merge 3 commits intomainfrom
users/ro-i/iglp-static-fix-2
Draft

[AMDGPU] IGLP: Fix static variables (alternative to #137549)#185436
ro-i wants to merge 3 commits intomainfrom
users/ro-i/iglp-static-fix-2

Conversation

@ro-i
Copy link
Contributor

@ro-i ro-i commented Mar 9, 2026

This is an alternative PR to #137549 that just fixes the correctness issue in the IGLP code for now, without caching.

Assisted-by: claude-4.6-opus-high

ro-i added 3 commits March 9, 2026 06:11
Replace global / class-level static variables with instance members and
guarantee thread-safety.
Assisted-by: claude-4.6-opus
// Compute the heuristics for the pipeline, returning whether or not the DAG
// is well formatted for the mutation
bool analyzeDAG(const SIInstrInfo *TII);
bool AnalysisResult;
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to move the member variable into a (nested) struct (similar to MFMAExpInterleaveCache from #137549 although I would not use this name here) and then change the type of this from bool to std::optional.

; GCN-NEXT: s_endpgm
entry:
%a = load i1, ptr %src, align 1
call void @llvm.amdgcn.iglp.opt(i32 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should have a similar test case for @llvm.amdgcn.iglp.opt(i32 2)

Copy link
Contributor

@jrbyrnes jrbyrnes left a comment

Choose a reason for hiding this comment

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

The PR should not change the semantics of the mutation / result in code changes for llvm.amdgcn.iglp.opt.exp.*.mir

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.

3 participants