AMDGPU/MC: Fix emitting absolute expressions #136789
Conversation
When absolute MCExprs appear in normal instruction operands, we have to emit them like a normal inline constant or literal. More generally, an MCExpr that happens to have an absolute evaluation should be treated exactly like an immediate operand here. No test; I found this downstream, and I don't think it can be triggered upstream yet. Fixes: 1623866 ("[AMDGPU][MC] Support UC_VERSION_* constants. (llvm#95618)")
|
@llvm/pr-subscribers-backend-amdgpu Author: Nicolai Hähnle (nhaehnle) ChangesWhen absolute MCExprs appear in normal instruction operands, we have to emit them like a normal inline constant or literal. More generally, an MCExpr that happens to have an absolute evaluation should be treated exactly like an immediate operand here. No test; I found this downstream, and I don't think it can be triggered upstream yet. Fixes: 1623866 ("[AMDGPU][MC] Support UC_VERSION_* constants. (#95618)") Full diff: https://github.com/llvm/llvm-project/pull/136789.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp
index 1e82ee36dc0eb..9cf712318bfa1 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp
@@ -647,13 +647,15 @@ void AMDGPUMCCodeEmitter::getMachineOpValueT16Lo128(
void AMDGPUMCCodeEmitter::getMachineOpValueCommon(
const MCInst &MI, const MCOperand &MO, unsigned OpNo, APInt &Op,
SmallVectorImpl<MCFixup> &Fixups, const MCSubtargetInfo &STI) const {
+ bool isLikeImm = false;
int64_t Val;
- if (MO.isExpr() && MO.getExpr()->evaluateAsAbsolute(Val)) {
- Op = Val;
- return;
- }
- if (MO.isExpr() && MO.getExpr()->getKind() != MCExpr::Constant) {
+ if (MO.isImm()) {
+ Val = MO.getImm();
+ isLikeImm = true;
+ } else if (MO.isExpr() && MO.getExpr()->evaluateAsAbsolute(Val)) {
+ isLikeImm = true;
+ } else if (MO.isExpr()) {
// FIXME: If this is expression is PCRel or not should not depend on what
// the expression looks like. Given that this is just a general expression,
// it should probably be FK_Data_4 and whatever is producing
@@ -683,8 +685,12 @@ void AMDGPUMCCodeEmitter::getMachineOpValueCommon(
Op = *Enc;
return;
}
- } else if (MO.isImm()) {
- Op = MO.getImm();
+
+ llvm_unreachable("Operand not supported for SISrc");
+ }
+
+ if (isLikeImm) {
+ Op = Val;
return;
}
|
arsenm
left a comment
arsenm
left a comment
There was a problem hiding this comment.
Don't see why this isn't testable
Any suggestions as to how? In the downstream case, this is triggered by a new MCResourceInfo::ResourceInfoKind which can be placed into an instruction operand. It's possible I missed something, but in main these enums only seem to be used in connection to the kernel descriptor. (And the code I'm changing was initially added for the MC-only purpose of supporting some s_version syntax.) |
So emit a kernel descriptor? |
Maybe I misunderstand what you mean, but kernel descriptors don't contain instructions, do they? |
When absolute MCExprs appear in normal instruction operands, we have to emit them like a normal inline constant or literal. More generally, an MCExpr that happens to have an absolute evaluation should be treated exactly like an immediate operand here. No test; I found this downstream, and I don't think it can be triggered upstream yet. Fixes: 1623866 ("[AMDGPU][MC] Support UC_VERSION_* constants. (llvm#95618)")
This is a follow-up to commit 24c8605 ("AMDGPU/MC: Fix emitting absolute expressions (#136789)"). In some downstream work, we end up with an MCTargetExpr that is a maximum (AGVK_Max) in an instruction operand. getMachineOpValueCommon recognizes the absolute nature of the expression and doesn't emit a fixup. getLitEncoding needs to be aligned with this decision, else we end up with a 0 immediate without a corresponding fixup. Note that evaluateAsAbsolute checks for MCConstantExpr as a fast path, so this accepts strictly more cases than before. I've tried several ways to write a test for this without success. The challenge is that there is no upstream way to generate this kind of expression in an instruction operand natively, and trying to create one via inline assembly fails because the assembly parser evaluates the expression to a constant during parsing.
3 participants
When absolute MCExprs appear in normal instruction operands, we have to emit them like a normal inline constant or literal. More generally, an MCExpr that happens to have an absolute evaluation should be treated exactly like an immediate operand here.
No test; I found this downstream, and I don't think it can be triggered upstream yet.
Fixes: 1623866 ("[AMDGPU][MC] Support UC_VERSION_* constants. (#95618)")