Skip to content

[RISCV] Change how MMO is rebuilt in lowerFixedLengthVectorLoadToRVV/lowerFixedLengthVectorStoreToRVV #88811

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 1 commit into from
Apr 16, 2024

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Apr 15, 2024

Copy the pointer info, flags, alignment, AAInfo, and ranges, but let getLoad rebuild the MMO using the scalable type used for the the new load/store. This makes sure the LLT minimum size matches the ContainerVT minimum size. This is important since vscale_range may have been used to determine that the fixed vector was the exact size of a scalable vector.

Fixes #88799

…lowerFixedLengthVectorStoreToRVV

Copy the pointer info, flags, alignment, AAInfo, and ranges, but
let getLoad rebuild the MMO using the scalable type used for the
the new load/store. This makes sure the LLT minimum size matches
the ContainerVT minimum size. This is important since vscale_range
may have been used to determine that the fixed vector was the exact
size of a scalable vector.

Fixes llvm#88799
@llvmbot
Copy link
Member

llvmbot commented Apr 15, 2024

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

Author: Craig Topper (topperc)

Changes

Copy the pointer info, flags, alignment, AAInfo, and ranges, but let getLoad rebuild the MMO using the scalable type used for the the new load/store. This makes sure the LLT minimum size matches the ContainerVT minimum size. This is important since vscale_range may have been used to determine that the fixed vector was the exact size of a scalable vector.

Fixes #88799


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+5-14)
  • (added) llvm/test/CodeGen/RISCV/rvv/pr88799.ll (+19)
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 27387595164a46..76a351ada67182 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -10403,14 +10403,10 @@ RISCVTargetLowering::lowerFixedLengthVectorLoadToRVV(SDValue Op,
   if (MinVLMAX == MaxVLMAX && MinVLMAX == VT.getVectorNumElements() &&
       getLMUL1VT(ContainerVT).bitsLE(ContainerVT)) {
     MachineMemOperand *MMO = Load->getMemOperand();
-    MachineFunction &MF = DAG.getMachineFunction();
-    MMO = MF.getMachineMemOperand(
-        MMO, MMO->getPointerInfo(),
-        MMO->getMemoryType().isValid()
-            ? LLT::scalable_vector(1, MMO->getMemoryType().getSizeInBits())
-            : MMO->getMemoryType());
     SDValue NewLoad =
-        DAG.getLoad(ContainerVT, DL, Load->getChain(), Load->getBasePtr(), MMO);
+        DAG.getLoad(ContainerVT, DL, Load->getChain(), Load->getBasePtr(),
+                    MMO->getPointerInfo(), MMO->getBaseAlign(), MMO->getFlags(),
+                    MMO->getAAInfo(), MMO->getRanges());
     SDValue Result = convertFromScalableVector(VT, NewLoad, DAG, Subtarget);
     return DAG.getMergeValues({Result, NewLoad.getValue(1)}, DL);
   }
@@ -10470,14 +10466,9 @@ RISCVTargetLowering::lowerFixedLengthVectorStoreToRVV(SDValue Op,
   if (MinVLMAX == MaxVLMAX && MinVLMAX == VT.getVectorNumElements() &&
       getLMUL1VT(ContainerVT).bitsLE(ContainerVT)) {
     MachineMemOperand *MMO = Store->getMemOperand();
-    MachineFunction &MF = DAG.getMachineFunction();
-    MMO = MF.getMachineMemOperand(
-        MMO, MMO->getPointerInfo(),
-        MMO->getMemoryType().isValid()
-            ? LLT::scalable_vector(1, MMO->getMemoryType().getSizeInBits())
-            : MMO->getMemoryType());
     return DAG.getStore(Store->getChain(), DL, NewValue, Store->getBasePtr(),
-                        MMO);
+                        MMO->getPointerInfo(), MMO->getBaseAlign(),
+                        MMO->getFlags(), MMO->getAAInfo());
   }
 
   SDValue VL = getVLOp(VT.getVectorNumElements(), ContainerVT, DL, DAG,
diff --git a/llvm/test/CodeGen/RISCV/rvv/pr88799.ll b/llvm/test/CodeGen/RISCV/rvv/pr88799.ll
new file mode 100644
index 00000000000000..7212a789f9e7e0
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/rvv/pr88799.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-unknown-linux-gnu -mattr=+v | FileCheck %s
+
+define i32 @main() vscale_range(2,2) {
+; CHECK-LABEL: main:
+; CHECK:       # %bb.0: # %vector.body
+; CHECK-NEXT:    lui a0, 1040368
+; CHECK-NEXT:    addiw a0, a0, -144
+; CHECK-NEXT:    vl2re16.v v8, (a0)
+; CHECK-NEXT:    vs2r.v v8, (zero)
+; CHECK-NEXT:    li a0, 0
+; CHECK-NEXT:    ret
+vector.body:
+  %0 = load <16 x i16>, ptr getelementptr ([3 x [23 x [23 x i16]]], ptr null, i64 -10593, i64 1, i64 22, i64 0), align 16
+  store <16 x i16> %0, ptr null, align 2
+  %wide.load = load <vscale x 8 x i16>, ptr getelementptr ([3 x [23 x [23 x i16]]], ptr null, i64 -10593, i64 1, i64 22, i64 0), align 16
+  store <vscale x 8 x i16> %wide.load, ptr null, align 2
+  ret i32 0
+}

@hiraditya
Copy link
Collaborator

LGTM but I'd let original author/code-owner review it. Thanks for the quick fix!

Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

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

LGTM.

@topperc topperc merged commit 17d6bf0 into llvm:main Apr 16, 2024
5 of 6 checks passed
@topperc topperc deleted the pr/fix-88799 branch April 16, 2024 05:39
@davemgreen
Copy link
Collaborator

Hi. Sorry for the bug and thanks for the fix. I think I missed that the typesize needed to be adjusted by the vscale range. The new code sounds good if its producing the right mmo sizes.

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.

[CodeGen][RISC-V] Assertion `(!MMO->getSize().hasValue() || !getSize().hasValue() || MMO->getSize() == getSize()) && "Size mismatch!"' failed.
5 participants