GlobalISel: Handle more cases for widenScalar for G_STORE

[aemerson]

lib/CodeGen/GlobalISel/LegalizerHelper.cpp

934

Why is this being changed?

946

SelectionDAG does zext for the same IR for all types, so I think we should be consistent there.

[arsenm]

lib/CodeGen/GlobalISel/LegalizerHelper.cpp

934

This is the entire problem. This was only handling the 1-to-8 bit case. All the AMDGPU 8 or 16-bit stores use 32-bit source registers

946

I don't think that's true?

In this test case for example, in PromoteIntOp_STORE/PromoteIntRes_LOAD it decided to use the morally equivalent ISD::EXTLOAD extload type.

define void @foo(i8 addrspace(1)* %in, i8 addrspace(1)* %out) {

%val = load i8, i8 addrspace(1)* %in
store i8 %val, i8 addrspace(1)* %out
ret void

}

The high bits also won't matter for a truncating store

[aemerson]

lib/CodeGen/GlobalISel/LegalizerHelper.cpp

934

Oh sorry, for some reason I thought this hunk was the before-diff not the *after*.

946

So I was using a test case like this to check:

define void @test(i12 %v, i12 *%ptr) {
  store i12 %v, i12* %ptr
  ret void
}

I get your point that in theory it should be semantically correct to do anyext. However I'd rather not we change the way i1s are handled.

[arsenm]

lib/CodeGen/GlobalISel/LegalizerHelper.cpp

946

This is preserving how i1 is handled. It's using zext for i1, and anyext for anything else. I don't think any other non-byte sizes are special cased though

[aemerson]

lib/CodeGen/GlobalISel/LegalizerHelper.cpp

946

Yes, can you remove the comment about i1s before the code then.

[arsenm]

r352585

lib/CodeGen/GlobalISel/LegalizerHelper.cpp

LegalizerHelper::widenScalar(MachineInstr &MI, unsigned TypeIdx, LLT WideTy) {
   case TargetOpcode::G_SEXTLOAD:
   case TargetOpcode::G_ZEXTLOAD:
     Observer.changingInstr(MI);
     widenScalarDst(MI, WideTy);
     Observer.changedInstr(MI);
     return Legalized;
 
   case TargetOpcode::G_STORE: {
-    if (MRI.getType(MI.getOperand(0).getReg()) != LLT::scalar(1) ||
-        WideTy != LLT::scalar(8))
+    if (TypeIdx != 0)
+      return UnableToLegalize;
+
+    LLT Ty = MRI.getType(MI.getOperand(0).getReg());
+    if (!isPowerOf2_32(Ty.getSizeInBits()))
       return UnableToLegalize;
 
     Observer.changingInstr(MI);
-    widenScalarSrc(MI, WideTy, 0, TargetOpcode::G_ZEXT);
+
+    // XXX: Should this really be making an exception for i1? It doesn't
+    // necessarily make sense to be using the target boolean type here.
+    unsigned ExtType = Ty.getScalarSizeInBits() == 1 ?
+      TargetOpcode::G_ZEXT : TargetOpcode::G_ANYEXT;
+    widenScalarSrc(MI, WideTy, 0, ExtType);
+
     Observer.changedInstr(MI);
     return Legalized;
   }
   case TargetOpcode::G_CONSTANT: {
     MachineOperand &SrcMO = MI.getOperand(1);
     LLVMContext &Ctx = MIRBuilder.getMF().getFunction().getContext();
     const APInt &Val = SrcMO.getCImm()->getValue().sext(WideTy.getSizeInBits());
     Observer.changingInstr(MI);

test/CodeGen/AMDGPU/GlobalISel/legalize-store.mir

 body: |
   bb.0:
     liveins: $vgpr0_vgpr1, $vgpr2_vgpr3_vgpr4_vgpr5
 
     ; CHECK-LABEL: name: test_truncstore_global_s128_to_s16
     ; CHECK: [[COPY:%[0-9]+]]:_(p1) = COPY $vgpr0_vgpr1
     ; CHECK: [[COPY1:%[0-9]+]]:_(s128) = COPY $vgpr2_vgpr3_vgpr4_vgpr5
     ; CHECK: [[TRUNC:%[0-9]+]]:_(s32) = G_TRUNC [[COPY1]](s128)
+    ; CHECK: G_STORE [[TRUNC]](s32), [[COPY]](p1) :: (store 2, addrspace 1)
+    %0:_(p1) = COPY $vgpr0_vgpr1
+    %1:_(s128) = COPY $vgpr2_vgpr3_vgpr4_vgpr5
+    G_STORE %1, %0 :: (store 2, addrspace 1)
+...
+
+---
+name: test_truncstore_global_s128_to_s8
+body: |
+  bb.0:
+    liveins: $vgpr0_vgpr1, $vgpr2_vgpr3_vgpr4_vgpr5
+
+    ; CHECK-LABEL: name: test_truncstore_global_s128_to_s8
+    ; CHECK: [[COPY:%[0-9]+]]:_(p1) = COPY $vgpr0_vgpr1
+    ; CHECK: [[COPY1:%[0-9]+]]:_(s128) = COPY $vgpr2_vgpr3_vgpr4_vgpr5
+    ; CHECK: [[TRUNC:%[0-9]+]]:_(s32) = G_TRUNC [[COPY1]](s128)
     ; CHECK: G_STORE [[TRUNC]](s32), [[COPY]](p1) :: (store 1, addrspace 1)
     %0:_(p1) = COPY $vgpr0_vgpr1
     %1:_(s128) = COPY $vgpr2_vgpr3_vgpr4_vgpr5
     G_STORE %1, %0 :: (store 1, addrspace 1)
 ...
+
+name: test_store_global_i1
+body: |
+  bb.0:
+    liveins: $vgpr0_vgpr1, $vgpr2
+
+    ; CHECK-LABEL: name: test_store_global_i1
+    ; CHECK: [[COPY:%[0-9]+]]:_(p1) = COPY $vgpr0_vgpr1
+    ; CHECK: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr2
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 1
+    ; CHECK: [[COPY2:%[0-9]+]]:_(s32) = COPY [[COPY1]](s32)
+    ; CHECK: [[AND:%[0-9]+]]:_(s32) = G_AND [[COPY2]], [[C]]
+    ; CHECK: G_STORE [[AND]](s32), [[COPY]](p1) :: (store 1, addrspace 1)
+    %0:_(p1) = COPY $vgpr0_vgpr1
+    %1:_(s32) = COPY $vgpr2
+    %2:_(s1) = G_TRUNC %1
+    G_STORE %2, %0 :: (store 1, addrspace 1)
+...
+
+---
+name: test_store_global_i8
+body: |
+  bb.0:
+    liveins: $vgpr0_vgpr1, $vgpr2
+
+    ; CHECK-LABEL: name: test_store_global_i8
+    ; CHECK: [[COPY:%[0-9]+]]:_(p1) = COPY $vgpr0_vgpr1
+    ; CHECK: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr2
+    ; CHECK: [[COPY2:%[0-9]+]]:_(s32) = COPY [[COPY1]](s32)
+    ; CHECK: G_STORE [[COPY2]](s32), [[COPY]](p1) :: (store 1, addrspace 1)
+    %0:_(p1) = COPY $vgpr0_vgpr1
+    %1:_(s32) = COPY $vgpr2
+    %2:_(s8) = G_TRUNC %1
+    G_STORE %2, %0 :: (store 1, addrspace 1)
+...
+
+---
+name: test_store_global_i16
+body: |
+  bb.0:
+    liveins: $vgpr0_vgpr1, $vgpr2
+
+    ; CHECK-LABEL: name: test_store_global_i16
+    ; CHECK: [[COPY:%[0-9]+]]:_(p1) = COPY $vgpr0_vgpr1
+    ; CHECK: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr2
+    ; CHECK: [[COPY2:%[0-9]+]]:_(s32) = COPY [[COPY1]](s32)
+    ; CHECK: G_STORE [[COPY2]](s32), [[COPY]](p1) :: (store 2, addrspace 1)
+    %0:_(p1) = COPY $vgpr0_vgpr1
+    %1:_(s32) = COPY $vgpr2
+    %2:_(s16) = G_TRUNC %1
+    G_STORE %2, %0 :: (store 2, addrspace 1)
+...
+
+---
+name: test_store_global_i128
+body: |
+  bb.0:
+    liveins: $vgpr0_vgpr1, $vgpr2_vgpr3_vgpr4_vgpr5
+
+    ; CHECK-LABEL: name: test_store_global_i128
+    ; CHECK: [[COPY:%[0-9]+]]:_(p1) = COPY $vgpr0_vgpr1
+    ; CHECK: [[COPY1:%[0-9]+]]:_(s128) = COPY $vgpr2_vgpr3_vgpr4_vgpr5
+    ; CHECK: G_STORE [[COPY1]](s128), [[COPY]](p1) :: (store 16, addrspace 1)
+    %0:_(p1) = COPY $vgpr0_vgpr1
+    %1:_(s128) = COPY $vgpr2_vgpr3_vgpr4_vgpr5
+    G_STORE %1, %0 :: (store 16, addrspace 1)
+...
+
+---
+name: test_store_global_v2s64
+body: |
+  bb.0:
+    liveins: $vgpr0_vgpr1, $vgpr2_vgpr3_vgpr4_vgpr5
+
+    ; CHECK-LABEL: name: test_store_global_v2s64
+    ; CHECK: [[COPY:%[0-9]+]]:_(p1) = COPY $vgpr0_vgpr1
+    ; CHECK: [[COPY1:%[0-9]+]]:_(<2 x s64>) = COPY $vgpr2_vgpr3_vgpr4_vgpr5
+    ; CHECK: G_STORE [[COPY1]](<2 x s64>), [[COPY]](p1) :: (store 16, addrspace 1)
+    %0:_(p1) = COPY $vgpr0_vgpr1
+    %1:_(<2 x s64>) = COPY $vgpr2_vgpr3_vgpr4_vgpr5
+    G_STORE %1, %0 :: (store 16, addrspace 1)
+
+...