Skip to content

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
@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