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

release/18.x: [RISCV][ISel] Fix types in tryFoldSelectIntoOp (#90659) #90682

Merged
merged 1 commit into from
May 1, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Apr 30, 2024

Backport 2647bd7

Requested by: @dtcxzyw

@llvmbot llvmbot added this to the LLVM 18.X Release milestone Apr 30, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Apr 30, 2024

@topperc What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Member Author

llvmbot commented Apr 30, 2024

@llvm/pr-subscribers-backend-risc-v

Author: None (llvmbot)

Changes

Backport 2647bd7

Requested by: @dtcxzyw


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+1-1)
  • (added) llvm/test/CodeGen/RISCV/pr90652.ll (+19)
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index a0cec426002b6f..d46093b9e260a2 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -14559,7 +14559,7 @@ static SDValue tryFoldSelectIntoOp(SDNode *N, SelectionDAG &DAG,
   EVT VT = N->getValueType(0);
   SDLoc DL(N);
   SDValue OtherOp = TrueVal.getOperand(1 - OpToFold);
-  EVT OtherOpVT = OtherOp->getValueType(0);
+  EVT OtherOpVT = OtherOp.getValueType();
   SDValue IdentityOperand =
       DAG.getNeutralElement(Opc, DL, OtherOpVT, N->getFlags());
   if (!Commutative)
diff --git a/llvm/test/CodeGen/RISCV/pr90652.ll b/llvm/test/CodeGen/RISCV/pr90652.ll
new file mode 100644
index 00000000000000..2162395b92ac3c
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/pr90652.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
+; RUN: llc < %s -mtriple=riscv64 | FileCheck %s
+
+define i1 @test(i64 %x, i1 %cond1, i1 %cond2) {
+; CHECK-LABEL: test:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    addi a3, a0, 1
+; CHECK-NEXT:    slt a0, a3, a0
+; CHECK-NEXT:    not a1, a1
+; CHECK-NEXT:    and a0, a1, a0
+; CHECK-NEXT:    or a0, a2, a0
+; CHECK-NEXT:    ret
+entry:
+  %sadd = call { i64, i1 } @llvm.sadd.with.overflow.i64(i64 %x, i64 1)
+  %ov = extractvalue { i64, i1 } %sadd, 1
+  %or = or i1 %cond2, %ov
+  %sel = select i1 %cond1, i1 %cond2, i1 %or
+  ret i1 %sel
+}

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

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

LGTM

```
SelectionDAG has 17 nodes:
  t0: ch,glue = EntryToken
    t6: i64,ch = CopyFromReg t0, Register:i64 %2
  t8: i1 = truncate t6
          t4: i64,ch = CopyFromReg t0, Register:i64 %1
        t7: i1 = truncate t4
            t2: i64,ch = CopyFromReg t0, Register:i64 %0
          t10: i64,i1 = saddo t2, Constant:i64<1>
        t11: i1 = or t8, t10:1
      t12: i1 = select t7, t8, t11
    t13: i64 = any_extend t12
  t15: ch,glue = CopyToReg t0, Register:i64 $x10, t13
  t16: ch = RISCVISD::RET_GLUE t15, Register:i64 $x10, t15:1
```

`OtherOpVT` should be i1, but `OtherOp->getValueType(0)` returns `i64`,
which ignores `ResNo` in `SDValue`.

Fix llvm#90652.

(cherry picked from commit 2647bd7)
@tstellar tstellar merged commit 20b9ed6 into llvm:release/18.x May 1, 2024
7 of 8 checks passed
@tstellar
Copy link
Collaborator

tstellar commented May 1, 2024

Hi @dtcxzyw (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

@dtcxzyw
Copy link
Member

dtcxzyw commented May 1, 2024

Fixed an incorrect type inference during RISC-V instruction selection, which causes an assertion failure when trying to fold selects into their operands.

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

Successfully merging this pull request may close these issues.

4 participants