Skip to content

AMDGPU/MC: Try harder to evaluate absolute MC expressions #145146

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nhaehnle
Copy link
Collaborator

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.

This is a follow-up to commit 24c8605 ("AMDGPU/MC: Fix emitting
absolute expressions (llvm#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.
@llvmbot
Copy link
Member

llvmbot commented Jun 21, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Nicolai Hähnle (nhaehnle)

Changes

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.


Full diff: https://github.com/llvm/llvm-project/pull/145146.diff

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp (+1-5)
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp
index ab7e51ac28322..abb5003f3320b 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp
@@ -255,13 +255,9 @@ AMDGPUMCCodeEmitter::getLitEncoding(const MCOperand &MO,
                                     const MCSubtargetInfo &STI) const {
   int64_t Imm;
   if (MO.isExpr()) {
-    const auto *C = dyn_cast<MCConstantExpr>(MO.getExpr());
-    if (!C)
+    if (!MO.getExpr()->evaluateAsAbsolute(Imm))
       return 255;
-
-    Imm = C->getValue();
   } else {
-
     assert(!MO.isDFPImm());
 
     if (!MO.isImm())

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

Successfully merging this pull request may close these issues.

3 participants