MachineInstr: Reason locally about some memory objects before going to AA.

[bmakam]

Summary

This addresses a FIXME in MachineInstr::mayAlias.

[gberry]

lib/CodeGen/MachineInstr.cpp

1662

I don't think this is right since getValue() will return nullptr for any PseudoSourceValue, so SameVal will always be true for any two PseudoSourceValues, even if they are different.

1665

I think the more interesting checks for PseudoValues are whether they can alias at all (i.e. PseudoSourceValue::isAliased/mayAlias), and if not they can only alias with themselves.

[bmakam]

lib/CodeGen/MachineInstr.cpp

1662

I am not sure I fully understand your comment, getValue will not return nullptr here because of the earlier condition that returns true (i.e. mayAlias) if it was nullptr.

[gberry]

lib/CodeGen/MachineInstr.cpp

1662

I missed that check before. Given that check is there, the code below is useless since an MMO can only have a Value or a PseudoValue, so it will never change SameVal.

I think there are three interesting cases (at least) here:

• pseudo vs. pseudo: i believe these can only alias if they are equal (and even then an overlap check can find cases that don't alias)
• pseudo vs. value: can never alias if pseudo is not mayAlias (perhaps a stronger check is possible)
• value vs. value: do overlap checking if no AA is present, otherwise just let AA do it

[bmakam]

Address Geoff's comments.

[efriedma]

Any idea what effect this has on performance, if any?

The changes to the AMDGPU tests need more explanation.

lib/CodeGen/MachineInstr.cpp

1664

This looks weird. If getPseudoValue() is non-null, getValue() is null; therefore PSVa is always null here.

[bmakam]

In D34181#782655, @efriedma wrote:

Any idea what effect this has on performance, if any?

This patch is performance neutral on Falkor/Kryo. This addresses a FIXME that I saw in the passing and attempted to fix as it saves compile time and catch corner cases that are currently not covered.

The changes to the AMDGPU tests need more explanation.

These lit tests were failing due to instruction scheduling changes, so I modified the tests to reflect the new code generation and made the AMDGPU tests more robust.

lib/CodeGen/MachineInstr.cpp

1664

getValue() will not be null here as we check for getValue() is null and return early at line 1638

if (!MMOa->getValue() || !MMOb->getValue())
  return true;

[efriedma]

lib/CodeGen/MachineInstr.cpp

1664

What I meant to say is "If getValue() is non-null, getPseudoValue() is null, therefore PSVa is always null here."

[gberry]

lib/CodeGen/MachineInstr.cpp

1664

This is what I was trying to express in my previous comment.
I also think these Pseduo value checks could be expanded to handle cases other than isConstant by checking the mayAlias/isAliased as I outlined above.

[bmakam]

I see the problem now, a part of the patch that addressed Geoff's comments earlier was missing. I will update the correct patch for review shortly.

[bmakam]

Uploading the correct patch. I was being conservative by checking for isConstant only, I can add more cases if you think they are more interesting.

[arsenm]

test/CodeGen/AMDGPU/load-global-i8.ll

370

R600 checks often break because the annoying clause marker syntax, so I'm not concerned about these

[gberry]

This change is enhancing the alias checking in three different scenarios, one of which is fairly subtle:

1. adding checking for non-Value MMOs
2. adding simple overlap checking for two equal Value MMOs when no AA is available
3. refining simple overlap checking for two equal Value MMOs when AA is available

Item 3 is the subtle one. The reason this is improving on the regular AA results is because of the way we're constructing the AA query for MMOs with a non-zero offset. For example, if we're asking about MMOa x[0:3] and MMOb x[4:7], we construct an AA query that instead asks about x[0:3] and x[0:7]. I assume this was done intentionally to avoid having to construct a new GEP for the non-zero offset case, but I don't know that for sure.

It seems like it might be reasonable to split off Item 1 from above into a separate patch and expand the cases it handles. Specifically, getting it to handle stack references on AArch64 when some are SP based and some are FP based would be interesting. You should be able to use the PseudoSourceValue::isAliased() to catch this. You should also be able to write MIR tests that confirm that this allows e.g. more load/store pairing on AArch64.

[hfinkel]

LGTM

lib/CodeGen/MachineInstr.cpp

1662

I think there are three interesting cases (at least) here:

• pseudo vs. pseudo: i believe these can only alias if they are equal (and even then an overlap check can find cases that don't alias)

I also believe that this is true.

• pseudo vs. value: can never alias if pseudo is not mayAlias (perhaps a stronger check is possible)

Yes, although I think you need to check isAliased, not mayAlias. What you care about is that the access to the PseudoSourceValue is accessing something that no IR value can point to (which is what isAliased checks) because you're checking against an IR pointer value.

I'd certainly like to see this as a follow-up patch.

[bmakam]

Thanks for taking a look. Rebased and committed in r310825.

lib/CodeGen/MachineInstr.cpp

bool MachineInstr::isSafeToMove(AliasAnalysis *AA, bool &SawStore) const {
 
   return true;
 }
 
 bool MachineInstr::mayAlias(AliasAnalysis *AA, MachineInstr &Other,
                             bool UseTBAA) {
   const MachineFunction *MF = getParent()->getParent();
   const TargetInstrInfo *TII = MF->getSubtarget().getInstrInfo();
+  const MachineFrameInfo &MFI = MF->getFrameInfo();
 
   // If neither instruction stores to memory, they can't alias in any
   // meaningful way, even if they read from the same address.
   if (!mayStore() && !Other.mayStore())
     return false;
 
   // Let the target decide if memory accesses cannot possibly overlap.
   if (TII->areMemAccessesTriviallyDisjoint(*this, Other, AA))
     return false;
 
-  if (!AA)
-    return true;
-
   // FIXME: Need to handle multiple memory operands to support all targets.
   if (!hasOneMemOperand() || !Other.hasOneMemOperand())
     return true;
 
   MachineMemOperand *MMOa = *memoperands_begin();
   MachineMemOperand *MMOb = *Other.memoperands_begin();
 
-  if (!MMOa->getValue() || !MMOb->getValue())
-    return true;
-
   // The following interface to AA is fashioned after DAGCombiner::isAlias
   // and operates with MachineMemOperand offset with some important
   // assumptions:
   //   - LLVM fundamentally assumes flat address spaces.
   //   - MachineOperand offset can *only* result from legalization and
   //     cannot affect queries other than the trivial case of overlap
   //     checking.
   //   - These offsets never wrap and never step outside
   //     of allocated objects.
   //   - There should never be any negative offsets here.
   //
   // FIXME: Modify API to hide this math from "user"
-  // FIXME: Even before we go to AA we can reason locally about some
+  // Even before we go to AA we can reason locally about some
   // memory objects. It can save compile time, and possibly catch some
   // corner cases not currently covered.
 
-  assert((MMOa->getOffset() >= 0) && "Negative MachineMemOperand offset");
-  assert((MMOb->getOffset() >= 0) && "Negative MachineMemOperand offset");
+  int64_t OffsetA = MMOa->getOffset();
+  int64_t OffsetB = MMOb->getOffset();
+
+  assert((OffsetA >= 0) && "Negative MachineMemOperand offset");
+  assert((OffsetB >= 0) && "Negative MachineMemOperand offset");
+
+  int64_t MinOffset = std::min(OffsetA, OffsetB);
+  int64_t WidthA = MMOa->getSize();
+  int64_t WidthB = MMOb->getSize();
+  const Value *ValA = MMOa->getValue();
+  const Value *ValB = MMOb->getValue();
+  bool SameVal = (ValA && ValB && (ValA == ValB));
+  if (!SameVal) {
+    const PseudoSourceValue *PSVa = MMOa->getPseudoValue();
+    const PseudoSourceValue *PSVb = MMOb->getPseudoValue();
+    if (PSVa && PSVa->isConstant(&MFI))
+      return false;
+    if (PSVb && PSVb->isConstant(&MFI))
+      return false;
+    if (PSVa && PSVb && (PSVa == PSVb))
+      SameVal = true;
+  }
+
+  if (SameVal) {
+    int64_t MaxOffset = std::max(OffsetA, OffsetB);
+    int64_t LowWidth = (MinOffset == OffsetA) ? WidthA : WidthB;
+    return (MinOffset + LowWidth > MaxOffset);
+  }
+
+  if (!AA)
+    return true;
+
+  if (!ValA || !ValB)
+    return true;
+
+  int64_t Overlapa = WidthA + OffsetA - MinOffset;
+  int64_t Overlapb = WidthB + OffsetB - MinOffset;
 
-  int64_t MinOffset = std::min(MMOa->getOffset(), MMOb->getOffset());
-  int64_t Overlapa = MMOa->getSize() + MMOa->getOffset() - MinOffset;
-  int64_t Overlapb = MMOb->getSize() + MMOb->getOffset() - MinOffset;
-
-  AliasResult AAResult =
-      AA->alias(MemoryLocation(MMOa->getValue(), Overlapa,
-                               UseTBAA ? MMOa->getAAInfo() : AAMDNodes()),
-                MemoryLocation(MMOb->getValue(), Overlapb,
-                               UseTBAA ? MMOb->getAAInfo() : AAMDNodes()));
+  AliasResult AAResult = AA->alias(
+      MemoryLocation(ValA, Overlapa, UseTBAA ? MMOa->getAAInfo() : AAMDNodes()),
+      MemoryLocation(ValB, Overlapb,
 
   return (AAResult != NoAlias);
 }
 
 /// hasOrderedMemoryRef - Return true if this instruction may have an ordered
 /// or volatile memory reference, or if the information describing the memory
 /// reference is not available. Return false if it is known to have no ordered
 /// memory references.

test/CodeGen/AArch64/ldst-opt.ll

entry:
   ret void
 }
 
 ; Similar to merge_zr32, but for 64-bit values and with unaligned stores.
 define void @merge_zr64_unalign(<2 x i64>* %p) {
 ; CHECK-LABEL: merge_zr64_unalign:
 ; CHECK: // %entry
 ; NOSTRICTALIGN-NEXT: stp xzr, xzr, [x{{[0-9]+}}]
-; STRICTALIGN: strb wzr,
+; STRICTALIGN: strb
 ; STRICTALIGN: strb
 ; STRICTALIGN: strb
 ; STRICTALIGN: strb
 ; STRICTALIGN: strb
 ; STRICTALIGN: strb
 ; STRICTALIGN: strb
 ; STRICTALIGN: strb
 ; STRICTALIGN: strb

test/CodeGen/AMDGPU/load-global-i16.ll

define amdgpu_kernel void @global_sextload_v2i16_to_v2i32(<2 x i32> addrspace(1)* %out, <2 x i16> addrspace(1)* %in) #0 {
   store <2 x i32> %ext, <2 x i32> addrspace(1)* %out
   ret void
 }
 
 ; FUNC-LABEL: {{^}}global_zextload_v3i16_to_v3i32:
 ; GCN-NOHSA: buffer_load_dwordx2
 ; GCN-HSA: flat_load_dwordx2
 
-; CM: MEM_RAT_CACHELESS STORE_DWORD [[ST_HI:T[0-9]]].X, {{T[0-9]\.[XYZW]}}
 ; CM: MEM_RAT_CACHELESS STORE_DWORD [[ST_LO:T[0-9]]], {{T[0-9]\.[XYZW]}}
+; CM: MEM_RAT_CACHELESS STORE_DWORD [[ST_HI:T[0-9]]].X, {{T[0-9]\.[XYZW]}}
 ; EG: MEM_RAT_CACHELESS STORE_RAW [[ST_HI:T[0-9]]].X, {{T[0-9]\.[XYZW]}},
 ; EG: MEM_RAT_CACHELESS STORE_RAW [[ST_LO:T[0-9]]].XY, {{T[0-9]\.[XYZW]}},
 ; EGCM-DAG: VTX_READ_32 [[DST_LO:T[0-9]\.[XYZW]]], {{T[0-9]\.[XYZW]}}, 0, #1
 ; EGCM-DAG: VTX_READ_16 [[DST_HI:T[0-9]\.[XYZW]]], {{T[0-9]\.[XYZW]}}, 4, #1
 ; TODO: This should use DST, but for some there are redundant MOVs
 ; EGCM: LSHR {{[* ]*}}[[ST_LO]].Y, {{T[0-9]\.[XYZW]}}, literal
 ; EGCM: 16
-; EGCM: AND_INT {{[* ]*}}[[ST_LO]].X, {{T[0-9]\.[XYZW]}}, literal
-; EGCM: AND_INT {{[* ]*}}[[ST_HI]].X, [[DST_HI]], literal
 define amdgpu_kernel void @global_zextload_v3i16_to_v3i32(<3 x i32> addrspace(1)* %out, <3 x i16> addrspace(1)* %in) {
 entry:
   %ld = load <3 x i16>, <3 x i16> addrspace(1)* %in
   %ext = zext <3 x i16> %ld to <3 x i32>
   store <3 x i32> %ext, <3 x i32> addrspace(1)* %out
   ret void
 }
 
 ; FUNC-LABEL: {{^}}global_sextload_v3i16_to_v3i32:
 ; GCN-NOHSA: buffer_load_dwordx2
 ; GCN-HSA: flat_load_dwordx2
 
-; CM: MEM_RAT_CACHELESS STORE_DWORD [[ST_HI:T[0-9]]].X, {{T[0-9]\.[XYZW]}}
 ; CM: MEM_RAT_CACHELESS STORE_DWORD [[ST_LO:T[0-9]]], {{T[0-9]\.[XYZW]}}
+; CM: MEM_RAT_CACHELESS STORE_DWORD [[ST_HI:T[0-9]]].X, {{T[0-9]\.[XYZW]}}
 ; EG: MEM_RAT_CACHELESS STORE_RAW [[ST_HI:T[0-9]]].X, {{T[0-9]\.[XYZW]}},
 ; EG: MEM_RAT_CACHELESS STORE_RAW [[ST_LO:T[0-9]]].XY, {{T[0-9]\.[XYZW]}},
 ; EGCM-DAG: VTX_READ_32 [[DST_LO:T[0-9]\.[XYZW]]], {{T[0-9].[XYZW]}}, 0, #1
 ; EGCM-DAG: VTX_READ_16 [[DST_HI:T[0-9]\.[XYZW]]], {{T[0-9].[XYZW]}}, 4, #1
 ; TODO: This should use DST, but for some there are redundant MOVs
 ; EGCM-DAG: ASHR {{[* ]*}}[[ST_LO]].Y, {{T[0-9]\.[XYZW]}}, literal
 ; EGCM-DAG: BFE_INT {{[* ]*}}[[ST_LO]].X, {{T[0-9]\.[XYZW]}}, 0.0, literal
 ; EGCM-DAG: BFE_INT {{[* ]*}}[[ST_HI]].X, [[DST_HI]], 0.0, literal

test/CodeGen/AMDGPU/load-global-i8.ll

define amdgpu_kernel void @global_zextload_v16i8_to_v16i32(<16 x i32> addrspace(1)* %out, <16 x i8> addrspace(1)* %in) #0 {
   store <16 x i32> %ext, <16 x i32> addrspace(1)* %out
   ret void
 }
 
 ; FUNC-LABEL: {{^}}global_sextload_v16i8_to_v16i32:
 
 ; EG: VTX_READ_128 [[DST:T[0-9]+\.XYZW]], T{{[0-9]+}}.X, 0, #1
 ; TODO: These should use DST, but for some there are redundant MOVs
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
-; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9].[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
+; EG-DAG: BFE_INT {{[* ]*}}T{{[0-9]*.[XYZW]}}, {{.*}}, 0.0, literal
 ; EG-DAG: 8
 ; EG-DAG: 8
 ; EG-DAG: 8
 ; EG-DAG: 8
 ; EG-DAG: 8
 ; EG-DAG: 8
 ; EG-DAG: 8
 ; EG-DAG: 8

test/CodeGen/AMDGPU/load-local-i16.ll

 
 ; GCN: ds_write_b64 v{{[0-9]+}}, v{{\[}}[[LO]]:[[HI]]]
 
 ; EG: MOV {{[* ]*}}[[FROM:T[0-9]+\.[XYZW]]], KC0[2].Z
 ; EG: LDS_USHORT_READ_RET {{.*}} [[FROM]]
 ; EG-DAG: MOV {{[* ]*}}[[DATA:T[0-9]+\.[XYZW]]], OQAP
 ; EG-DAG: MOV {{[* ]*}}[[TO:T[0-9]+\.[XYZW]]], KC0[2].Y
 ; EG-DAG: LDS_WRITE
-; EG: LDS_WRITE {{\*?}} [[TO]], [[DATA]]
 define amdgpu_kernel void @local_zextload_i16_to_i64(i64 addrspace(3)* %out, i16 addrspace(3)* %in) #0 {
   %a = load i16, i16 addrspace(3)* %in
   %ext = zext i16 %a to i64
   store i64 %ext, i64 addrspace(3)* %out
   ret void
 }
 
 ; FUNC-LABEL: {{^}}local_sextload_i16_to_i64:

 
 ; FUNC-LABEL: {{^}}local_zextload_v1i16_to_v1i64:
 
 ; EG: MOV {{[* ]*}}[[FROM:T[0-9]+\.[XYZW]]], KC0[2].Z
 ; EG: LDS_USHORT_READ_RET {{.*}} [[FROM]]
 ; EG-DAG: MOV {{[* ]*}}[[DATA:T[0-9]+\.[XYZW]]], OQAP
 ; EG-DAG: MOV {{[* ]*}}[[TO:T[0-9]+\.[XYZW]]], KC0[2].Y
 ; EG-DAG: LDS_WRITE
-; EG: LDS_WRITE {{\*?}} [[TO]], [[DATA]]
 define amdgpu_kernel void @local_zextload_v1i16_to_v1i64(<1 x i64> addrspace(3)* %out, <1 x i16> addrspace(3)* %in) #0 {
   %load = load <1 x i16>, <1 x i16> addrspace(3)* %in
   %ext = zext <1 x i16> %load to <1 x i64>
   store <1 x i64> %ext, <1 x i64> addrspace(3)* %out
   ret void
 }
 
 ; FUNC-LABEL: {{^}}local_sextload_v1i16_to_v1i64:

test/CodeGen/ARM/2009-10-27-double-align.ll

-; RUN: llc < %s  -mtriple=arm-linux-gnueabi | FileCheck %s
-; RUN: llc < %s  -mtriple=arm-linux-gnueabi -regalloc=basic | FileCheck %s
+; RUN: llc < %s  -mtriple=arm-linux-gnueabi | FileCheck %s --check-prefix=NOREGALLOC
+; RUN: llc < %s  -mtriple=arm-linux-gnueabi -regalloc=basic | FileCheck %s --check-prefix=REGALLOC
 
 @.str = private constant [1 x i8] zeroinitializer, align 1
 
 define void @g() {
 entry:
 ;CHECK: [sp, #8]
-;CHECK: [sp, #12]
-;CHECK: [sp]
+;NOREGALLOC: [sp, #12]
+;NOREGALLOC: [sp]
+;REGALLOC: [sp]
+;REGALLOC: [sp, #12]
         tail call  void (i8*, ...) @f(i8* getelementptr ([1 x i8], [1 x i8]* @.str, i32 0, i32 0), i32 1, double 2.000000e+00, i32 3, double 4.000000e+00)
         ret void
 }
 
 declare void @f(i8*, ...)

test/CodeGen/ARM/illegal-bitfield-loadstore.ll

 ; LE-NEXT:    orr r1, r1, #384
 ; LE-NEXT:    bic r1, r1, #127
 ; LE-NEXT:    str r1, [r0]
 ; LE-NEXT:    mov pc, lr
 ;
 ; BE-LABEL: i56_and_or:
 ; BE:       @ BB#0:
 ; BE-NEXT:    mov r1, r0
+; BE-NEXT:    ldr r12, [r0]
 ; BE-NEXT:    mov r3, #128
 ; BE-NEXT:    ldrh r2, [r1, #4]!
-; BE-NEXT:    strb r3, [r1, #2]
 ; BE-NEXT:    lsl r2, r2, #8
-; BE-NEXT:    ldr r12, [r0]
+; BE-NEXT:    strb r3, [r1, #2]
 ; BE-NEXT:    orr r2, r2, r12, lsl #24
 ; BE-NEXT:    orr r2, r2, #384
 ; BE-NEXT:    lsr r3, r2, #8
 ; BE-NEXT:    strh r3, [r1]
 ; BE-NEXT:    bic r1, r12, #255
 ; BE-NEXT:    orr r1, r1, r2, lsr #24
 ; BE-NEXT:    str r1, [r0]
 ; BE-NEXT:    mov pc, lr

test/CodeGen/X86/illegal-bitfield-loadstore.ll

; CHECK-NEXT:    retq
   ret void
 }
 
 define void @i56_or(i56* %a) {
 ; CHECK-LABEL: i56_or:
 ; CHECK:       # BB#0:
 ; CHECK-NEXT:    movzwl 4(%rdi), %eax
 ; CHECK-NEXT:    movzbl 6(%rdi), %ecx
-; CHECK-NEXT:    movl (%rdi), %edx
 ; CHECK-NEXT:    movb %cl, 6(%rdi)
 ; CHECK-NEXT:    # kill: %ECX<def> %ECX<kill> %RCX<kill> %RCX<def>
 ; CHECK-NEXT:    shll $16, %ecx
 ; CHECK-NEXT:    orl %eax, %ecx
 ; CHECK-NEXT:    shlq $32, %rcx
-; CHECK-NEXT:    orq %rcx, %rdx
-; CHECK-NEXT:    orq $384, %rdx # imm = 0x180
-; CHECK-NEXT:    movl %edx, (%rdi)
-; CHECK-NEXT:    shrq $32, %rdx
-; CHECK-NEXT:    movw %dx, 4(%rdi)
+; CHECK-NEXT:    movl (%rdi), %eax
+; CHECK-NEXT:    orq %rcx, %rax
+; CHECK-NEXT:    orq $384, %rax # imm = 0x180
+; CHECK-NEXT:    movl %eax, (%rdi)
+; CHECK-NEXT:    shrq $32, %rax
+; CHECK-NEXT:    movw %ax, 4(%rdi)
 ; CHECK-NEXT:    retq
   %aa = load i56, i56* %a, align 1
   %b = or i56 %aa, 384
   store i56 %b, i56* %a, align 1
   ret void
 }
 
 define void @i56_and_or(i56* %a) {
 ; CHECK-LABEL: i56_and_or:
 ; CHECK:       # BB#0:
 ; CHECK-NEXT:    movzwl 4(%rdi), %eax
 ; CHECK-NEXT:    movzbl 6(%rdi), %ecx
-; CHECK-NEXT:    movl (%rdi), %edx
 ; CHECK-NEXT:    movb %cl, 6(%rdi)
 ; CHECK-NEXT:    # kill: %ECX<def> %ECX<kill> %RCX<kill> %RCX<def>
 ; CHECK-NEXT:    shll $16, %ecx
 ; CHECK-NEXT:    orl %eax, %ecx
 ; CHECK-NEXT:    shlq $32, %rcx
-; CHECK-NEXT:    orq %rcx, %rdx
-; CHECK-NEXT:    orq $384, %rdx # imm = 0x180
-; CHECK-NEXT:    movabsq $72057594037927808, %rax # imm = 0xFFFFFFFFFFFF80
-; CHECK-NEXT:    andq %rdx, %rax
-; CHECK-NEXT:    movl %eax, (%rdi)
-; CHECK-NEXT:    shrq $32, %rax
-; CHECK-NEXT:    movw %ax, 4(%rdi)
+; CHECK-NEXT:    movl (%rdi), %eax
+; CHECK-NEXT:    orq %rcx, %rax
+; CHECK-NEXT:    orq $384, %rax # imm = 0x180
+; CHECK-NEXT:    movabsq $72057594037927808, %rcx # imm = 0xFFFFFFFFFFFF80
+; CHECK-NEXT:    andq %rax, %rcx
+; CHECK-NEXT:    movl %ecx, (%rdi)
+; CHECK-NEXT:    shrq $32, %rcx
+; CHECK-NEXT:    movw %cx, 4(%rdi)
 ; CHECK-NEXT:    retq
   %b = load i56, i56* %a, align 1
   %c = and i56 %b, -128
   %d = or i56 %c, 384
   store i56 %d, i56* %a, align 1
   ret void
 }
 
 define void @i56_insert_bit(i56* %a, i1 zeroext %bit) {
 ; CHECK-LABEL: i56_insert_bit:
 ; CHECK:       # BB#0:
 ; CHECK-NEXT:    movzbl %sil, %eax
 ; CHECK-NEXT:    movzwl 4(%rdi), %ecx
 ; CHECK-NEXT:    movzbl 6(%rdi), %edx
-; CHECK-NEXT:    movl (%rdi), %esi
 ; CHECK-NEXT:    movb %dl, 6(%rdi)
 ; CHECK-NEXT:    # kill: %EDX<def> %EDX<kill> %RDX<kill> %RDX<def>
 ; CHECK-NEXT:    shll $16, %edx
 ; CHECK-NEXT:    orl %ecx, %edx
 ; CHECK-NEXT:    shlq $32, %rdx
-; CHECK-NEXT:    orq %rdx, %rsi
+; CHECK-NEXT:    movl (%rdi), %ecx
+; CHECK-NEXT:    orq %rdx, %rcx
 ; CHECK-NEXT:    shlq $13, %rax
-; CHECK-NEXT:    movabsq $72057594037919743, %rcx # imm = 0xFFFFFFFFFFDFFF
-; CHECK-NEXT:    andq %rsi, %rcx
-; CHECK-NEXT:    orq %rax, %rcx
-; CHECK-NEXT:    movl %ecx, (%rdi)
-; CHECK-NEXT:    shrq $32, %rcx
-; CHECK-NEXT:    movw %cx, 4(%rdi)
+; CHECK-NEXT:    movabsq $72057594037919743, %rdx # imm = 0xFFFFFFFFFFDFFF
+; CHECK-NEXT:    andq %rcx, %rdx
+; CHECK-NEXT:    orq %rax, %rdx
+; CHECK-NEXT:    movl %edx, (%rdi)
+; CHECK-NEXT:    shrq $32, %rdx
+; CHECK-NEXT:    movw %dx, 4(%rdi)
 ; CHECK-NEXT:    retq
   %extbit = zext i1 %bit to i56
   %b = load i56, i56* %a, align 1
   %extbit.shl = shl nuw nsw i56 %extbit, 13
   %c = and i56 %b, -8193
   %d = or i56 %c, %extbit.shl
   store i56 %d, i56* %a, align 1
   ret void
 }