[PATCH] D57071: GlobalISel: Handle more cases for widenScalar for G_STORE

[Matt Arsenault via Phabricator via llvm-commits]

arsenm marked 2 inline comments as done.
arsenm added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:934
   case TargetOpcode::G_STORE: {
-    if (MRI.getType(MI.getOperand(0).getReg()) != LLT::scalar(1) ||
-        WideTy != LLT::scalar(8))
+    if (TypeIdx != 0)
+      return UnableToLegalize;
----------------
aemerson wrote:
> Why is this being changed?
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


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:946
+    unsigned ExtType = Ty.getScalarSizeInBits() == 1 ?
+      TargetOpcode::G_ZEXT : TargetOpcode::G_ANYEXT;
+    widenScalarSrc(MI, WideTy, 0, ExtType);
----------------
aemerson wrote:
> SelectionDAG does zext for the same IR for all types, so I think we should be consistent there.
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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57071/new/

https://reviews.llvm.org/D57071