Skip to content

[RISCV] Create disjoint or in RISCVGatherScatterLowering #151981

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

Merged
merged 3 commits into from
Aug 5, 2025

Conversation

el-ev
Copy link
Member

@el-ev el-ev commented Aug 4, 2025

We can create disjoint or in IRBuilder since #146350.

@llvmbot
Copy link
Member

llvmbot commented Aug 4, 2025

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

Author: Iris Shi (el-ev)

Changes

We can creating disjoint or in IRBuilder since #146350.


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

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVGatherScatterLowering.cpp (+2-3)
diff --git a/llvm/lib/Target/RISCV/RISCVGatherScatterLowering.cpp b/llvm/lib/Target/RISCV/RISCVGatherScatterLowering.cpp
index 82c0d8d4738a4..28567ada6c004 100644
--- a/llvm/lib/Target/RISCV/RISCVGatherScatterLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVGatherScatterLowering.cpp
@@ -167,9 +167,8 @@ static std::pair<Value *, Value *> matchStridedStart(Value *Start,
   default:
     llvm_unreachable("Unexpected opcode");
   case Instruction::Or:
-    // TODO: We'd be better off creating disjoint or here, but we don't yet
-    // have an IRBuilder API for that.
-    [[fallthrough]];
+    Start = Builder.CreateOr(Start, Splat, "", /* IsDisjoint = */ true);
+    break;
   case Instruction::Add:
     Start = Builder.CreateAdd(Start, Splat);
     break;

@topperc
Copy link
Collaborator

topperc commented Aug 4, 2025

Don't we have IR tests for this pass that shouldn't be affected by this change?

@mshockwave
Copy link
Member

Don't we have IR tests for this pass that shouldn't be affected by this change?

(I guess you meant "that should be affected by this change")
There is a test in test/CodeGen/RISCV/rvv/strided-load-store.ll for pattern matching disjoint OR, but not sure whether if we even have one to check whether we generate a disjoint OR. @el-ev could you add a test if there isn't one?

@topperc
Copy link
Collaborator

topperc commented Aug 4, 2025

Don't we have IR tests for this pass that shouldn't be affected by this change?

(I guess you meant "that should be affected by this change") There is a test in test/CodeGen/RISCV/rvv/strided-load-store.ll for pattern matching disjoint OR, but not sure whether if we even have one to check whether we generate a disjoint OR. @el-ev could you add a test if there isn't one?

Yes that is what I meant.

@el-ev el-ev force-pushed the users/el-ev/matchStridedStart-disjoint-or branch from 35838ed to 62c43b0 Compare August 5, 2025 03:39
@el-ev el-ev requested a review from mshockwave August 5, 2025 03:40
Copy link
Contributor

@lukel97 lukel97 left a comment

Choose a reason for hiding this comment

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

LGTM

@el-ev el-ev merged commit 0885740 into main Aug 5, 2025
9 checks passed
@el-ev el-ev deleted the users/el-ev/matchStridedStart-disjoint-or branch August 5, 2025 04:19
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.

5 participants