[GlobalISel][Legalizer] Support reducing load/store width in big endian order

[0x59616e]

Summary

LegalizerHelper::reduceLoadStoreWidth uses little endian order when splitting type. This patch adds big endian support.

[0x59616e]

update diff

[0x59616e]

update diff

[0x59616e]

rebase

[0x59616e]

update diff

[arsenm]

llvm/test/CodeGen/M68k/GlobalISel/legalizer_load_store.ll

1

Generally legalizer tests are MIR->MIR. I think it would be best to have some end to end IR tests, in addition to MIR->MIR tests that only run the legalizer

[0x59616e]

llvm/test/CodeGen/M68k/GlobalISel/legalizer_load_store.ll

1

Do you mind explaining the meaning of "end to end IR tests" ?

[0x59616e]

update diff

Rewrite tests in mir form suggested by @arsenm. Thanks !

[0x59616e]

llvm/test/CodeGen/M68k/GlobalISel/legalizer_load_store.ll

1

Guess: If you mean IR->assembly test, that needs instruction selector & regbank selector support, which I haven't started to do.

[0x59616e]

ping.

[0x59616e]

ping

[RKSimon]

llvm/test/CodeGen/M68k/GlobalISel/legalize-load-store.mir

98 ↗

(On Diff #401596)

pre-commit + rebase this so that the patch some the codegen diff?

[0x59616e]

rebase

[0x59616e]

llvm/test/CodeGen/M68k/GlobalISel/legalize-load-store.mir

98 ↗

(On Diff #401596)

Thanks for commenting !

Although I did some search on google and mailing list, I still don't know what "pre-commit" should do. I tried to commit this patch on the main branch and no error message happened. Do you mean "git clang-format" or other stuff ?

Apologize for my stupidity.

[0x59616e]

move test file into another patch + rebase

[0x59616e]

llvm/test/CodeGen/M68k/GlobalISel/legalize-load-store.mir

98 ↗

(On Diff #401596)

I guess what you mean is to separate test file into a "pre-commit test" patch ?

[RKSimon]

I meant rebase this patch on top of D119132 - that way we show the effect of this patch on the test's codegen. So D119132 must pass with current upstream trunk

[0x59616e]

Address feedback from @RKSimon. Thanks !

[RKSimon]

LGTM

[0x59616e]

Thanks @arsenm @RKSimon !

Since I don't have commit access, could someone commit this on behalf of me ?

[arsenm]

146c7820d9b67eea4577b59d7dce9601d774cf66

[0x59616e]

Thanks again @arsenm @RKSimon ! Really appreciate your help ;)

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

LegalizerHelper::reduceLoadStoreWidth(GLoadStore &LdStMI, unsigned TypeIdx,
   const LLT OffsetTy = LLT::scalar(PtrTy.getSizeInBits());
 
   unsigned TotalSize = ValTy.getSizeInBits();
 
   // Split the load/store into PartTy sized pieces starting at Offset. If this
   // is a load, return the new registers in ValRegs. For a store, each elements
   // of ValRegs should be PartTy. Returns the next offset that needs to be
   // handled.
+  bool isBigEndian = MIRBuilder.getDataLayout().isBigEndian();
   auto MMO = LdStMI.getMMO();
   auto splitTypePieces = [=](LLT PartTy, SmallVectorImpl<Register> &ValRegs,
-                             unsigned Offset) -> unsigned {
+                             unsigned NumParts, unsigned Offset) -> unsigned {
     MachineFunction &MF = MIRBuilder.getMF();
     unsigned PartSize = PartTy.getSizeInBits();
     for (unsigned Idx = 0, E = NumParts; Idx != E && Offset < TotalSize;
-         Offset += PartSize, ++Idx) {
+         ++Idx) {
       unsigned ByteOffset = Offset / 8;
       Register NewAddrReg;
 
       MIRBuilder.materializePtrAdd(NewAddrReg, AddrReg, OffsetTy, ByteOffset);
 
       MachineMemOperand *NewMMO =
           MF.getMachineMemOperand(&MMO, ByteOffset, PartTy);
 
       if (IsLoad) {
         Register Dst = MRI.createGenericVirtualRegister(PartTy);
         ValRegs.push_back(Dst);
         MIRBuilder.buildLoad(Dst, NewAddrReg, *NewMMO);
       } else {
         MIRBuilder.buildStore(ValRegs[Idx], NewAddrReg, *NewMMO);
       }
+      Offset = isBigEndian ? Offset - PartSize : Offset + PartSize;
     }
 
     return Offset;
   };
 
-  unsigned HandledOffset = splitTypePieces(NarrowTy, NarrowRegs, 0);
+  unsigned Offset = isBigEndian ? TotalSize - NarrowTy.getSizeInBits() : 0;
+  unsigned HandledOffset =
+      splitTypePieces(NarrowTy, NarrowRegs, NumParts, Offset);
 
   // Handle the rest of the register if this isn't an even type breakdown.
   if (LeftoverTy.isValid())
-    splitTypePieces(LeftoverTy, NarrowLeftoverRegs, HandledOffset);
+    splitTypePieces(LeftoverTy, NarrowLeftoverRegs, NumLeftover, HandledOffset);
 
   if (IsLoad) {
     insertParts(ValReg, ValTy, NarrowTy, NarrowRegs,
                 LeftoverTy, NarrowLeftoverRegs);
   }
 
   LdStMI.eraseFromParent();
   return Legalized;

llvm/test/CodeGen/M68k/GlobalISel/legalizer_load_store.ll

+; RUN: llc -mtriple=m68k -global-isel -stop-after=legalizer %s -o - | FileCheck %s
+
+define void @test_store_i64(i64* %0) nounwind {
+; CHECK-LABEL: name: test_store_i64
+; CHECK:  [[FI:%[0-9]+]]:_(p0) = G_FRAME_INDEX %fixed-stack.0
+; CHECK-NEXT:  [[PTR_Hi:%[0-9]+]]:_(p0) = G_LOAD [[FI]](p0)
+; CHECK-NEXT:  [[Lo:%[0-9]+]]:_(s32) = G_CONSTANT i32 1
+; CHECK-NEXT:  [[Hi:%[0-9]+]]:_(s32) = G_CONSTANT i32 2
+; CHECK-NEXT:  [[Offset:%[0-9]+]]:_(s32) = G_CONSTANT i32 4
+; CHECK-NEXT:  [[PTR_Lo:%[0-9]+]]:_(p0) = G_PTR_ADD [[PTR_Hi]], [[Offset]](s32)
+; CHECK-NEXT:  G_STORE [[Lo]](s32), [[PTR_Lo]](p0)
+; CHECK-NEXT:  G_STORE [[Hi]](s32), [[PTR_Hi]](p0)
+; CHECK-NEXT:  RTS
+  store i64 8589934593, i64* %0
+  ret void
+}
+
+define i64 @test_load_i64(i64* %0) nounwind {
+; CHECK-LABEL: name: test_load_i64
+; CHECK:       [[FI:%[0-9]+]]:_(p0) = G_FRAME_INDEX %fixed-stack.0
+; CHECK-NEXT:  [[PTR_Hi:%[0-9]+]]:_(p0) = G_LOAD [[FI]](p0)
+; CHECK-NEXT:  [[Offset:%[0-9]+]]:_(s32) = G_CONSTANT i32 4
+; CHECK-NEXT:  [[PTR_Lo:%[0-9]+]]:_(p0) = G_PTR_ADD [[PTR_Hi]], [[Offset]](s32)
+; CHECK-NEXT:  [[Lo:%[0-9]+]]:_(s32) = G_LOAD [[PTR_Lo]]
+; CHECK-NEXT:  [[Hi:%[0-9]+]]:_(s32) = G_LOAD [[PTR_Hi]]
+; CHECK-NEXT:  $d1 = COPY [[Lo]]
+; CHECK-NEXT:  $d0 = COPY [[Hi]]
+; RTS implicit $d1, implicit $d0
+  %2 = load i64, i64* %0
+  ret i64 %2
+}