Skip to content

Update tests to not use Br instruction#139

Merged
piotrAMD merged 1 commit intoGPUOpen-Drivers:devfrom
mariusz-sikora-at-amd:pub-test-update
Apr 1, 2026
Merged

Update tests to not use Br instruction#139
piotrAMD merged 1 commit intoGPUOpen-Drivers:devfrom
mariusz-sikora-at-amd:pub-test-update

Conversation

@mariusz-sikora-at-amd
Copy link
Copy Markdown
Contributor

@mariusz-sikora-at-amd mariusz-sikora-at-amd commented Apr 1, 2026

Upstream LLVM split Br into CondBr/UncondBr
llvm/llvm-project#184027

@mariusz-sikora-at-amd
Update tests to not use Br instruction
7364e0f 
Upstream LLVM split Br into CondBr/UncondBr
llvm/llvm-project#184027
mariusz-sikora-at-amd
mariusz-sikora-at-amd commented Apr 1, 2026
View reviewed changes
test/unit/interface/OpMapTests.cpp
OpMap<StringRef> map;

OpDescription retDesc = OpDescription::fromCoreOp(Instruction::Ret);
OpDescription brDesc = OpDescription::fromCoreOp(Instruction::Br);
Copy link
Copy Markdown
Contributor Author

@mariusz-sikora-at-amd mariusz-sikora-at-amd Apr 1, 2026

Choose a reason for hiding this comment

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

I think it is safe to change this test, the main purpose of this test is not to test Br instruction. Ret and Br are (were) two first Core LLVM instructions.

@dstutt
Copy link
Copy Markdown
Member

dstutt commented Apr 1, 2026

Is this safe to submit without guards for llvm main revision (I think yes?)

@mariusz-sikora-at-amd
Copy link
Copy Markdown
Contributor Author

mariusz-sikora-at-amd commented Apr 1, 2026

Is this safe to submit without guards for llvm main revision (I think yes?)

I think it is safe. This test was just taking two first instructions from LLVM Core list of instructions. It is not testing Br or Ret in any way. You can use any other instructions. I also removed magic numbers from other test.

@mariusz-sikora-at-amd
Copy link
Copy Markdown
Contributor Author

mariusz-sikora-at-amd commented Apr 1, 2026

Is this safe to submit without guards for llvm main revision (I think yes?)

We could use llvm main revision and test Br and UncondBr/CondBr but this is not the main point of this test, so I think it would be better just to change Br to something different.

@mariusz-sikora-at-amd
Copy link
Copy Markdown
Contributor Author

mariusz-sikora-at-amd commented Apr 1, 2026

I don't see merge button. Someone else needs to merge this PR.

@piotrAMD piotrAMD merged commit aed217e into GPUOpen-Drivers:dev Apr 1, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dstutt dstutt dstutt approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mariusz-sikora-at-amd @dstutt @piotrAMD