[PowerPC] Folding XForm to DForm loads requires alignment for some DForm loads.

[stefanp]

Summary

Going from LDX to LD requires that the LDX be word aligned in the first place. This is a requirement of the LD instruction bot not of the LDX instruction. If we are not word aligned we must leave the load as LDX.
This bug is causing a compile-time failure in the benchmark h264ref.

[jsji]

This is added by QingShan Zhang in https://reviews.llvm.org/D49007, add him as reviewer.

[jsji]

Reduced testcases?

[steven.zhang]

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

3069 ↗

(On Diff #165082)

isImmElgibleForForwarding() will check if the ImmMO is elgible for forwarding. And it will check the alignment there according to the restriction with different instructions.
For your case, if the ImmMO is Global, isImmElgibleForForwarding() will return false if it is not the operand of ADDItocL. It covers your case. This missing part is that, if it is global, we still need to check the alignment restriction.

[nemanjai]

Please add a test case that we used to transform to a D-Form and no longer do with your patch (preferably one for DS-Form and one for DQ-Form).

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

3067 ↗

(On Diff #165082)

This comment also needs to be updated. The instruction does not require the data to be aligned, it requires the displacement to be a multiple of 4.
I suppose the problem is that unaligned data may reside at non-multiple-of-4 offsets from the base reg and we don't know yet if that is the case. However, adding the check here just shifts the bug because we could be handling an unaligned vector access with a DQ-Form instruction. If the data happens to be 4-byte aligned and the displacement ends up being 12, this check won't catch it and we're in just as much trouble.
So I am with Steven here that we should be doing the check in isImmElgibleForForwarding() where we have access to how this is being used and know what ImmMustBeMultipleOf.

[stefanp]

Moved condition to the correct location and added a test case.

[nemanjai]

LGTM.

llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp

bool PPCInstrInfo::isImmElgibleForForwarding(const MachineOperand &ImmMO,
   if (DefMI.getOpcode() == PPC::ADDItocL) {
     // The operand for ADDItocL is CPI, which isn't imm at compiling time,
     // However, we know that, it is 16-bit width, and has the alignment of 4.
     // Check if the instruction met the requirement.
     if (III.ImmMustBeMultipleOf > 4 ||
        III.TruncateImmTo || III.ImmWidth != 16)
       return false;
 
+    // Going from XForm to DForm loads means that the displacement needs to be
+    // not just an immediate but also a multiple of 4, or 16 depending on the
+    // load. A DForm load cannot be represented if it is a multiple of say 2.
+    // XForm loads do not have this restriction.
+    if (ImmMO.isGlobal() &&
+        ImmMO.getGlobal()->getAlignment() < III.ImmMustBeMultipleOf)
+      return false;
+
     return true;
   }
 
   if (ImmMO.isImm()) {
     // It is Imm, we need to check if the Imm fit the range.
     int64_t Immediate = ImmMO.getImm();
     // Sign-extend to 64-bits.
     Imm = ((uint64_t)Immediate & ~0x7FFFuLL) != 0 ?

llvm/trunk/test/CodeGen/PowerPC/p9-dform-load-alignment.ll

+; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \
+; RUN:   -verify-machineinstrs -ppc-asm-full-reg-names \
+; RUN:   -ppc-vsr-nums-as-vr < %s | FileCheck %s
+
+@best8x8mode = external dso_local local_unnamed_addr global [4 x i16], align 2
+define dso_local void @AlignDSForm() local_unnamed_addr {
+entry:
+  %0 = load <4 x i16>, <4 x i16>* bitcast ([4 x i16]* @best8x8mode to <4 x i16>*), align 2
+  store <4 x i16> %0, <4 x i16>* undef, align 4
+  unreachable
+; CHECK-LABEL: AlignDSForm
+; CHECK: addis r{{[0-9]+}}, r{{[0-9]+}}, best8x8mode@toc@ha
+; CHECK: addi r[[REG:[0-9]+]], r{{[0-9]+}}, best8x8mode@toc@l
+; CHECK: ldx r{{[0-9]+}}, 0, r[[REG]]
+}
+