-
Notifications
You must be signed in to change notification settings - Fork 15.2k
AMDGPU: Correct inst size for av_mov_b32_imm_pseudo #154459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AMDGPU: Correct inst size for av_mov_b32_imm_pseudo #154459
Conversation
In the AGPR case this will be an 8 byte instruction, which is part of why this case is a pain to deal with in the first place.
|
@llvm/pr-subscribers-backend-amdgpu Author: Matt Arsenault (arsenm) ChangesIn the AGPR case this will be an 8 byte instruction, Full diff: https://github.com/llvm/llvm-project/pull/154459.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/SIInstructions.td b/llvm/lib/Target/AMDGPU/SIInstructions.td
index bd5dfa92a8e43..d3c15bd8f672a 100644
--- a/llvm/lib/Target/AMDGPU/SIInstructions.td
+++ b/llvm/lib/Target/AMDGPU/SIInstructions.td
@@ -155,7 +155,8 @@ def AV_MOV_B32_IMM_PSEUDO
let VOP3 = 1;
let isMoveImm = 1;
let SchedRW = [Write32Bit];
- let Size = 4;
+ let Size = 8;
+ let FixedSize = true;
let UseNamedOperandTable = 1;
}
|
| let SchedRW = [Write32Bit]; | ||
| let Size = 4; | ||
| let Size = 8; | ||
| let FixedSize = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if it is lowered to V_MOV_B32?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be 4, but we need to report worst case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will it affect code size calculation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but by the time we do anything with the size calculation these should have been expanded already. I'm not sure anything is using getInstSizeInBytes this early

In the AGPR case this will be an 8 byte instruction,
which is part of why this case is a pain to deal with in the
first place.