[PowerPC] Update alignment for ReuseLoadInfo in LowerFP_TO_INTForReuse

[lkail]

Summary

In LowerFP_TO_INTForReuse, when emitting stfiwx, alignment of 4 is
set for the MachineMemOperand, but RLI(ReuseLoadInfo)'s alignment is not updated
for following loads.

It's related to failed alignment check reported in https://bugs.llvm.org/show_bug.cgi?id=45297

[steven.zhang]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

8058

You are setting the RLI.Alignment to uninitialized value if it is not i32Stack.

[lkail]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

8058

The default constructor of Align will set it to 1. This is consistent with Chain = DAG.getStore(DAG.getEntryNode(), dl, Tmp, FIPtr, MPI); which also will have alignment of 1 for the store. I don't have deep insight of if it's correct for the Chain to have alignment of 1 yet.

[steven.zhang]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

8058

You shouldn't have the assumption of the value of the default arguments of the Alignment and the getStore(), as they can be changed. You should specify the explicit initial value of the Alignment. (i.e. Align Alignment(1)).

[steven.zhang]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

8040

Replace the Align(4) with Alignment.

8045

Pass the Alignment to the getStore() as what we have done for getMachineMemOperand().

[lkail]

Use MaybeAlign to initialize default alignment explicitly.

[lkail]

Address comment.

[lkail]

This comment was removed by lkail.

[lkail]

Discussed with @steven.zhang, it seems more appropriate to use Align directly and initialize it to the default alignment of the stored value.

[daltenty]

llvm/test/CodeGen/PowerPC/kernel-fp-round.ll

2–8

We run into this problem in 32/64-bit big endian as well when targeting pwr6+ (i.e. -mtriple=powerpc-unknown-unknown -mattr=-vsx -mcpu=pwr6), please add a run line for it.

[lkail]

Add run lines for 32/64bit big endian on pwr6.

[steven.zhang]

LGTM now, but pls hold on for some days in case others have more comments.

[daltenty]

LGTM

[sfertile]

LGTM now, but pls hold on for some days in case others have more comments.

LGTM too. Not a bad idea, but maybe give a days grace instead of a few days.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

bool i32Stack = Op.getValueType() == MVT::i32 && Subtarget.hasSTFIWX() &&
     (Op.getOpcode() == ISD::FP_TO_SINT || Subtarget.hasFPCVT());
   SDValue FIPtr = DAG.CreateStackTemporary(i32Stack ? MVT::i32 : MVT::f64);
   int FI = cast<FrameIndexSDNode>(FIPtr)->getIndex();
   MachinePointerInfo MPI =
       MachinePointerInfo::getFixedStack(DAG.getMachineFunction(), FI);
 
   // Emit a store to the stack slot.
   SDValue Chain;
+  Align Alignment(DAG.getEVTAlign(Tmp.getValueType()));
   if (i32Stack) {
     MachineFunction &MF = DAG.getMachineFunction();
+    Alignment = Align(4);
     MachineMemOperand *MMO =
-        MF.getMachineMemOperand(MPI, MachineMemOperand::MOStore, 4, Align(4));
+        MF.getMachineMemOperand(MPI, MachineMemOperand::MOStore, 4, Alignment);
     SDValue Ops[] = { DAG.getEntryNode(), Tmp, FIPtr };
     Chain = DAG.getMemIntrinsicNode(PPCISD::STFIWX, dl,
               DAG.getVTList(MVT::Other), Ops, MVT::i32, MMO);
   } else
-    Chain = DAG.getStore(DAG.getEntryNode(), dl, Tmp, FIPtr, MPI);
+    Chain = DAG.getStore(DAG.getEntryNode(), dl, Tmp, FIPtr, MPI, Alignment);
 
   // Result is a load from the stack slot.  If loading 4 bytes, make sure to
   // add in a bias on big endian.
   if (Op.getValueType() == MVT::i32 && !i32Stack) {
     FIPtr = DAG.getNode(ISD::ADD, dl, FIPtr.getValueType(), FIPtr,
                         DAG.getConstant(4, dl, FIPtr.getValueType()));
     MPI = MPI.getWithOffset(Subtarget.isLittleEndian() ? 0 : 4);
   }
 
   RLI.Chain = Chain;
   RLI.Ptr = FIPtr;
   RLI.MPI = MPI;
+  RLI.Alignment = Alignment;
 }
 
 /// Custom lowers floating point to integer conversions to use
 /// the direct move instructions available in ISA 2.07 to avoid the
 /// need for load/store combinations.
 SDValue PPCTargetLowering::LowerFP_TO_INTDirectMove(SDValue Op,
                                                     SelectionDAG &DAG,
                                                     const SDLoc &dl) const {

llvm/test/CodeGen/PowerPC/kernel-fp-round.ll

 ; RUN: llc -simplify-mir -verify-machineinstrs -stop-after=finalize-isel \
 ; RUN:   -mtriple=powerpc64le-unknown-unknown -mattr=-vsx < %s | FileCheck %s
+; RUN: llc -simplify-mir -verify-machineinstrs -stop-after=finalize-isel \
+; RUN:   -mtriple=powerpc-unknown-unknown -mcpu=pwr6 -mattr=-vsx < %s | \
+; RUN:   FileCheck --check-prefix=CHECK-P6 %s
+; RUN: llc -simplify-mir -verify-machineinstrs -stop-after=finalize-isel \
+; RUN:   -mtriple=powerpc64-unknown-unknown -mcpu=pwr6 -mattr=-vsx < %s | \
+; RUN:   FileCheck --check-prefix=CHECK-P6-64 %s
 
 define float @test(float %a) {
 ; CHECK:        stack:
 ; CHECK-NEXT:   - { id: 0, size: 4, alignment: 4 }
 ; CHECK:        %2:f8rc = FCTIWZ killed %1, implicit $rm
 ; CHECK:        STFIWX killed %2, $zero8, %3
-; CHECK-NEXT:   %4:f8rc = LFIWAX $zero8, %3 :: (load 4 from %stack.0, align 1)
+; CHECK-NEXT:   %4:f8rc = LFIWAX $zero8, %3 :: (load 4 from %stack.0)
+; CHECK-NEXT:   %5:f4rc = FCFIDS killed %4, implicit $rm
+; CHECK-NEXT:   $f1 = COPY %5
+; CHECK-NEXT:   BLR8 implicit $lr8, implicit $rm, implicit $f1
+
+; CHECK-P6:        stack:
+; CHECK-P6-NEXT:   - { id: 0, size: 4, alignment: 4 }
+; CHECK-P6:        %2:f8rc = FCTIWZ killed %1, implicit $rm
+; CHECK-P6:        STFIWX killed %2, $zero, %3
+; CHECK-P6-NEXT:   %4:f8rc = LFIWAX $zero, %3 :: (load 4 from %stack.0)
+; CHECK-P6-NEXT:   %5:f8rc = FCFID killed %4, implicit $rm
+; CHECK-P6-NEXT:   %6:f4rc = FRSP killed %5, implicit $rm
+; CHECK-P6-NEXT:   $f1 = COPY %6
+; CHECK-P6-NEXT:   BLR implicit $lr, implicit $rm, implicit $f1
+
+; CHECK-P6-64:        stack:
+; CHECK-P6-64-NEXT:   - { id: 0, size: 4, alignment: 4 }
+; CHECK-P6-64:        %2:f8rc = FCTIWZ killed %1, implicit $rm
+; CHECK-P6-64:        STFIWX killed %2, $zero8, %3
+; CHECK-P6-64-NEXT:   %4:f8rc = LFIWAX $zero8, %3 :: (load 4 from %stack.0)
+; CHECK-P6-64-NEXT:   %5:f8rc = FCFID killed %4, implicit $rm
+; CHECK-P6-64-NEXT:   %6:f4rc = FRSP killed %5, implicit $rm
+; CHECK-P6-64-NEXT:   $f1 = COPY %6
+; CHECK-P6-64-NEXT:   BLR8 implicit $lr8, implicit $rm, implicit $f1
+
 entry:
   %b = fptosi float %a to i32
   %c = sitofp i32 %b to float
   ret float %c
 }