Skip to content
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] Fix Ins64 clamp in the VOPProfile. NFC. #81925

Merged
merged 2 commits into from
Feb 16, 2024

Conversation

rampitec
Copy link
Collaborator

For some reason only IntClamp was added to the Ins64, bit not FPClamp. As is this is NFC, but fails to produce proper dag downstream.

For some reason only IntClamp was added to the Ins64, bit not FPClamp.
As is this is NFC, but fails to produce proper dag downstream.
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 15, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Stanislav Mekhanoshin (rampitec)

Changes

For some reason only IntClamp was added to the Ins64, bit not FPClamp. As is this is NFC, but fails to produce proper dag downstream.


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

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.td (+1-1)
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.td b/llvm/lib/Target/AMDGPU/SIInstrInfo.td
index 4b7555de712c80..e5fbcfafd3809f 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.td
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.td
@@ -2344,7 +2344,7 @@ class VOPProfile <list<ValueType> _ArgVT, bit _EnableClamp = 0> {
 
   field dag Ins32 = getIns32<Src0RC32, Src1RC32, NumSrcArgs>.ret;
   field dag Ins64 = getIns64<Src0RC64, Src1RC64, Src2RC64, NumSrcArgs,
-                             HasIntClamp, HasModifiers, HasSrc2Mods,
+                             HasClamp, HasModifiers, HasSrc2Mods,
                              HasOMod, Src0Mod, Src1Mod, Src2Mod>.ret;
   field dag InsVOP3P = getInsVOP3P<Src0RC64, Src1RC64, Src2RC64,
                                    NumSrcArgs, HasClamp, HasOpSel,

@jayfoad
Copy link
Contributor

jayfoad commented Feb 16, 2024

There's another use of HasIntClamp in VOP3_Pseudo in VOPInstructions.td:

  let AsmMatchConverter =
    !if(isVOP3P,
        "cvtVOP3P",
        !if(!or(P.HasModifiers, P.HasOMod, P.HasIntClamp),
            "cvtVOP3",
            ""));

Does that also need changing?

@rampitec rampitec merged commit 0b1c25c into llvm:main Feb 16, 2024
3 of 4 checks passed
@rampitec rampitec deleted the ins64-clamp branch February 16, 2024 18:18
@rampitec
Copy link
Collaborator Author

There's another use of HasIntClamp in VOP3_Pseudo in VOPInstructions.td:

  let AsmMatchConverter =
    !if(isVOP3P,
        "cvtVOP3P",
        !if(!or(P.HasModifiers, P.HasOMod, P.HasIntClamp),
            "cvtVOP3",
            ""));

Does that also need changing?

Yes, nothing breaks if I use HasClamp.

#82020

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.

None yet

3 participants